Improve Dynamic Home Server Documentation & Exec Integration for NAPTR Resolution #5598
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This refactor keeps functionality equivalent to the original, while improving: • Input validation (with quoting and read -r) • Shell hygiene (e.g., set --, $(...)) • Compatibility with FreeRADIUS exec module expectations • File safety using validated $REALM instead of raw $1 • Noise suppression in production via >/dev/null 2>&1 • Maintainability through clearer control flow and comments
Author
|
Cleaned up PR from previous #5595 which I closed. As far as maintaining the v.3.2.x branch commit history this pr can be squashed at merge time. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
✅ Description
This PR improves and corrects the existing documentation and supporting script for dynamic home server provisioning using DNS-based NAPTR/SRV resolution, targeted for FreeRADIUS v3.2.x deployments.
It addresses key gaps and broken behaviors in the original guidance, while preserving existing behavior and introducing a clean, production-ready script that can be used in real-world OpenRoaming, eduroam, or other dynamic federation scenarios.
⸻
📌 Summary of Changes
🧾 Documentation
• Clarified how dynamic home servers are enabled and configured via proxy.conf.
• Corrected guidance on using the exec module for dynamic discovery, including:
• Properly capturing output using wait = yes.
• Avoiding %{exec:...} failures caused by non-blocking behavior.
• Ensured compatibility notes and expected behavior are explicitly explained.
🔧 Code and Script
• Introduced a new module block: dpd_exec
• Based on exec, but with wait = yes and a short timeout for safety and predictability.
• Refactored the DNS NAPTR resolution script with the following improvements:
• Input validation and safe argument handling (
read -r, quoting)• POSIX shell hygiene (
set --,$(...))• Compatible with FreeRADIUS
execexpectations• Secure use of
$REALMto prevent command injection or unsafe file naming• Silent failure fallback (exits cleanly with status 0)
• Clean and maintainable structure with inline documentation
⸻
🔍 Technical Rationale
FreeRADIUS’s exec module does not wait for script completion by default, which leads to two critical issues when used in dynamic home server discovery:
1. The lookup may not complete in time, resulting in empty results.
2. Without wait = yes, script output cannot be used in attributes via %{exec:...} expansion.
To avoid altering global behavior or impacting other modules, a new module block dpd_exec was introduced with these safe defaults, providing targeted functionality.
The updated script and module were verified in a live FreeRADIUS v3.2.7 deployment.
⸻
⚙️ Why This PR Matters
The existing documentation does not reflect how dynamic home servers actually work in practice—especially when using DNS-based realm lookups.
This PR brings the documentation in line with functional, tested usage and provides a minimal, non-breaking patch that:
• Works as advertised
• Introduces no regressions
• Improves developer/operator clarity
• Avoids reliance on tribal knowledge or source-diving
⸻
✅ Tested With
• ✅ FreeRADIUS v3.2.7
• ✅ Dynamic realm configuration
• ✅ OpenRoaming NAPTR/SRV fallback behavior
• ✅ Live RADIUS proxy environment
• ✅ Both failed and successful DNS resolution scenarios
⸻
🧠 A Note on Style and Commit History
All unnecessary whitespace or formatting changes have been removed.
⸻
🙋 Feedback Welcome
If there’s a more idiomatic way to accomplish the same goal or if this could be better integrated into the existing module ecosystem, I’d appreciate the guidance.