[PATCH] services: nginx: Harden php-location settings.

  • Done
  • quality assurance status badge
Details
3 participants
  • Jonathan Brielmaier
  • Tobias Geerinckx-Rice
  • Bruno Victal
Owner
unassigned
Submitted by
Bruno Victal
Severity
normal

Debbugs page

B
B
Bruno Victal wrote on 5 Apr 2023 08:34
(address . guix-patches@gnu.org)(name . Bruno Victal)(address . mirai@makinata.eu)
ad598ba1ce644a29c997139c82d790a3ac65f4b4.1680708757.git.mirai@makinata.eu
Incorporate advice from [2], which mitigates httpoxy[1] vulnerability and
disallows passing non-php files to the PHP backend.

note 4.

* gnu/services/web.scm (nginx-php-location): Only pass existing php files to
backend. Mitigate httpoxy vulnerability.
---

Tested with: make check-system TESTS="nginx php-fpm"

gnu/services/web.scm | 4 ++++
1 file changed, 4 insertions(+)

Toggle diff (19 lines)
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index d56e893527..f5ed027bb4 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -1123,6 +1123,10 @@ (define* (nginx-php-location
(uri "~ \\.php$")
(body (list
"fastcgi_split_path_info ^(.+\\.php)(/.+)$;"
+ ;; Mitigate https://httpoxy.org/ vulnerabilities
+ "fastcgi_param HTTP_PROXY \"\";"
+ ;; Only pass existing php files to the backend.
+ "if (!-f $document_root$fastcgi_script_name) { return 404; }"
(string-append "fastcgi_pass unix:" socket ";")
"fastcgi_index index.php;"
(list "include " nginx-package "/share/nginx/conf/fastcgi.conf;")))))

base-commit: 6311493d7a6271bfbc51f4693857f9a12fe9965d
--
2.39.2
J
J
Jonathan Brielmaier wrote on 5 Apr 2023 13:19
(address . 62678@debbugs.gnu.org)
068a52bd-8597-a449-c452-4c110f645ca0@web.de
I wonder if we should at least make the HTTP_PROXY variable
configurable. It may need to be set to something else then "" in some
scenarios. I don't know...
B
B
Bruno Victal wrote on 6 Apr 2023 06:11
(name . Jonathan Brielmaier)(address . jonathan.brielmaier@web.de)(address . 62678@debbugs.gnu.org)
65a26f2b-0ef5-b9ac-b4df-4e3b73ad4474@makinata.eu
Hi Jonathan,

On 2023-04-05 21:19, Jonathan Brielmaier wrote:
Toggle quote (4 lines)
> I wonder if we should at least make the HTTP_PROXY variable
> configurable. It may need to be set to something else then "" in some
> scenarios. I don't know...

No, there's no legitimate reason for this, since 'PROXY' is not
a standard HTTP header according to [1]. PROXY being passed to a cgi application
as HTTP_PROXY is what the exploit is about, since HTTP_PROXY is recognized as
a variable for configuring proxies (for curl, wget, etc.)
Allowing HTTP_PROXY to be set remotely (due to a confusion with the non-standard 'PROXY' header)
is simply incomprehensible.

Regarding user intent, that is, configuring the proxy used by the cgi application by
setting HTTP_PROXY via nginx?
I don't have this use-case but IMO it feels like an extreme poor design, since it's
exploiting a name confusion to change the system environment variables for the
cgi application.

If for some reason you really need this, you can always use the regular
nginx-location-configuration to manually craft a php-location.




Cheers,
Bruno
B
B
Bruno Victal wrote on 22 Jun 2023 06:33
control-msg
(name . control)(address . control@debbugs.gnu.org)
4e7e3d93-01e3-da95-fc0a-fc07dd7e0734@makinata.eu
close 58166
tag 62678 + security

quit
T
T
Tobias Geerinckx-Rice wrote on 7 Jul 2023 07:22
Re: [bug#62678] [PATCH] services: nginx: Harden php-location settings.
(name . Bruno Victal)(address . mirai@makinata.eu)(address . 62678-done@debbugs.gnu.org)
87wmzber45.fsf@nckx
Hi Bruno,

Bruno Victal 写道：
Toggle quote (9 lines)
> Incorporate advice from [2], which mitigates httpoxy[1]
> vulnerability and
> disallows passing non-php files to the PHP backend.
>
> [1]: <https://httpoxy.org/>
> [2]:
> <https://www.nginx.com/resources/wiki/start/topics/examples/phpfcgi/>,
> note 4.

This is a better comment than commit message. I made it so and
pushed your changes as commit
cbc14b3baea457cf2718b85f767d39ff3911ce91.

Thanks!

T G-R
-----BEGIN PGP SIGNATURE-----

iIMEARYKACsWIQT12iAyS4c9C3o4dnINsP+IT1VteQUCZKggOg0cbWVAdG9iaWFz
LmdyAAoJEA2w/4hPVW15N1gA/0p4sLULM7SdMhyD1XLP6pWJ/a4ZmKzdA1lJv98Y
fHE0APwKaqXe+r15WzkH9KX9gtsrvq2OnEfiGTYisoyzEpfRAA==
=EMsW
-----END PGP SIGNATURE-----

Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 62678@patchwise.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 62678
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch