[RFC] nginx-util: use UCI for server configuration

[peter-stadler]

Maintainer: me, @Ansuel and @heil (for nginx)
Compile tested: x86_64, x86_64 qemu, master
Run tested: x86_64, x86_64 qemu, master, run tests, nginx and luci-nginx

Description: It is based on the discussion in #11456 and described below.

There is also a new feature: add_ssl resp. del_ssl accept a manager as an optional parameter for adding resp. removing SSL directives in the configuration without creating resp. removing SSL certificate and key (it is expected that the manager puts/links resp. deletes the files at /etc/nginx/conf.d/$name.{crt,key}). edit: nginx-util will create symlinks /etc/nginx/conf.d/$name.{crt,key} to the provided paths if needed (see below). It could be used e.g. by (u)acme as nginx-util add_ssl $name acme /path/to/crt /path/to/key, @tohojo and @lucize what do you think?

tl;dr: The functions {add,del}_ssl modify a server section of the UCI config if there is no .conf file with the same name in /etc/nginx/conf.d/.

Then init_lan creates /var/lib/nginx/uci.conf files by copying the /etc/nginx/uci.conf.template and standard options from the UCI config; additionally the special path logd can be used in {access,error}_log.

The init does not change the configuration beside re-creating self-signed certificates when needed. This is also the only purpose of the new check_ssl, which is installed as yearly cron job.

Initialization:

Invoking nginx-util init_lan parses the UCI configuration for package nginx. It creates a server part in /var/lib/nginx/uci.conf for each section server '$name' by copying all UCI options but the following:

• option uci_manage_ssl is skipped. It is set to 'self-signed' by nginx-util add_ssl $name, removed by nginx-util del_ssl $name and used by nginx-util check_ssl (see below).

• logd as path in error_log or access_log writes them to STDERR respective STDOUT, which are fowarded by Nginx's init to the log daemon. Specifically:
  option error_log 'logd' becomes error_log stderr; and
  option access_log 'logd openwrt' becomes access_log /proc/self/fd/1 openwrt;

Other [option|list] key 'value' entries just become key value; directives.

The init.d calls internally also check_ssl for rebuilding self-signed SSL certificates if needed (see below). And it still sets up /var/lib/nginx/lan{,_ssl}.listen files as it is doing in the current version (so they stay available).

Defaults:

The package installs the file /etc/nginx/restrict_locally containing allow/deny directives for restricting the access to LAN addresses by including it into a server part. The default server '_lan' includes this file and listens on all IPs (instead of only the local IPs as it did before; other servers do not need to listen explicitly on the local IPs anymore). The default server is contained together with a server that redirects HTTP requests for inexistent URLs to HTTPS in the UCI configuration file /etc/config/nginx. Furthermore, the packages installs a /etc/nginx/uci.conf.template containing the current setup and a marker, which will be replaced by the created UCI servers when calling init_lan.

Other:

If there is a file named /etc/nginx/conf.d/$name.conf the functions init_lan, add_ssl $name and del_ssl $name will use that file instead of a UCI server section (this is similar to the current version).

Else it selects the UCI section server $name, or, when there is no such section, it searches for the first one having option server_name '… $name …'. For this section:

• nginx-util add_ssl $name will add to it:
  option uci_manage_ssl 'self-signed'
option ssl_certificate '/etc/nginx/conf.d/$name.crt'
option ssl_certificate_key '/etc/nginx/conf.d/$name.key'
option ssl_session_cache 'shared:SSL:32k'
option ssl_session_timeout '64m'
  If these options are already present, they will stay the same; just the first option uci_manage_ssl will always be changed to 'self-signed'. The command also changes all listen list items to use port 443 and ssl instead of port 80 (without ssl). If they stated another port than 80 before, they are kept the same. Furthermore, it creates a self-signed SSL certificate if necessary, i.e., if there is no valid certificate and key at the locations given by the options ssl_certificate and ssl_certificate_key.

• nginx-util del_ssl $name checks if uci_manage_ssl is set 'self-signed' in the corresponding UCI section. Only then it removes all of the above options regardless of the value looking just at the key name. Then, it also changes all listen list items to use port 80 (without ssl) instead of port 443 with ssl. If stating another port than 443, they are kept the same. Furthermore, it removes the SSL certificate and key that were indicated by ssl_certificate{,_key}.

• nginx-util check_ssl looks through all server sections of the UCI config for uci_manage_ssl 'self-signed'. On every hit it checks if the SSL certificate-key-pair indicated by the options ssl_certificate{,_key} is expired. Then it re-creates a self-signed certificate. If there exists at least one section server with uci_manage_ssl 'self-signed', it will try to install itself as cron job. If there are no such sections, it removes that cron job if possible.

edit: For installing a ssl certificate and key managed by another app, you can call:
nginx-util add_ssl $name $manager $crtpath $keypath
Hereby $name is as above, $manager is an arbitrary string, and the the ssl certificate and its key are indicated by their absolute path. If you want to remove the directives again, then you can use: nginx-util del_ssl $name $manager

[tohojo]

peter-stadler <notifications@github.com> writes:

There is also a new feature: `add_ssl` resp. `del_ssl` accept a manager as an optional parameter for adding resp. removing SSL directives in the configuration without creating resp. removing SSL certificate and key (it is expected that the manager puts/links resp. deletes the files at `/etc/nginx/conf.d/$name.{crt,key}`). It could be used e.g. by (u)acme as `nginx-util add_ssl $name acme`, @tohojo and @lucize what do you think?

I'm fine with supporting nginx-util from the acme wrapper script, but why can't it just supply a path to the cert?

[peter-stadler]

It was easier for me (less code change), but I would change it if it is too difficult (I do not expect a lot of apps to use this option, so I wanted to ask you if this would be used at all) …
Edit: I remember that I thought about allowing another path, but then it get less stable (I am using a regex for changing the conf files not a full parser, so a fixed path is more robust). I think the two links are the better option.

[tohojo]

peter-stadler <notifications@github.com> writes:

It was easier for me (less code change), but I would change it if it is too difficult (I do not expect a lot of apps to use this option, so I wanted to ask you if this would be used at all) …

As long as we don't need too much code to handle it, sure! Right now we're just doing sed-based replace of the cert path in /etc/nginx/nginx.conf, so if we could just wrap that in, say, something like: which nginx-util && nginx-util add_ssl acme $certfile $keyfile that would be pretty easy :)

[peter-stadler]

I change it to create/remove the symlinks (if needed) and will update the PR after some tests. The command line would be something like:
which nginx-util && nginx-util add_ssl "$domain" acme "$absolute_path_to_cert_file" "$absolute_path_to_key_file"
(other managers should use another name than acme)
Removing a managed certificate/key would be something like:
which nginx-util && nginx-util del_ssl "$domain" acme && rm "$absolute_path_to_cert_file" "$absolute_path_to_key_file"

[tohojo]

SGTM :)

[peter-stadler]

Done. I would mark it ready for review on Tuesday.

[peter-stadler]

(rebased)
I think I cannot get rid of the Lint failure, I suspect it is the same problem as described in issue #13395 ...

[peter-stadler]

(now the tests that where good before are failing, too, but I did not change any code beside rebasing …)

[tohojo]

From a 'using this in acme.sh' PoV, is there a way to discover whether nginx-util manages a particular domain? Can we just blindly call nginx-util add_ssl $DOMAIN ... and expect it to do the right thing (exit with an error, I suppose?) if no config exists for that domain? Or do we need to call nginx-util does-this-section-exist $DOMAIN first or something? :)

[peter-stadler]

Yes, it will exit with an error (I will post the output later).
Unrelated: I made a silly error when I rebased (using the old version), I mark it as draft until I correct it.
Thank you for asking :-)

[peter-stadler]

(rebased the right version)

For the output of nginx-util add_ssl $name acme /crt /key; echo "exit value: $?":

• If nginx-util cannot resolve $name it fails:

 * nginx-util lookup error: neither there is a file named '/etc/nginx/conf.d/$name.conf' nor the UCI config has a nginx server with section name or 'server_name': $name
exit value: 1

• If there is a file /etc/nginx/conf.d/$name.conf, but it has no directive 'server_name $name' it fails:

 * nginx-util add_ssl_directives_to error: cannot add SSL directives to $name.conf, missing: 
    server_name '$name';

exit value: 1

• If there is a file /etc/nginx/conf.d/$name.conf with directive server_name $name, it will try to create symlinks to the supplied /crt and /key at /etc/nginx/conf.d/$name.crt and /etc/nginx/conf.d/$name.key. This fails if there are already files with such names, e.g.:

 * nginx-util add_ssl_if_needed error: cannot link ssl_certificate /etc/nginx/conf.d/$name.crt -> /crt (17): File exists
exit value: 1

• If there is a file /etc/nginx/conf.d/$name.conf with directive server_name $name and no files /etc/nginx/conf.d/$name.crt and /etc/nginx/conf.d/$name.key it will succeed; regardless if there are files /crt and /key, it is supposed that the manager (=acme) will create them:

Added SSL directives to /etc/nginx/conf.d/$name.conf
exit value: 0

• If there is no file /etc/nginx/conf.d/$name.conf but the UCI config has a nginx server with section name or 'server_name': $name, it will succeed; regardless if there are files /crt and /key or /etc/nginx/conf.d/$name.crt and /etc/nginx/conf.d/$name.key:

Adding SSL directives to UCI server: nginx.$name
        uci_manage_ssl='acme'
        ssl_certificate='/crt'
        ssl_certificate_key='/key'
        listen='443 ssl default_server' (replacing)
        listen='[::]:443 ssl default_server' (replacing)
        ssl_session_cache='shared:SSL:32k'
        ssl_session_timeout='64m'
nginx-util exit value: 0

Hereby only the SSL directives that will be changed are displayed.

Similar it is for nginx-util del_ssl $name acme ...

[peter-stadler]

(Renamed filter to key_filter in ubus.hpp because of issue #13754)

[peter-stadler]

I made some non-functional changes:

• Reformated the code with clang-format (and rebased).
• Added .clang-format and .clang-tidy files with the used configurations.
• Added a LICENSE file: BSD-2-Clause (as this is what Nginx uses)

[neheb]

Please rebase to fix those red checkmarks.

[peter-stadler]

Done. Thank you :-)

[neheb]

is this ready to go?

[peter-stadler]

Yes, I think so :-)