search

NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.11k stars 14.15k forks source link

nginx: recommended-proxy-headers.conf is included both globally and when using proxy_pass #156956

Open bbigras opened 2 years ago

bbigras commented 2 years ago

Describe the bug

When using recommendedProxySettings, recommended-proxy-headers.conf in included in the "global" http block and in every location if proxy_pass is used.

Isn't the location include redundant?

Also, since it's included in the http I cannot figure out how to override one header. I want to override X-Forwarded-Proto. I should be able to add a "manual" location using extraConfig but since the same header is set in the http block, it's only appended and causing problems. I would have to disable recommendedProxySettings, which I don't want to do.

Steps To Reproduce

Expected behavior

Screenshots

Additional context

Notify maintainers

@fpletz

Metadata

kevincox commented 2 years ago

I ran into this too. I wrote https://github.com/NixOS/nixpkgs/commit/29de7bb539ac09d8ca568bf4dcd9342156e24fd0 before realizing that the situation was more messed up than that.

Basically the settings are applied both globally and for proxy-passing locations. This is both pointless (the specific locations are already covered by the global config) but also means that the per-location opt-out is basically useless. It can only be used as an opt-in. And if it is used as an opt-in half of the settings don't get applied because some only get applied globally.

So we should clarify what the global recommendedProxySettings means.

  1. If it applies to all proxy configs, even those not using the nixos module's proxyPass then we need to set it globally. In that case setting recommendedProxySettings=false in a location block probably needs to undo that work.
  2. If it only applies to the proxyPass from the module then we should probably put *all of the configs into the include file and remove the global setting.
kevincox commented 2 years ago

The workaround to opt-out is something like this:

locations."/" = {
    recommendedProxySettings = false;
    extraConfig = ''
        proxy_set_header X-Forwarded-For "";
    '';
};

Of course be sure to revert every setting that you care about.

bmwagner18 commented 3 months ago

Hi all, I ran into a similar issue today. My transmission service doesn't like the recommended proxy settings so I tried to override them with the following:

services = {
    nginx = {
      recommendedProxySettings = true;
      recommendedTlsSettings = true;
      recommendedOptimisation = true;
      recommendedGzipSettings = true;
      virtualHosts = {
        "transmission.example.com" = {
          locations."/" = {
            proxyPass = "http://127.0.0.1:9091";
            recommendedProxySettings = false;
          };
          forceSSL = true;
          enableACME = true;
          # Use DNS Challenge.
          acmeRoot = null;
        }

Transmission's web UI loads as expected when the global recommendedProxySettings is set to false. I would expect that when I set recommendedProxySettings to false in the per site config that it would revert those back but that does not appear to be the case. I believe this is the behavior you mentioned in item 1 on your last post, @kevincox

fpletz commented 3 months ago

Basically the settings are applied both globally and for proxy-passing locations. This is both pointless (the specific locations are already covered by the global config) but also means that the per-location opt-out is basically useless. It can only be used as an opt-in. And if it is used as an opt-in half of the settings don't get applied because some only get applied globally.

No, if you use proxy_set_header in a location, all globally defined proxy_set_header settings are ignored. That is why there is the location-specific override and by default all recommended proxy_set_header settings are duplicated in the locations. Headers are being set globally to cover locations that not being generated by the module.

fpletz commented 3 months ago

I would expect that when I set recommendedProxySettings to false in the per site config that it would revert those back but that does not appear to be the case.

I agree that this is a pretty weird behavior if you don't know nginx but this is how nginx works after all. Not sure if we should change this or change the documentation. :slightly_frowning_face:

kevincox commented 3 months ago

I think the most obvious path is to make the global recommendedProxySettings just a default value for the per-location recommendedProxySettings. This is of course a breaking change if people are not using the NixOS module to define some of their configs. We can either just accept this and put it in the release notes or rename the global to something like defaultRecommendedProxySettings and then users need to take manual action and understand the change.

bmwagner18 commented 3 months ago

In case someone else stumbles into this issue when diagnosing a similar problem, or if we want to add it to the documentation, this is the required config to undo the recommendedProxySettings as per the nginx documentation.

services = {
    nginx = {
      recommendedProxySettings = true;
      recommendedTlsSettings = true;
      recommendedOptimisation = true;
      recommendedGzipSettings = true;
      virtualHosts = {
        "transmission.example.com" = {
          locations."/" = {
            proxyPass = "http://127.0.0.1:9091";
            # Undo the "recommendedProxySettings"
            extraConfig = ''
                    proxy_set_header Host $proxy_host;
                    proxy_set_header Connection close;
                    proxy_set_header X-Real-IP "";
                    proxy_set_header X-Forwarded-For "";
                    proxy_set_header X-Forwarded-Proto "";
                    proxy_set_header X-Forwarded-Host "";
                    proxy_set_header X-Forwarded-Server "";
            '';
          };
          forceSSL = true;
          enableACME = true;
          # Use DNS Challenge.
          acmeRoot = null;
};