From ecb06398e741b754fa84e22257c24c81ef31194b Mon Sep 17 00:00:00 2001 From: Guillaume Girol Date: Wed, 4 Jan 2023 20:15:25 +0100 Subject: [PATCH 1/3] Revert "nixos/nginx: disable configuration validation for now" This reverts commit 7ef58bce9d2e261ecfd4f0b5c1fb4a7da6888dc0. --- nixos/modules/services/web-servers/nginx/default.nix | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/nixos/modules/services/web-servers/nginx/default.nix b/nixos/modules/services/web-servers/nginx/default.nix index 6fafae8928a6..b2dd58180f1d 100644 --- a/nixos/modules/services/web-servers/nginx/default.nix +++ b/nixos/modules/services/web-servers/nginx/default.nix @@ -581,9 +581,7 @@ in }; validateConfig = mkOption { - # FIXME: re-enable if we can make of the configurations work. - #default = pkgs.stdenv.hostPlatform == pkgs.stdenv.buildPlatform; - default = false; + default = pkgs.stdenv.hostPlatform == pkgs.stdenv.buildPlatform; defaultText = literalExpression "pkgs.stdenv.hostPlatform == pkgs.stdenv.buildPlatform"; type = types.bool; description = lib.mdDoc '' From aa4780077a715970541db2e382e5a108de51098e Mon Sep 17 00:00:00 2001 From: Guillaume Girol Date: Wed, 4 Jan 2023 20:15:57 +0100 Subject: [PATCH 2/3] Revert "nixos: add release notes for nginx config validation" This reverts commit 26a411b2cb177f88856dfbd1c85367a7647d4d61. --- .../manual/from_md/release-notes/rl-2305.section.xml | 10 ---------- nixos/doc/manual/release-notes/rl-2305.section.md | 2 -- 2 files changed, 12 deletions(-) diff --git a/nixos/doc/manual/from_md/release-notes/rl-2305.section.xml b/nixos/doc/manual/from_md/release-notes/rl-2305.section.xml index 82e28b913a19..0424d269ac7c 100644 --- a/nixos/doc/manual/from_md/release-notes/rl-2305.section.xml +++ b/nixos/doc/manual/from_md/release-notes/rl-2305.section.xml @@ -248,16 +248,6 @@ llvmPackages_rocm.clang-unwrapped. - - - The Nginx module now validates the syntax of config files at - build time. For more complex configurations (using - include with out-of-store files notably) - you may need to disable this check by setting - services.nginx.validateConfig - to false. - - The EC2 image module previously detected and automatically diff --git a/nixos/doc/manual/release-notes/rl-2305.section.md b/nixos/doc/manual/release-notes/rl-2305.section.md index b01d372c2dcb..2fe557949b05 100644 --- a/nixos/doc/manual/release-notes/rl-2305.section.md +++ b/nixos/doc/manual/release-notes/rl-2305.section.md @@ -67,8 +67,6 @@ In addition to numerous new and upgraded packages, this release has the followin - `llvmPackages_rocm.llvm` will not contain `clang` or `compiler-rt`. `llvmPackages_rocm.clang` will not contain `llvm`. `llvmPackages_rocm.clangNoCompilerRt` has been removed in favor of using `llvmPackages_rocm.clang-unwrapped`. -- The Nginx module now validates the syntax of config files at build time. For more complex configurations (using `include` with out-of-store files notably) you may need to disable this check by setting [services.nginx.validateConfig](#opt-services.nginx.validateConfig) to `false`. - - The EC2 image module previously detected and automatically mounted ext3-formatted instance store devices and partitions in stage-1 (initramfs), storing `/tmp` on the first discovered device. This behaviour, which only catered to very specific use cases and could not be disabled, has been removed. Users relying on this should provide their own implementation, and probably use ext4 and perform the mount in stage-2. - The EC2 image module previously detected and activated swap-formatted instance store devices and partitions in stage-1 (initramfs). This behaviour has been removed. Users relying on this should provide their own implementation. From cb73862665c040d89fbb4a66210ead376f69e468 Mon Sep 17 00:00:00 2001 From: Guillaume Girol Date: Wed, 4 Jan 2023 20:16:10 +0100 Subject: [PATCH 3/3] Revert "nixos/nginx: validate syntax of config file at build time" This reverts commit a768871934dd263423fca2979cedd7f5e8c7379d. This is too fragile, it breaks at least on: * ssl dh params * hostnames in proxypass and upstreams are resolved in the sandbox --- .../services/web-servers/nginx/default.nix | 47 ++----------------- nixos/tests/nginx.nix | 2 +- 2 files changed, 4 insertions(+), 45 deletions(-) diff --git a/nixos/modules/services/web-servers/nginx/default.nix b/nixos/modules/services/web-servers/nginx/default.nix index b2dd58180f1d..c723b962c847 100644 --- a/nixos/modules/services/web-servers/nginx/default.nix +++ b/nixos/modules/services/web-servers/nginx/default.nix @@ -288,7 +288,7 @@ let configPath = if cfg.enableReload then "/etc/nginx/nginx.conf" - else finalConfigFile; + else configFile; execCommand = "${cfg.package}/bin/nginx -c '${configPath}'"; @@ -440,38 +440,6 @@ let ); mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix; - - snakeOilCert = pkgs.runCommand "nginx-config-validate-cert" { nativeBuildInputs = [ pkgs.openssl.bin ]; } '' - mkdir $out - openssl genrsa -des3 -passout pass:xxxxx -out server.pass.key 2048 - openssl rsa -passin pass:xxxxx -in server.pass.key -out $out/server.key - openssl req -new -key $out/server.key -out server.csr \ - -subj "/C=UK/ST=Warwickshire/L=Leamington/O=OrgName/OU=IT Department/CN=example.com" - openssl x509 -req -days 1 -in server.csr -signkey $out/server.key -out $out/server.crt - ''; - validatedConfigFile = pkgs.runCommand "validated-nginx.conf" { nativeBuildInputs = [ cfg.package ]; } '' - # nginx absolutely wants to read the certificates even when told to only validate config, so let's provide fake certs - sed ${configFile} \ - -e "s|ssl_certificate .*;|ssl_certificate ${snakeOilCert}/server.crt;|g" \ - -e "s|ssl_trusted_certificate .*;|ssl_trusted_certificate ${snakeOilCert}/server.crt;|g" \ - -e "s|ssl_certificate_key .*;|ssl_certificate_key ${snakeOilCert}/server.key;|g" \ - > conf - - LD_PRELOAD=${pkgs.libredirect}/lib/libredirect.so \ - NIX_REDIRECTS="/etc/resolv.conf=/dev/null" \ - nginx -t -c $(readlink -f ./conf) > out 2>&1 || true - if ! grep -q "syntax is ok" out; then - echo nginx config validation failed. - echo config was ${configFile}. - echo 'in case of false positive, set `services.nginx.validateConfig` to false.' - echo nginx output: - cat out - exit 1 - fi - cp ${configFile} $out - ''; - - finalConfigFile = if cfg.validateConfig then validatedConfigFile else configFile; in { @@ -580,15 +548,6 @@ in ''; }; - validateConfig = mkOption { - default = pkgs.stdenv.hostPlatform == pkgs.stdenv.buildPlatform; - defaultText = literalExpression "pkgs.stdenv.hostPlatform == pkgs.stdenv.buildPlatform"; - type = types.bool; - description = lib.mdDoc '' - Validate the generated nginx config at build time. The check is not very robust and can be disabled in case of false positives. This is notably the case when cross-compiling or when using `include` with files outside of the store. - ''; - }; - additionalModules = mkOption { default = []; type = types.listOf (types.attrsOf types.anything); @@ -1126,7 +1085,7 @@ in }; environment.etc."nginx/nginx.conf" = mkIf cfg.enableReload { - source = finalConfigFile; + source = configFile; }; # This service waits for all certificates to be available @@ -1145,7 +1104,7 @@ in # certs are updated _after_ config has been reloaded. before = sslTargets; after = sslServices; - restartTriggers = optionals cfg.enableReload [ finalConfigFile ]; + restartTriggers = optionals cfg.enableReload [ configFile ]; # Block reloading if not all certs exist yet. # Happens when config changes add new vhosts/certs. unitConfig.ConditionPathExists = optionals (sslServices != []) (map (certName: certs.${certName}.directory + "/fullchain.pem") dependentCertNames); diff --git a/nixos/tests/nginx.nix b/nixos/tests/nginx.nix index 73f1133bd6ca..d9d073822a14 100644 --- a/nixos/tests/nginx.nix +++ b/nixos/tests/nginx.nix @@ -61,7 +61,7 @@ import ./make-test-python.nix ({ pkgs, ... }: { specialisation.reloadWithErrorsSystem.configuration = { services.nginx.package = pkgs.nginxMainline; - services.nginx.virtualHosts."hello".extraConfig = "access_log /does/not/exist.log;"; + services.nginx.virtualHosts."!@$$(#*%".locations."~@#*$*!)".proxyPass = ";;;"; }; }; };