jolheiser/nixpkgs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
c8f26afbbf
nixpkgs/nixos/tests/nginx-pubhtml.nix

22 lines
731 B
Nix
Raw Normal View History

nginx: Fix ETag patch to ignore realpath(3) error While our ETag patch works pretty fine if it comes to serving data off store paths, it unfortunately broke something that might be a bit more common, namely when using regexes to extract path components of location directives for example. Recently, @devhell has reported a bug with a nginx location directive like this: location ~^/\~([a-z0-9_]+)(/.*)?$" { alias /home/$1/public_html$2; } While this might look harmless at first glance, it does however cause issues with our ETag patch. The alias directive gets broken up by nginx like this: *2 http script copy: "/home/" *2 http script capture: "foo" *2 http script copy: "/public_html/" *2 http script capture: "bar.txt" In our patch however, we use realpath(3) to get the canonicalised path from ngx_http_core_loc_conf_s.root, which returns the *configured* value from the root or alias directive. So in the example above, realpath(3) boils down to the following syscalls: lstat("/home", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0 lstat("/home/$1", 0x7ffd08da6f60) = -1 ENOENT (No such file or directory) During my review[1] of the initial patch, I didn't actually notice that what we're doing here is returning NGX_ERROR if the realpath(3) call fails, which in turn causes an HTTP 500 error. Since our patch actually made the canonicalisation (and thus additional syscalls) necessary, we really shouldn't introduce an additional error so let's - at least for now - silently skip return value if realpath(3) has failed. However since we're using the unaltered root from the config we have another issue, consider this root: /nix/store/...-abcde/$1 Calling realpath(3) on this path will fail (except if there's a file called "$1" of course), so even this fix is not enough because it results in the ETag not being set to the store path hash. While this is very ugly and we should fix this very soon, it's not as serious as getting HTTP 500 errors for serving static files. I added a small NixOS VM test, which uses the example above as a regression test. It seems that my memory is failing these days, since apparently I *knew* about this issue since digging for existing issues in nixpkgs, I found this similar pull request which I even reviewed: https://github.com/NixOS/nixpkgs/pull/66532 However, since the comments weren't addressed and the author hasn't responded to the pull request, I decided to keep this very commit and do a follow-up pull request. [1]: https://github.com/NixOS/nixpkgs/pull/48337 Signed-off-by: aszlig <aszlig@nix.build> Reported-by: @devhell Acked-by: @7c6f434c Acked-by: @yorickvP Merges: https://github.com/NixOS/nixpkgs/pull/80671 Fixes: https://github.com/NixOS/nixpkgs/pull/66532
2020-02-20 19:43:42 +00:00
import ./make-test-python.nix {
name = "nginx-pubhtml";
machine = { pkgs, ... }: {
nixos/tests: fix nginx-pubhtml test
2020-04-20 17:12:31 +00:00
systemd.services.nginx.serviceConfig.ProtectHome = "read-only";
nginx: Fix ETag patch to ignore realpath(3) error While our ETag patch works pretty fine if it comes to serving data off store paths, it unfortunately broke something that might be a bit more common, namely when using regexes to extract path components of location directives for example. Recently, @devhell has reported a bug with a nginx location directive like this: location ~^/\~([a-z0-9_]+)(/.*)?$" { alias /home/$1/public_html$2; } While this might look harmless at first glance, it does however cause issues with our ETag patch. The alias directive gets broken up by nginx like this: *2 http script copy: "/home/" *2 http script capture: "foo" *2 http script copy: "/public_html/" *2 http script capture: "bar.txt" In our patch however, we use realpath(3) to get the canonicalised path from ngx_http_core_loc_conf_s.root, which returns the *configured* value from the root or alias directive. So in the example above, realpath(3) boils down to the following syscalls: lstat("/home", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0 lstat("/home/$1", 0x7ffd08da6f60) = -1 ENOENT (No such file or directory) During my review[1] of the initial patch, I didn't actually notice that what we're doing here is returning NGX_ERROR if the realpath(3) call fails, which in turn causes an HTTP 500 error. Since our patch actually made the canonicalisation (and thus additional syscalls) necessary, we really shouldn't introduce an additional error so let's - at least for now - silently skip return value if realpath(3) has failed. However since we're using the unaltered root from the config we have another issue, consider this root: /nix/store/...-abcde/$1 Calling realpath(3) on this path will fail (except if there's a file called "$1" of course), so even this fix is not enough because it results in the ETag not being set to the store path hash. While this is very ugly and we should fix this very soon, it's not as serious as getting HTTP 500 errors for serving static files. I added a small NixOS VM test, which uses the example above as a regression test. It seems that my memory is failing these days, since apparently I *knew* about this issue since digging for existing issues in nixpkgs, I found this similar pull request which I even reviewed: https://github.com/NixOS/nixpkgs/pull/66532 However, since the comments weren't addressed and the author hasn't responded to the pull request, I decided to keep this very commit and do a follow-up pull request. [1]: https://github.com/NixOS/nixpkgs/pull/48337 Signed-off-by: aszlig <aszlig@nix.build> Reported-by: @devhell Acked-by: @7c6f434c Acked-by: @yorickvP Merges: https://github.com/NixOS/nixpkgs/pull/80671 Fixes: https://github.com/NixOS/nixpkgs/pull/66532
2020-02-20 19:43:42 +00:00
services.nginx.enable = true;
services.nginx.virtualHosts.localhost = {
locations."~ ^/\\~([a-z0-9_]+)(/.*)?$".alias = "/home/$1/public_html$2";
};
users.users.foo.isNormalUser = true;
};
testScript = ''
machine.wait_for_unit("nginx")
machine.wait_for_open_port(80)
machine.succeed("chmod 0711 /home/foo")
machine.succeed("su -c 'mkdir -p /home/foo/public_html' foo")
machine.succeed("su -c 'echo bar > /home/foo/public_html/bar.txt' foo")
machine.succeed('test "$(curl -fvvv http://localhost/~foo/bar.txt)" = bar')
'';
}
Reference in New Issue Copy Permalink