nut: enable SSL via NSS by default#29893
Conversation
danielfdickinson
force-pushed
the
pr-nut-enable-nss-for-ssl-by-default
branch
from
f6f5b0f to
bfb8e11
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Enable NSS (libnss / Mozilla NSS) as an SSL backend for NUT on OpenWrt, making NSS the default SSL provider.
Changes:
- Add
CONFIG_NUT_SSL_NSSbuild option and default it to enabled. - Add libnss as a conditional dependency and wire NSS/OpenSSL selection into
CONFIGURE_ARGS. - Minor whitespace/indent cleanup in
MakefileandConfig.in.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| net/nut/Makefile | Adds NSS config/deps and updates ./configure flags to select NSS/OpenSSL SSL backends. |
| net/nut/Config.in | Introduces a new NUT_SSL_NSS Kconfig option (default y) and adjusts OpenSSL option dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
danielfdickinson
force-pushed
the
pr-nut-enable-nss-for-ssl-by-default
branch
from
bfb8e11 to
240f8a6
Compare
danielfdickinson
force-pushed
the
pr-nut-enable-nss-for-ssl-by-default
branch
from
240f8a6 to
b9bacf5
Compare
danielfdickinson
force-pushed
the
pr-nut-enable-nss-for-ssl-by-default
branch
from
b9bacf5 to
c95a44d
Compare
|
@BKPepe I wonder if drafts could/should be excluded from LLM review and/or run testing until taken off draft? Or at least get a much lower priority. At least for me a draft is in (usually in vain) hopes of other eyeballs / comments / suggestions on a PR, but isn't necessarily ready for hard-core review. |
The previous configuration did not make clear the need for productid and vendorid options. See openwrt#29607 (comment) Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
We should fallback to the default if one is not specified. In addition find_statepath is the wrong place to check for its existence - it should be created by the initscript if it does not exist. See openwrt#29607 (comment) Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
While driverpath is could in theory be configurable, this was not implemented in the OpenWrt NUT package, and there is not need for it. Therefore remove the sample configuration for it, and setting that configuration in the upsd.conf file. See openwrt#29607 (comment) Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
The trigger script and instance to reload should depend on which initscript is calling interface_triggers. It wasn't, so we fix that. See openwrt#29607 (comment) Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
The defaults were triggiering on any activity on the network, which caused excessive restarts. So default to no interface trigger, with 'all' (or specific interfaces) as an option. See openwrt#29607 (comment) Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Script executables have changed, therefore bump PKG_RELEASE Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
* Fixes a missing word in a comment * Fixes inconsistent indentation in sample configurations * Drops redundant default assignment in find_statepath() Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
We claimed we only supported a single upsd section named 'upsd' and a single uspmon section named 'upsmon', but when determining RUNAS and STATEPATH we were using overly-complex functions as we were attempting to support multiple upsd and upsmon sections. Simplify the logic by adhering to our stated support. While were are at it we make RUNAS handling more consistent between nut-server (upsd) and nut-monitor (upsmon). We no longer try to limp along if RUNAS is not set by find_runas; to do this properly would require over-complicated logic when setting file and directory permissions and ownership. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Factor out some common validation code into some validator functions, and add more UCI config validation. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Using add_interface_triggers and check_interface_up is a better separation of concerns and the code has little in common (no duplication of code concerns). While we are at it make both nut-monitor and nut-server failure of add_interface_triggers non-fatal with a logged error message. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Enable libnss (Mozilla NSS) for SSL, and make it the default. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
danielfdickinson
force-pushed
the
pr-nut-enable-nss-for-ssl-by-default
branch
from
c95a44d to
bf8bf3b
Compare
danielfdickinson
force-pushed
the
pr-nut-enable-nss-for-ssl-by-default
branch
2 times, most recently
from
c02f99a to
c6e9a12
Compare
openwrt-ai
left a comment
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed the NUT commits added since the last review (SSL/NSS options, cert verification, the shared NSS database setup, the RUNAS/STATEPATH and interface-trigger refactors, and the Makefile whitespace reformat). Commit messages match their changes. Two functional bugs in the new nut-{server,upsmon}.default uci-defaults scripts are noted inline, plus a couple of nits.
Generated by Claude Code
Expose the configuration knobs as UCI config for more SSL options. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Enable CERTVERIFY, CERTREQUEST, and related options for requiring verified certificates from peers. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
The new CI reminds on non-standard indentation, so adjust Makefile to quiet the CI. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Create a group to allow sharing directories files between upsd and upsmon, and create a shared NSS certificate/key database on postinst of upsd or upsmon (providing the database dir does not already exist). Also enables preserving the shared database across sysupgrades. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Update deprecated configure option for NUT build. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
danielfdickinson
force-pushed
the
pr-nut-enable-nss-for-ssl-by-default
branch
from
c6e9a12 to
c6fa3ec
Compare
openwrt-ai
left a comment
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed the changes since the last review (branch was rebased). The new diff addresses the prior feedback: group_exist → group_exists, the broken if "$(command -v certutil)" → if command -v certutil >/dev/null 2>&1, disable_weak_ssl now read via config_get_bool with an unconditional DISABLE_WEAK_SSL write, and the Makefile postinst shebang de-indented to column 0. All correct; no new issues found.
Generated by Claude Code
📦 Package Details
Maintainer: @danielfdickinson
Description:
Enable libnss (Mozilla NSS) for SSL, and make it the default.
* A nossl variant is not required. That is, with no certificate configured NSS-enabled version accepts the old default (no SSL) version's connections and vice-versa. Behaviour with certificates configured depends on whether SSL required is configured.
nut-server-config.shnut-monitor-config.sh🧪 Run Testing Details
✅ Formalities