Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FS#4121 - uhttpd: checked and used certificate are different in uhttpd.init #9106

Open
openwrt-bot opened this issue Nov 4, 2021 · 1 comment
Labels

Comments

@openwrt-bot
Copy link

@openwrt-bot openwrt-bot commented Nov 4, 2021

j123b567:

uhttpd init script checks different certificate files then it finally uses in some situations. This causes uhttpd fail to start.

In the init script, there are lines

config_get UHTTPD_KEY  "$cfg" key  /etc/uhttpd.key
config_get UHTTPD_CERT "$cfg" cert /etc/uhttpd.crt

Some checks are performed on UHTTPD_KEY and UHTTPD_CERT and later, there is

append_arg "$cfg" cert "-C"
append_arg "$cfg" key  "-K"

This can cause problem if key or cert option is missing. In the first place, defalt value is used and in the second place, empty string is used.

Deleting key and cert option and restarting uhttpd will pass all checks, new certificate and key will be generated in default location and later, empty certificate and key paths are passed to uhttpd which will fail.

I would like to send PR to fix this but I would like to ask for the best solution. Naive approach would be to just do

append_arg "$cfg" cert "-C" "$UHTTPD_CERT"
append_arg "$cfg" key  "-K" "$UHTTPD_KEY"

So it will fallback to already resolved values and at least don't fail, if key and cert are both missing.

Unfortunately, this can cause other problems comes with LuCI, where there is easily possible to screw up the upload completely and for example delete the certificate option and keeping the key option.

In this example UHTTPD_KEY will contain new key and UHTTPD_CERT will contain unrelated old certificate, which is wrong and again uhttpd fail to start completely.

After reading the source code of the init script, it seems, that the default fallback values never worked, so it would be safe to do solution like this

config_get UHTTPD_KEY  "$cfg" key
config_get UHTTPD_CERT "$cfg" cert

This is also more robust to wrong configurations from LuCI because at least pure HTTP will be available in most cases.

Default values /etc/uhttpd.key and /etc/uhttpd.crt are still present in default uci config, so nothing should change for anybody and it will just be more robust to wrong configuration from LuCI.

@openwrt-bot
Copy link
Author

@openwrt-bot openwrt-bot commented Nov 8, 2021

j123b567:

Default values are not present anywhere else so
login screen from luci /usr/lib/lua/luci/view/sysauth.htm is also failing on

local https_key = uci:get("uhttpd", "main", "key")
...
if https_port and fs.access(https_key) then
https_port = https_port:match("(%d+)$")

with error message, that access need string not nil.

So answering myself. Removing default values from config_get in init script is probably the best solution for everything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant