Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nginx option defaultListen doesn't consider unix socket paths #370905

Open
Bert-Proesmans opened this issue Jan 4, 2025 · 3 comments
Open

Nginx option defaultListen doesn't consider unix socket paths #370905

Bert-Proesmans opened this issue Jan 4, 2025 · 3 comments
Labels
0.kind: bug Something is broken

Comments

@Bert-Proesmans
Copy link

Bert-Proesmans commented Jan 4, 2025

Describe the bug

The option services.nginx.defaultListen maps into a nginx http->server section for each defined virtual host, but this transform always sets a value on the port attribute. The mapping code assumes the addr attribute is always of type AF_INET, while AF_UNIX is also possible.

Steps To Reproduce

The nixos module stub below results in the nginx configuration stub below. Note that a port is appended to the unix socket segment.

services.nginx = {
  enable = true;
  defaultListen = [
    { addr = "unix:/run/nginx/virtualhosts.sock"; port = null; ssl = true; }
  ];

  virtualHosts = {
    "default" = {
      default = true;
      rejectSSL = true;
      locations."/".return = "404";
    };
  };
};
http {
server {
  listen unix:/run/nginx/virtualhosts.sock:443 ssl default_server ; # <-- port is added here
  server_name default ;
  ssl_reject_handshake on;
  location / {
    return 404;
  }
}
}

Expected behavior

The mapping code should not force a port if the addr has prefix unix:.
If port attribute remains at null, the port will not be written out to the nginx configuration, effectively omitting the ":443" from the above snippet

Additional context

Code in question is here (I think)

mkDefaultListenVhost = listenLines:
# If this vhost has SSL or is a SSL rejection host.
# We enable a TLS variant for lines without explicit ssl or ssl = true.
optionals (hasSSL || vhost.rejectSSL)
(map (listen: { port = cfg.defaultSSLListenPort; ssl = true; } // listen)
(filter (listen: !(listen ? ssl) || listen.ssl) listenLines))
# If this vhost is supposed to serve HTTP
# We provide listen lines for those without explicit ssl or ssl = false.
++ optionals (!onlySSL)
(map (listen: { port = cfg.defaultHTTPListenPort; ssl = false; } // listen)
(filter (listen: !(listen ? ssl) || !listen.ssl) listenLines));

Metadata

[bert-proesmans@development:~/nix]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 6.6.66, NixOS, 25.05 (Warbler), 25.05.20241219.d70bd19`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.24.11`
 - nixpkgs: `/nix/store/8fwsiv0hd7nw1brkvka0jf1frk3m7qkr-source`

Notify maintainers

No meta tag or maintainers reference, so falling back to;


Note for maintainers: Please tag this issue in your PR.


Add a 👍 reaction to issues you find important.

@Bert-Proesmans Bert-Proesmans added the 0.kind: bug Something is broken label Jan 4, 2025
@Bert-Proesmans
Copy link
Author

A workaround is setting the listen configuration manually on each virtual host attribute set.
As mentioned above, the nginx configuration printer already handles an unset port correctly.

services.nginx = {
  enable = true;
  defaultListen = [
    { addr = "unix:/run/nginx/virtualhosts.sock"; port = null; ssl = true; }
  ];

  virtualHosts = 
    let
      defaultListen = {
        listen = [
          { addr = "unix:/run/nginx/virtualhosts.sock"; port = null; ssl = true; }
        ];
      };
    in
    {
    "default" = {
      default = true;
      rejectSSL = true;
      locations."/".return = "404";
    } // defaultListen;
  };
};

@SuperSandro2000
Copy link
Member

Could you provide a fix PR for that?

@Bert-Proesmans
Copy link
Author

Sure, is something like the following ok?

optionals (hasSSL || vhost.rejectSSL) 
     (map (listen: { port = lib.mkIf !(lib.strings.hasPrefix "unix:" cfg.addr) cfg.defaultSSLListenPort; ssl = true; } // listen) 
     [..]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug Something is broken
Projects
None yet
Development

No branches or pull requests

2 participants