Skip to content

radsecproxy: add more config options for UCI#30044

Open
Janfred wants to merge 1 commit into
openwrt:masterfrom
Janfred:feature_radsecproxy
Open

radsecproxy: add more config options for UCI#30044
Janfred wants to merge 1 commit into
openwrt:masterfrom
Janfred:feature_radsecproxy

Conversation

@Janfred

@Janfred Janfred commented Jul 19, 2026

Copy link
Copy Markdown

📦 Package Details

Maintainer: @commodo

Description:
This patch adds additional configuration options that can come from the UCI config files.


🧪 Run Testing Details

  • OpenWrt Version: 25.12.5 r33051-f5dae5ece4
  • OpenWrt Target/Subtarget: ath79/generic
  • OpenWrt Device: TP-Link Archer C7 v5

✅ Formalities

  • I have reviewed the CONTRIBUTING.md file for detailed contributing guidelines.

@Janfred
Janfred force-pushed the feature_radsecproxy branch from f09a6a6 to 01223a0 Compare July 19, 2026 12:24
@Janfred Janfred changed the title WIP: radsecproxy: add more config options for UCI radsecproxy: add more config options for UCI Jul 19, 2026

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 new commit. One inline note on package release hygiene; the commit message accurately describes the change.


Generated by Claude Code

Include host type secret tls matchCertificateAttribute duplicateInterval \
AddTTL fticksVISCOUNTRY fticksVISINST rewrite rewriteIn rewriteOut \
rewriteAttribute
rewriteAttribute serverName PSKkey PSKidentity

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds new UCI-driven directives to the installed /etc/init.d/radsecproxy script, which is a functional change to package contents. PKG_RELEASE in the Makefile should be incremented (2 → 3) so existing installs receive the rebuilt package on opkg upgrade. See net/radsecproxy/Makefile#L12. The bot's release-audit note applies here — this is not a cosmetic edit, so the "do not increment for minor changes" exception does not apply.


Generated by Claude Code

@commodo

commodo commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

needs PKG_RELEASE version bump

rest looks fine

@Janfred

Janfred commented Jul 21, 2026

Copy link
Copy Markdown
Author

I'm right now looking at more config options, so I'll push an update, and add the PKG_RELEASE version bump in there as well.

Unfortunately the init script also needs a change, because right now everything is escaped with single quotes, and the single quote has a special meaning for the attribute rewrites.

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the new commit (dc02adaa). The PKG_RELEASE bump requested earlier is now in place and the commit message matches the changes. Two inline issues on the new init-script changes: a shell syntax error in the escaping case block, and a misspelled TLS directive.


Generated by Claude Code

Comment thread net/radsecproxy/files/radsecproxy.init Outdated
Comment on lines +26 to +27
*"'"*) echo " $param \"$value\"" >> "$CONFFILE"
*) echo " $param '$value'" >> "$CONFFILE"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both case branches are missing their ;; terminators, so this is a shell syntax error, not just a style slip. sh -n/dash -n on this block fails with Syntax error: ")" unexpected (expecting ";;"). Because it sits inside append_params, which every *_block function calls, the whole init script fails to parse and radsecproxy can no longer be configured/started — this breaks the package rather than just the new escaping path.

Suggested change
*"'"*) echo " $param \"$value\"" >> "$CONFFILE"
*) echo " $param '$value'" >> "$CONFFILE"
*"'"*) echo " $param \"$value\"" >> "$CONFFILE" ;;
*) echo " $param '$value'" >> "$CONFFILE" ;;

Generated by Claude Code

Comment thread net/radsecproxy/files/radsecproxy.init Outdated
append_params "$cfg" \
Include CACertificateFile CACertificatePath certificateFile certificateKeyFile \
certificateKeyPassword cacheExpiry policyOID
certificateKeyPassword cacheExpiry policyOID CipherList ChiperhSuites \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ChiperhSuites appears to be a typo. The adjacent token on the same line spells Cipher correctly (CipherList), and the radsecproxy TLS directive for cipher suites is CipherSuites. As written, the emitted directive/option name is misspelled, so setting the cipher suites via UCI would silently not take effect (or produce an unknown directive). (I could not reach upstream docs from this environment to confirm the exact casing, but the intra-word transposition is self-evident against the neighbouring CipherList.)

Suggested change
certificateKeyPassword cacheExpiry policyOID CipherList ChiperhSuites \
certificateKeyPassword cacheExpiry policyOID CipherList CipherSuites \

Generated by Claude Code

This adds more config options for use in UCI, esp. new configuration
options that were introduced in newer radsecproxy versions and it fixes
an escaping issue in the init script where a string containing single
quotes would still be escaped using single quotes, causing radsecproxy
to reject the config.

This also bumps the PKG_RELEASE in the Makefile

Signed-off-by: Jan-Frederik Rieckers <rieckers@dfn.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants