nginx: support dynamic conf files

[hgl]

Maintainer: @Ansuel

Description:

This implements #19875, and is based on #19878

[hgl]

@peter-stadler let's discuss here, since the PR is ready for review.

First of all, thanks for the kinds. Totally forgot I cc'd you in the acme PR, lol.

---

I've just finished the implementation of dynamic configs, and tested it on a real device. It support two main features:

1. Listen on interfaces and make nginx reload whenever an interface changes.
2. Generate user specified self-signed certificates.

For #2, I modeled after uhttpd, letting users specify cert options in /etc/config/nginx.

This design should address the use cases you mentioned:

My use case is to install luci-nginx and etebase (another possibility would be ariang) and both should be accessible from LAN by default

Here I must take care as packager to not interfere with other apps (luci should be the only app at the root location /).

Since users are now supposed to have total control over nginx's config files, we just need to make sure the default config works and luci is served from /. Once they're are installed, users can edit them however they want, and it's their responsibility to ensure no paths should clash.

This prevents exposing the _lan server with all *.locations to the WAN (while the firewall is open for the other server).

Since it's now possible to listen on specific interfaces, firewall should be able to specify such rules. Allow/deny list should also be unnecessary.

---

Let me know if I missed other use cases that was provided by nginx-util.

[peter-stadler]

For the dynamic config mechanism

I stumbled upon the magic return values: I think it would be more verbose to let each dyn.conf.d/*.conf file create/delete/... the corresponding /var/etc/nginx/conf.d/*.conf file directly (maybe we should give them the directory as parameter $1 or a variable like $VAR_CONF_D).

The init reverses the behavior: Until now it uses a user created /etc/nginx/nginx.conf if present, else it creates and uses /etc/nginx/uci.conf. Now we would deliver a default /etc/nginx/nginx.conf and use it only if there is no /etc/nginx/uci.conf present. I am not sure if this can lead to problems ...

The $(ngx_ssl_cert) needs the user to install a px5g or openssl-util as generator (this was one reason to implement nginx-util, where I started with a px5g behavior but linked against openssl, which is used by nginx). Should we just add openssl-util as dependency?

Who is in charge of removing on uninstall the files automatically created by $(ngx_ssl_cert), e.g. by dyn.conf.d/luci installed with nginx-mod-luci?

Will you adopt the wiki after replacing nginx-util by the dynamic config, too?

Since it's now possible to listen on specific interfaces, firewall should be able to specify such rules. Allow/deny list should also be unnecessary.

Would I get all local IPs by $(ngx_listen lan 443 ssl) and $(ngx_listen localhost 443 ssl). Previously, I had an issue with link local addresses and rare problems with timing (e.g. on reboot). Or is the recommended way to use listen 443 and listen [::]:443 for the local server and add another server with $(ngx_listen wan 443 ssl)?

For the provided config

Do you see a problem in splitting dync.conf.d/luci into two files?

• dyn.conf.d/default_server installed by default nginx
• conf.d/luci.locations installed by nginx-mod-luci

I think it would be better to install the default_server together with nginx in the first place. Else it can be problematic if the user installs nginx, modifies the config and than installs nginx-mod-luci, which contains the default_server.

Maybe it would have been better to add a nginx-mod-etebase and nginx-mod-ariang to this Makefile (beside nginx-mod-luci) and let etebase respective ariang depend on it for installing the corresponding conf.d/*.locations. So these back dependencies would be visible, now it is only documented in the wiki that other apps can install such files.

Nonetheless, I would really like it, if other apps (like etebase and ariang) could still install their conf.d/*.locations in a way that they work out of the box locally; the user could include them in a WAN server part and open the firewall, too. If users need to change these files, they loose the ability to automatically follow upstream changes.

I do not understand, why the location .well-known/acme-challenge is installed with nginx-mod-luci. Which package is responsible for the destination /var/lib/acme/challenge?

[hgl]

I think it would be more verbose to let each dyn.conf.d/.conf file create/delete/... the corresponding /var/etc/nginx/conf.d/.conf file directly

With all due respect, I don't think it's a good idea. This loses one-to-one mapping. Currently a /var/etc/nginx/conf.d/*.conf is automatically removed if its corresponding dyn.conf.d/*.conf is removed. This wouldn't be possible if users can create arbitrary files.

Now we would deliver a default /etc/nginx/nginx.conf and use it only if there is no /etc/nginx/uci.conf present. I am not sure if this can lead to problems ...

What problems do you have in mind? I see three cases here:

1. /etc/nginx/uci.conf exists, and users install this new package, then their previous config is used, no dynamic config is loaded.
2. /etc/nginx/nginx.conf exists, and users install this new package, same as above
3. It's a fresh install, dynamic config takes effect.

Should we just add openssl-util as dependency?

I modeled this after uhttpd, who also uses them to generate certificates, but does not list them as dependencies. I'll ask jow what's his rational behind this choice.

Who is in charge of removing on uninstall the files automatically created by $(ngx_ssl_cert)

Users themselves. They can simply remove the $(ngx_ssl_cert) command from the dynamic config, and do a nginx reload, then ssl is automatically disabled. The nginx config files provided in this package are basically like the default config, users are responsible to modify them.

Will you adopt the wiki after replacing nginx-util by the dynamic config, too?

I can try. Would you do the honor of reviewing my draft?

Would I get all local IPs by $(ngx_listen lan 443 ssl) and $(ngx_listen localhost 443 ssl)
Previously, I had an issue with link local addresses and rare problems with timing

You get what's returned by network_get_ipaddr and network_get_ipaddrs6, which are IPs assigned on that interface, which I think should cover 99% of the use cases. I don't think we should make the effort to support link local addresses out of box, at least not without a compelling use case. With dynamic config, users can implement it themselves if they want it (it shouldn't be hard since parsing ip addr show br-lan should be enough). As for the timing issue, it shouldn't exist, since nginx will be automatically reloaded whenever interfaces change.

Or is the recommended way to use listen 443 and listen [::]:443 for the local server and add another server with $(ngx_listen wan 443 ssl)?

With dynamic config, users should only use listen 443 if they do want to listen for ALL interfaces.

---

Do you see a problem in splitting dync.conf.d/luci into two files?
dyn.conf.d/default_server installed by default nginx
conf.d/luci.locations installed by nginx-mod-luci

nginx by itself doesn't install any conf files in dyn.conf.d or conf.d except the wan example file which won't take effect, it only provides /etc/nginx/nginx.conf which contains no servers. nginx-mod-luci then adds dyn.conf.d/luci.conf.

Nonetheless, I would really like it, if other apps (like etebase and ariang) could still install their conf.d/*.locations in a way that they work out of the box locally

I guess there might be some misunderstanding here. In my mind they could just simply put files in conf.d or dyn.conf.d? What did I miss?

I do not understand, why the location .well-known/acme-challenge is installed with nginx-mod-luci.

It's not installed by nginx-mod-luci, but by nginx-ssl. /var/lib/acme/challenge is the standard acme challenge path we established in the acme package PR. The rational is that in the wan example, since ssl is used, users are very likely to also want to use acme to issue certificates, then they would want to include acme.conf to support that.

[hgl]

Should we just add openssl-util as dependency?

uhttpd's rational for not including openssl-util or px5g is that, without them, uhttpd simply disables https, and only provides http. I think we should do something similar in nginx.

[hgl]

I've updated PR that splits dyn.conf.d/luci to luci and luci-ssl. If openssl and px5g don't exist, no https luci is offered, just like how uhttpd works.

[github-actions]

No usage of AUTORELEASE found in changes

[peter-stadler]

I think it would be more verbose to let each dyn.conf.d/.conf file create/delete/... the corresponding /var/etc/nginx/conf.d/.conf file directly

[...] Currently a /var/etc/nginx/conf.d/*.conf is automatically removed if its corresponding dyn.conf.d/*.conf is removed. This wouldn't be possible if users can create arbitrary files.

So it would be feasible by passing the filename to be used. (The user can always create arbitrary files, but imho it is easier to get the magic values wrong.)

Now we would deliver a default /etc/nginx/nginx.conf and use it only if there is no /etc/nginx/uci.conf present. I am not sure if this can lead to problems ...

What problems do you have in mind? [...]

None in particular, so, it should be good.

Should we just add openssl-util as dependency?

I modeled this after uhttpd, who also uses them to generate certificates, but does not list them as dependencies. [...]

We already depend on libopenssl. The openssl-util would be complementary.

We decided a while ago to use nginx with ssl by default. I would like to stick with https, albeit uhttpd is doing it differently. (Are all users aware that luci_password is not encrypted, when using http?)

Who is in charge of removing on uninstall the files automatically created by $(ngx_ssl_cert)

Users themselves. [...]

What about the one created by dyn.conf.d/luci-ssl?

Will you adopt the wiki after replacing nginx-util by the dynamic config, too?

I can try. Would you do the honor of reviewing my draft?

Sure, but it will take some time.

Do you see a problem in splitting dync.conf.d/luci into two files?
dyn.conf.d/default_server installed by default nginx
conf.d/luci.locations installed by nginx-mod-luci

nginx by itself doesn't install any conf files in dyn.conf.d or conf.d except the wan example file which won't take effect, it only provides /etc/nginx/nginx.conf which contains no servers. nginx-mod-luci then adds dyn.conf.d/luci.conf.

I understood that and see the problem when the user:

1. installs nginx,
2. modifies the config
3. installs nginx-mod-luci, which contains the default_server.

So, do you see a problem to install the default_server already with nginx and let nginx-mod-luci add conf.d/luci.locations?

Nonetheless, I would really like it, if other apps (like etebase and ariang) could still install their conf.d/*.locations in a way that they work out of the box locally

I guess there might be some misunderstanding here. In my mind they could just simply put files in conf.d or dyn.conf.d? What did I miss?

They need a server part to run out of the box, but cannot install a server each at the same time. So, they need a common server part that includes their locations (when installing more than one of luci, etebase, ariang, ...)

I do not understand, why the location .well-known/acme-challenge is installed with nginx-mod-luci.

It's not installed by nginx-mod-luci, but by nginx-ssl.

I meant the include acme.conf in the old version of dyn.conf.d/luci. Now the https server part is in dyn.conf.d/luci-ssl, which is missing also the rest of the forwarding http server part:

server {
	listen 80 default_server;
	listen [::]:80 default_server;
	location / {
		return 302 https://\$host\$request_uri;
	}
}

/var/lib/acme/challenge is the standard acme challenge path we established in the acme package PR. The rational is that in the wan example, since ssl is used, users are very likely to also want to use acme to issue certificates, then they would want to include acme.conf to support that.

I think then the acme package should install the acme.conf file, too (else /var/lib/acme/challenge would not exist).

Edit: Sorry, I submitted too fast; had to format quotes correctly and improve the spelling.

BTW: I think functions.sh should use network_get_ipaddrs (instead of network_get_ipaddr) as it does with network_get_ipaddrs6

[hgl]

So it would be feasible by passing the filename to be used.

Asking users to deal with designated filename isn't necessarily making their lives easier IMHO. For example, if they need to remove the file, they either have to use rm -f or check previous config's existence. Since the filename is variable where users aren't supposed to know the real path, it can be especially weird.

And thinking about it, I actually want to eliminate the ability to do nothing for a dynamic config script (i.e., leaving previous generated config untouched). It has the potential to leave config in an unexpected state, and I can't think of a legitimate use case. If you agree supporting it might not be a good idea, it's impossible to enforce if users are allow to directly create/delete the file.

I would like to stick with https, albeit uhttpd is doing it differently.

(Are all users aware that luci_password is not encrypted, when using http?)

I don't think doing it in a better way just for nginx is a good idea here. If clear text luci_password is a concern, we should take it up with the uhttpd maintainers instead of just calling shots in our own package. IMHO, consistency is more important in this case.

Personally, I don't mind a http only config as long as users have the option to choose luci-nginx-ssl. And depending on libopenssl but not containing https config by default for luci-nginx (but users still have the ability to add https config themselves), sounds reasonable to me.

In other words, I think we should just provide a nginx package, and no nginx-ssl (or make nginx-ssl just depend on nginx for backward compatibility), then luci-nginx should provide http only config, and luci-nginx-ssl should depend on openssl-util or px5g to offer https config automatically. Just like how the uhttpd and luci packages work. What do you think?

What about the one created by dyn.conf.d/luci-ssl?

Sorry, maybe I missed something again? dyn.conf.d/*.conf are still considered user configs, so they can just remove $(ngx_ssl_cert) to disable https if they want?

(Hoping that users will treat dyn.conf.d/*.conf almost the same as conf.d/*.conf is one of the reasons I want to rely on return status instead of reminding users the notion of a generated config file)

So, do you see a problem to install the default_server already with nginx and let nginx-mod-luci add conf.d/luci.locations?

They need a server part to run out of the box, but cannot install a server each at the same time

I see what I missed, thanks for the clarification. Yeah it's a problem, and I think your previous suggestion of providing the default server in the main package is a good one. I'm not a fan of the .locations extension though. What if users have a WAN server with server_name, but also want isolated location files for that server?

What about a design like this:

conf.d/*.conf and dyn.conf.d/*.conf are supposed to contain server defs, conf.d/default_server/*.conf are supposed to contain location defs for the default server. For WAN servers with server_name, users can use conf.d/example.com/*.conf. Since these are legit nginx config files, let's not invent new extension name.

I meant the include acme.conf in the old version of dyn.conf.d/luci.

It was a mistake. I should've mentioned it in my previous reply. Since default https server uses a self-signed certificate, it should not support acme. It's fixed in my new commit. Sorry for the confusion.

I think then the acme package should install the acme.conf file, too

I don't think the acme package should directly deal with httpds. If it needs to add nginx config, then it needs to add config for all other httpds, which isn't maintainable.

else /var/lib/acme/challenge would not exist

Since acme.conf by default is only included in an example, it shouldn't be a concern. Users who actually modify the example to enable it are expected to install the acme package. Maybe we can comment the example to remind them that fact?

I think functions.sh should use network_get_ipaddrs

Good point. I'll fix that.

[peter-stadler]

Thank you for your detailed reply.

I agree with the points for the most part, the main difference I see with https:

I would like to stick with https, albeit uhttpd is doing it differently. [...]

[...] IMHO, consistency is more important in this case.

I see the inequality justified by the idea that uhttpd is very lightweight whereas nginx handles advanced use cases. I think we can expect that nginx is not used on the most limited devices.

For uhttpd one can choose between libustream-mbedtls, libustream-wolfssl or libustream-openssl, and could use corresponding px5g-mbedtls, px5g-wolfssl or openssl-util.

In contrast nginx depends on libopenssl; it makes less sense to depend on libmbedtls or libwolfssl, too. And I would expect more users of nginx than of uhttpd to use https. NB: acme.sh depends on openssl-util, but uacme could depend only on libopenssl (or libmbedtls or libgnutls).

In other words, I think [...] luci-nginx should provide http only config, and luci-nginx-ssl should depend on openssl-util or px5g to offer https config automatically. [...] What do you think?

You did write that before mentioning to move the default server to the main package. I will think the quoted statement with nginx and nginx-ssl instead of luci-nginx respective luci-nginx-ssl.

Then luci-nginx would depend on nginx, while luci-nginx-ssl would depend on nginx-ssl. But, would other apps (like ariang and etebase) provide two variants, too? Does the user has to keep them consistent? What if the user wants to change between nginx and nginx-ssl?

That is a lot of hassle beside maintaining two default_server. What do we gain besides the dependency on openssl-util less (would a smaller px5g-openssl help there)? Is it worth the effort?

---

For the rest I just have some notes:

[...] Since the filename is variable where users aren't supposed to know the real path, it can be especially weird.
And thinking about it, I actually want to eliminate the ability to do nothing for a dynamic config script [...]

I get the point. Then I would suggest commenting the return values in all provided dyn.conf.d files (luci-ssl or default_server and wan_example) besides documenting the dyn.conf.d mechanism in the wiki. BTW: Is it doing something if the dynamic config returns 1?

Sorry, maybe I missed something again? dyn.conf.d/*.conf are still considered user configs, so they can just remove $(ngx_ssl_cert) to disable https if they want?

I meant: If you would install nginx-mod-luci-ssl, start nginx and uninstall all again, you have a different state than before. The generated cert files are not deleted, are they?

I'm not a fan of the .locations extension though. What if users have a WAN server with server_name, but also want isolated location files for that server?

Currently I am including specific, e.g. etebase.locations in my wan server.

But I like your design:

conf.d/*.conf and dyn.conf.d/*.conf are supposed to contain server defs, conf.d/default_server/*.conf are supposed to contain location defs for the default server. For WAN servers with server_name, users can use conf.d/example.com/*.conf.

As this means to transition the other packages (luci, etebase and ariang), should we add there both ways for some time: conf.d/app.locations symlinked to conf.d/default_server/app.conf?

It was a mistake. I should've mentioned it in my previous reply. Since default https server uses a self-signed certificate, it should not support acme. It's fixed in my new commit.

No problem, I thought we talk about different things. Was it intentional to remove the redirect from http to https (for not interfering with the http only server)?

Since acme.conf by default is only included in an example, it shouldn't be a concern. Users who actually modify the example to enable it are expected to install the acme package. Maybe we can comment the example to remind them that fact?

We could comment the line include acme.conf; out and add another comment, that says to enable it if using acme.

[hgl]

I've updated the PR. I think it's in a much better shape, thanks for the suggestions.

Change summary:

1. Make default http server live at conf.d/default_server.conf.
2. Make dynamic configs live at dynconf.d/*.dynconf, so it's consistent with conf.d/*.conf and make it possible to define both dynconf.d/default_server.dynconf and dynconf.d/default_server/*.dynconf. Without the .dynconf, we previously can only create one dyn.conf.d/default_server.
3. Make default https server live at dynconf.d/default_server.dynconf
4. Make LuCI locations live at conf.d/default_server/luci.conf
5. In dynconf, returning 0 means generate the file and 1 remove the file.
6. Commented on how to use return status and acme.conf in WAN example file
7. Use network_get_ipaddrs

---

I will think the quoted statement with nginx and nginx-ssl instead of luci-nginx respective luci-nginx-ssl.

I did mean nginx and nginx-ssl. I think I failed to make it clear how that works, sorry about that.

With dynamic config, it's now possible to offer both http and https server conf files but automatically disables https if no certificate utils exist. That means we can just provide a single nginx package with conditional ssl, depending on the existence of openssl or px5g, which is controlled by the choice of installing luci-nginx or luci-nginx-ssl. Both the default http and https server should include the same location defs, so that they behave identically.

would other apps (like ariang and etebase) provide two variants, too?

Other apps, when dealing with the default server, should not care about http or https at all. They should just put locations in conf.d/default_server or dynconf.d/default_server.

I see the inequality justified by the idea that uhttpd is very lightweight whereas nginx handles advanced use cases

I think that's orthogonal to if redirecting http to https by default for the default server. The new design where independent http and https servers include identical locations can also support advanced use cases IMO.

Then I would suggest commenting the return values in all provided dyn.conf.d files

Fixed.

The generated cert files are not deleted, are they?

Sorry, I totally misread your previous descriptions. Yes, they are not deleted, but that shouldn't cause any problem? And I think it's actually desirable to keep the certificate, because reinstalling nginx-mod-luci-ssl makes LuCi use the same certificate.

We could comment the line include acme.conf;

Fixed.

---

Given the new design, I propose that we make nginx the main package, and make nginx-ssl a meta package depending on it to be backward compatible, and change luci-nginx and luci-nginx-ssl's dependencies accordingly. What do you think?

Since we're changing the package, I'd also like to take the opportunity to rename nginx-all-module. It at least should be named nginx-all-modules, but I'd like to rename it to just nginx-full. What do you think?

[hgl]

@peter-stadler @Ansuel Are you available for reviewing this PR?

[peter-stadler]

It took me some time (and I fear this will be the case for future comments, too).

I think for now, we should focus on the structure. The code changes can help with it, but unless you want to, there is no need to adjust it every time.

---

I have two major points:

1. I am affraid the dynconf mechanism will not work as intended. The exit values are not passed through from $(...). How are you testing it?

2. I thought a bit about the default_server:

• We could extend the Config_ssl.in by:

config NGINX_DEFAULT_SERVER
	bool
	prompt "Install a default server"
	help
		TODO.
	default y

• Is it worth to maintain a HTTP only configuration? Then I suggest something like:

config NGINX_DEFAULT_SERVER_HTTPS
	bool
	prompt "Secure the default server with HTTPS"
	depends on NGINX_DEFAULT_SERVER
	help
		TODO.
	default y

• The configuration should not change by installing another package (luci-nginx-ssl)
• We should redirect from HTTP to HTTPS whenever possible; I think it is better not to include identical locations in the HTTP and HTTPS server (although it would be possible, but it entices using the insecure channel).
• I would classify creating the ssl directives as one-time setup not as dynamic configuration, which can change with each run (or even during it).
• What about renewing the certificate?

---

And some minor comments:

Yes, they are not deleted, but that shouldn't cause any problem? And I think it's actually desirable to keep the certificate, because reinstalling nginx-mod-luci-ssl makes LuCi use the same certificate.

It is not a problem, more a different view: I think it is more common to try something out, i.e. install and uninstall it, than to reinstall it (except upgrading), where you can reuse them. In the first case it is desirable to remove the created files.

I thought about commenting the exit values of the *.dynconf files just below the . /usr/lib/nginx/functions.sh ...

Is there a use case to create dynconf.d/*/*.dynconf files (not only dynconf.d/*.dynconf)? It is cumbersome to include /var/etc/nginx/conf.d/example.com/*.conf; beside include conf.d/example.com/*.conf; especially as you stated previously:

[...] users aren't supposed to know the real path [...]

Maybe we should think about putting the *.dynconf files in the conf.d folder structure (as they have their own extension).

In the wan.dynconf.example it is better to comment out include acme.conf;, such that it is not used without an acme client.

I see no advantage in merely making the options for the self-signed certificate configurable via uci.

I have no preferences for the renaming, just keep in mind that users will not get upgrades if a package name is dropped ...

Have you thought about:

As this means to transition the other packages (luci, etebase and ariang), should we add there both ways for some time: conf.d/app.locations symlinked to conf.d/default_server/app.conf?

Or should we have include conf.d/*.locations; here (for some time)?

[peter-stadler]

P.S.: The following should only be included if CONFIG_NGINX_UBUS=y:

location /ubus {
	ubus_interpreter;
	ubus_socket_path /var/run/ubus/ubus.sock;
	ubus_parallel_req 2;
}

[hgl]

I think for now, we should focus on the structure.

Good idea, since the design seems still in flux, but I couldn't resist implementing a suggestion you made. :)

The exit values are not passed through from $(...).

Functions use exit not return, so the script should immediately exit with the status?

We could extend the Config_ssl.in by:

The following should only be included if CONFIG_NGINX_UBUS=y:

I'm not sure this works, since they're really a dependency of the package? For example, what should happen, if a user installs nginx-mod-luci, but with NGINX_DEFAULT_SERVER or CONFIG_NGINX_UBUS turned off?

The configuration should not change by installing another package (luci-nginx-ssl)

So if NGINX_DEFAULT_SERVER_HTTPS is off, does that mean luci-nginx-ssl now does nothing, since http is always supported? This doesn't sound right.

We should redirect from HTTP to HTTPS whenever possible

Peter, I'm with you on this one from strictly security point of view. My only gripe is that I think the nginx package is in no position to implement it BY DEFAULT, since the official httpd (uhttpd) package doesn't take approach. I don't think Nginx being more complex or heavyweight has anything to do with if it should take a different approach on how to support https.

Redirecting HTTP to HTTPS by default can lead to some deeper level change. For example, if uhttpd is to redirect HTTP to HTTPS by default, then that means the luci package supports HTTPS out of the box, which in turn mean either px5g or openssl-utils should be supported out of the box, and luci-ssl should be deprecated, etc. This should impact how other httpd should be packaged.

IMHO we should either make that deeper change or follow the convention set by uhttpd.

Users who are concerned about security implication could always add the redirection themselves.

Let me know if you can agree with me on this one.

In the first case it is desirable to remove the created files.

I'm fine with removing such file as long as they could be recreated identically, which is not the case with certificates. I don't think we should default to a behavior where user could lost files they could never get back. The alternative that a removed package leaves an unused certificate behind isn't as serious.

Maybe we should think about putting the *.dynconf files in the conf.d folder structure

I really like this suggestion and has changed the PR accordingly.

It is cumbersome to include /var/etc/nginx/conf.d/example.com/*.conf

I think it's should be fine, since for any /etc/nginx/conf.d/.conf, they usually only need to include the server of their concern, and not others, meaning usually only /etc/nginx/conf.d/example.com.conf would included /etc/nginx/conf.d/example.com/.conf.

In the wan.dynconf.example it is better to comment out include acme.conf;, such that it is not used without an acme client.

Will change.

I see no advantage in merely making the options for the self-signed certificate configurable via uci.

What do you suggest we do with self-signed certificates? They shouldn't be customizable or the options should come from elsewhere?

just keep in mind that users will not get upgrades if a package name is dropped

We will make the original package depend on the renamed package for backward compatibility.

Or should we have include conf.d/*.locations; here (for some time)?

This sounds like the easier approach, I think it's a good idea to make the default server include them, making the transition easier.

[hgl]

@peter-stadler Are you still available?

[peter-stadler]

It was a long delay, I will keep having some but little spare time.

Functions use exit not return, so the script should immediately exit with the status?

How did you test that it works as intended? I have problems that e.g. default_server.dynconf is not exiting with the same value if $(ngx_ssl_cert) fails.

I'm not sure this works, since they're really a dependency of the package? For example, what should happen, if a user installs nginx-mod-luci, but with NGINX_DEFAULT_SERVER or CONFIG_NGINX_UBUS turned off?

When you turn NGINX_DEFAULT_SERVER off, you are responsible to provide a server for luci to work at all.

When you turn CONFIG_NGINX_UBUS off, all should work beside the parts of luci that rely on ubus.
(The location /ubus part would make problems if nginx has no ubus support and luci is installed for it.)

So if NGINX_DEFAULT_SERVER_HTTPS is off, does that mean luci-nginx-ssl now does nothing, since http is always supported? This doesn't sound right.

We would need to remove luci-nginx-ssl and keep just luci-nginx modifying the description that the usage of HTTPS depends on nginx (it would be on if the configuration is not changed). I still prefer to just install a default server with SSL enabled.

Peter, I'm with you on this one from strictly security point of view. My only gripe is that I think the nginx package is in no position to implement it BY DEFAULT, since the official httpd (uhttpd) package doesn't take approach. I don't think Nginx being more complex or heavyweight has anything to do with if it should take a different approach on how to support https.

Redirecting HTTP to HTTPS by default can lead to some deeper level change. For example, if uhttpd is to redirect HTTP to HTTPS by default, then that means the luci package supports HTTPS out of the box, which in turn mean either px5g or openssl-utils should be supported out of the box, and luci-ssl should be deprecated, etc. This should impact how other httpd should be packaged.

IMHO we should either make that deeper change or follow the convention set by uhttpd.

Users who are concerned about security implication could always add the redirection themselves.

Let me know if you can agree with me on this one.

I thought a while about it and I still want to stick to https by default. I think it is justifiable that nginx and uhttpd behave different here; they have different use cases:

• nginx can serve different apps on different suburls (AFAIK uhttpd can serve one site on a given domain:port)
• uhttpd is very small (nginx is bigger and depends on libopenssl, the overhead of openssl-util is smaller)

But, if you still want to change it, you do not need to convince me: Ansuel is the maintainer, he can decide.

What do you suggest we do with self-signed certificates? They shouldn't be customizable or the options should come from elsewhere?

I would say the first: there is very little reason to deviate from secure default options.

[hgl]

I'll try to have a working package on my local device before I continue the discussion, to save us both some time.

Ansuel is the maintainer, he can decide.

@Ansuel could you comment on where the direction we are taking is good? It'd be a waste if the implement is shot down because the direction was wrong.

I accidentally deleted the branch, recreated it, but it seems I can't reopen the PR anymore.