nixos/nginx: make sslCertificate and sslCertificateKey nullable

[ncfavier]

Motivation for this change

It sometimes makes sense to have addSSL = true without defining sslCertificate or sslCertificateKey, for example if one wants to use the ssl_reject_handshake directive.

cc @globin @aanderse @Ma27

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[aanderse]

@ncfavier can you please share some example nixos configuration to demonstrate how you are using this?

[ncfavier]

@ncfavier can you please share some example nixos configuration to demonstrate how you are using this?

{
  services.nginx.virtualHosts = {
    "foo.example.com" = {
      forceSSL = true;
      enableACME = true;
      root = "/var/lib/www/foo";
    };

    default = {
      default = true;
      addSSL = true;
      extraConfig = ''
        ssl_reject_handshake on;
        return 444;
      '';
    };
  };
}

The scenario here is that I have a wildcard A record for *.example.com, but if someone tries to connect to bar.example.com, which I don't serve, over HTTPS, then they should get an SSL_ERROR_UNRECOGNIZED_NAME_ALERT error, rather than an SSL_ERROR_BAD_CERT_DOMAIN.

[aanderse]

That makes sense.

I don't like giving up that check, but I don't like nginx possible configurations not being possible in nixos even more.

I would love to hear some feedback from someone like @emilazy, @arianvp, @andir, etc...

[ncfavier]

We could also instead add a rejectSSL option that implements this, with an assertion on the nginx version (ssl_reject_handshake was introduced in 1.19.4 but the current stable is 1.18.0). Then the check would be something like hasSSL -> rejectSSL == true || (sslCertificate != null && sslCertificateKey != null).

It looks like this would match the internal check performed by nginx:

https://github.com/nginx/nginx/blob/eb52de83114e8d98722cd17ec8435c47956b6315/src/http/modules/ngx_http_ssl_module.c#L1366-L1374

[lukegb]

I prefer the suggestion of having an explicit rejectSSL option, I think, which would also help with discoverability next to the other settings - the addSSL/ssl_reject_handshake combo feels a bit odd to me.

[ncfavier]

I prefer the suggestion of having an explicit rejectSSL option, I think, which would also help with discoverability next to the other settings - the addSSL/ssl_reject_handshake combo feels a bit odd to me.

Opened #119186 as an alternative to this PR.