nixos/nginx: validate config at build time

[symphorien]

Description of changes

Shamelessly stolen from nixcloud-webservices:
https://github.com/nixcloud/nixcloud-webservices/blob/master/modules/web/webserver/lib/nginx_check_config.nix

The nixos test testing the behavior of nginx in case of faulty config
would not build with this change (on purpose), so I modified it so that
the failure is not syntactic.

cc @qknight
cc nginx maintainers @thoughtpolice @fpletz @globin @ajs124 @7c6f434c

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.05 Release Notes (or backporting 22.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[winterqt]

@ofborg test nginx

[ajs124]

Aren't we already sort of doing this here?

nixpkgs/nixos/modules/services/web-servers/nginx/default.nix

Line 93 in 26a411b

configFile = pkgs.writers.writeNginxConfig "nginx.conf" ''

[symphorien]

This runs gixy, which is a linter for some specific kinds of semantic issues, but not syntax errors. Note that this change caught a (deliberate) syntax error in the nginx nixos test that was not caught by gixy.

[symphorien]

Will self-merge in a few days if nobody opposes.

[delroth]

This breaks in presence of backend DNS names in nginx configuration, since the sandbox can't access DNS:

2022/12/22 04:39:50 [emerg] 7#7: host not found in upstream "xxx.yyy.zzz" in /build/conf:793

proxy_pass https://xxx.yyy.zzz:443;

[RaitoBezarius]

This breaks in presence of backend DNS names in nginx configuration, since the sandbox can't access DNS:

2022/12/22 04:39:50 [emerg] 7#7: host not found in upstream "xxx.yyy.zzz" in /build/conf:793

proxy_pass https://xxx.yyy.zzz:443;

Do you think it's a dealbreaker even if services.nginx.validateConfig = false; would disable this feature?

[symphorien]

Could you share more of the config? I'm trying to reproduce with the nginx nixos test:

diff --git a/nixos/tests/nginx.nix b/nixos/tests/nginx.nix
index d9d073822a1..1b06759c162 100644
--- a/nixos/tests/nginx.nix
+++ b/nixos/tests/nginx.nix
@@ -13,6 +13,7 @@ import ./make-test-python.nix ({ pkgs, ... }: {
   nodes = {
     webserver = { pkgs, lib, ... }: {
       services.nginx.enable = true;
+      services.nginx.upstreams.yay.servers."1.2.3.4:3000" = {};
       services.nginx.commonHttpConfig = ''
         log_format ceeformat '@cee: {"status":"$status",'
           '"request_time":$request_time,'
@@ -57,6 +58,7 @@ import ./make-test-python.nix ({ pkgs, ... }: {
 
       specialisation.reloadRestartSystem.configuration = {
         services.nginx.package = pkgs.nginxMainline;
+        services.nginx.virtualHosts."yay.com".locations."/".proxyPass = "https://1.2.3.4:443";
       };
 
       specialisation.reloadWithErrorsSystem.configuration = {

but it still builds.

[RaitoBezarius]

Could you share more of the config? I'm trying to reproduce with the nginx nixos test:

diff --git a/nixos/tests/nginx.nix b/nixos/tests/nginx.nix
index d9d073822a1..1b06759c162 100644
--- a/nixos/tests/nginx.nix
+++ b/nixos/tests/nginx.nix
@@ -13,6 +13,7 @@ import ./make-test-python.nix ({ pkgs, ... }: {
   nodes = {
     webserver = { pkgs, lib, ... }: {
       services.nginx.enable = true;
+      services.nginx.upstreams.yay.servers."1.2.3.4:3000" = {};
       services.nginx.commonHttpConfig = ''
         log_format ceeformat '@cee: {"status":"$status",'
           '"request_time":$request_time,'
@@ -57,6 +58,7 @@ import ./make-test-python.nix ({ pkgs, ... }: {
 
       specialisation.reloadRestartSystem.configuration = {
         services.nginx.package = pkgs.nginxMainline;
+        services.nginx.virtualHosts."yay.com".locations."/".proxyPass = "https://1.2.3.4:443";
       };
 
       specialisation.reloadWithErrorsSystem.configuration = {

but it still builds.

You need DNS in your proxyPass, try something like https://example.com:443 and maybe some DNSMasq pointing example.com to 1.2.3.4.

[symphorien]

diff --git a/nixos/tests/nginx.nix b/nixos/tests/nginx.nix
index d9d073822a1..4172f3ea365 100644
--- a/nixos/tests/nginx.nix
+++ b/nixos/tests/nginx.nix
@@ -13,6 +13,7 @@ import ./make-test-python.nix ({ pkgs, ... }: {
   nodes = {
     webserver = { pkgs, lib, ... }: {
       services.nginx.enable = true;
+      services.nginx.upstreams.yay.servers."example.com:3000" = {};
       services.nginx.commonHttpConfig = ''
         log_format ceeformat '@cee: {"status":"$status",'
           '"request_time":$request_time,'
@@ -57,6 +58,7 @@ import ./make-test-python.nix ({ pkgs, ... }: {
 
       specialisation.reloadRestartSystem.configuration = {
         services.nginx.package = pkgs.nginxMainline;
+        services.nginx.virtualHosts."yay.com".locations."/".proxyPass = "https://some.dns.info:443";
       };
 
       specialisation.reloadWithErrorsSystem.configuration = {

also builds sucessfully.

[delroth]

diff --git a/nixos/tests/nginx.nix b/nixos/tests/nginx.nix
index 73f1133bd6c..b1010ac1f7d 100644
--- a/nixos/tests/nginx.nix
+++ b/nixos/tests/nginx.nix
@@ -30,6 +30,7 @@ import ./make-test-python.nix ({ pkgs, ... }: {
         extraConfig = ''
           access_log syslog:server=unix:/dev/log,facility=user,tag=mytag,severity=info ceeformat;
           location /favicon.ico { allow all; access_log off; log_not_found off; }
+          location / { proxy_pass https://foo.bar.com:443; }
         '';
       };

nginx config validation failed.
config was /nix/store/mkhs81aygpwhhc44q6iispfy9lvsdswa-nginx.conf.
in case of false positive, set `services.nginx.validateConfig` to false.
nginx output:
nginx: [alert] could not open error log file: open() "/var/log/nginx/error.log" failed (2: No such file or directory)
2022/12/22 22:07:17 [emerg] 7#7: host not found in upstream "foo.bar.com" in /build/conf:44
nginx: configuration file /build/conf test failed

I've disabled the config validation for my machine config for now, but I don't have a strong opinion on how important this regression is -- the module maintainers might.

[symphorien]

forget what I said, I was not trying on a recent enough revision, it did not contain this PR yet...

I can reproduce, thanks

[winterqt]

Perhaps we just disable this by default, or document that one should disable it in this case?

[symphorien]

I propose a fix in #207413

[Mic92]

Even with #207413 there are still other issues if your nginx has secrets:

validated-nginx.conf> nginx: configuration file /etc/nginx.conf test failed
error: build of '/nix/store/95x32m0621fxbpkwn21z5h8zw85k7d1x-validated-nginx.conf.drv' on 'ssh-ng://nix@ryan.dse.in.tum.de' failed: builder for '/nix/store/95x32m0621fxbpkwn21z5h8zw85k7d1x-validated-nginx.conf.drv' failed with exit code 1;
       last 8 log lines:
       > nginx config validation failed.
       > config was /nix/store/p68y009kly52wrkamjwsv58msp6cy980-nginx.conf.
       > in case of false positive, set `services.nginx.validateConfig` to false.
       > nginx output:
       > /nix/store/mgn3zckncarpmswhv9vn57i43f0rn2jr-nginxWithResolver: line 3: cannot redirect standard input from /dev/null: No such file or directory
       > 8::1            foo.bar
       > nginx: [emerg] BIO_new_file("/var/lib/dhparams/nginx.pem") failed (SSL: error:80000002:system library::No such file or directory:calling fopen(/var/lib/dhparams/nginx.pem, r) error:10000080:BIO routines::no such file)
       > nginx: configuration file /etc/nginx.conf test failed
       For full logs, run 'nix log /nix/store/95x32m0621fxbpkwn21z5h8zw85k7d1x-validated-nginx.conf.drv'.

error: build of '/nix/store/v8r4s5rkc6l2q1gqxfgi3j6drwky1bk3-validated-nginx.conf.drv' on 'ssh-ng://nix@ryan.dse.in.tum.de' failed: builder for '/nix/store/v8r4s5rkc6l2q1gqxfgi3j6drwky1bk3-validated-nginx.conf.drv' failed with exit code 1;
       last 8 log lines:
       > nginx config validation failed.
       > config was /nix/store/9kls1g005iry35f8d91irjh73nr5xyv9-nginx.conf.
       > in case of false positive, set `services.nginx.validateConfig` to false.
       > nginx output:
       > /nix/store/mgn3zckncarpmswhv9vn57i43f0rn2jr-nginxWithResolver: line 3: cannot redirect standard input from /dev/null: No such file or directory
       > 8::1            foo.bar
       > nginx: [emerg] open() "/run/secrets/nginx-secure-link" failed (2: No such file or directory) in /etc/nginx.conf:522
       > nginx: configuration file /etc/nginx.conf test failed
       For full logs, run 'nix log /nix/store/v8r4s5rkc6l2q1gqxfgi3j6drwky1bk3-validated-nginx.conf.drv'.

I would suggest to disable validation by default until we have a baseline.

[Mic92]

Please merge #207532 for now so that we can review potential fixes to this problem without hurry.

[SuperSandro2000]

This should be pkgs.stdenv.buildPlatform.canExecute stdenv.hostPlatform

[SuperSandro2000]

Not sure if we want to use runCommandLocal here. We probably don't want to cache or substitute this, or do we?

[symphorien]

this certificate is only used during build, so reusing it or sharing it via a binary cache is not a problem

[SuperSandro2000]

I am not sure how robust the regex is. For using the nix options it is certainly enough.