diff --git a/docs/source/deploy.rst b/docs/source/deploy.rst index 6871421a..c6bdbe11 100644 --- a/docs/source/deploy.rst +++ b/docs/source/deploy.rst @@ -246,20 +246,24 @@ to the newly created unix socket: After=network.target [Service] + # gunicorn can let systemd know when it is ready Type=notify + NotifyAccess=main # the specific user that our service will run as User=someuser Group=someuser - # another option for an even more restricted service is - # DynamicUser=yes - # see http://0pointer.net/blog/dynamic-users-with-systemd.html + # this user can be transiently created by systemd + # DynamicUser=true RuntimeDirectory=gunicorn + WorkingDirectory=~ WorkingDirectory=/home/someuser/applicationroot ExecStart=/usr/bin/gunicorn applicationname.wsgi ExecReload=/bin/kill -s HUP $MAINPID KillMode=mixed TimeoutStopSec=5 PrivateTmp=true + # if your app does not need administrative capabilities, let systemd know + # ProtectSystem=strict [Install] WantedBy=multi-user.target @@ -272,11 +276,12 @@ to the newly created unix socket: [Socket] ListenStream=/run/gunicorn.sock # Our service won't need permissions for the socket, since it - # inherits the file descriptor by socket activation - # only the nginx daemon will need access to the socket + # inherits the file descriptor by socket activation. + # Only the nginx daemon will need access to the socket: SocketUser=www-data - # Optionally restrict the socket permissions even more. - # SocketMode=600 + SocketGroup=www-data + # Once the user/group is correct, restrict the permissions: + SocketMode=0660 [Install] WantedBy=sockets.target diff --git a/docs/source/faq.rst b/docs/source/faq.rst index 64b44c90..10572c1d 100644 --- a/docs/source/faq.rst +++ b/docs/source/faq.rst @@ -11,7 +11,9 @@ How do I set SCRIPT_NAME? ------------------------- By default ``SCRIPT_NAME`` is an empty string. The value could be set by -setting ``SCRIPT_NAME`` in the environment or as an HTTP header. +setting ``SCRIPT_NAME`` in the environment or as an HTTP header. Note that +this headers contains and underscore, so it is only accepted from trusted +forwarders listed in the ``forwarded-allow-ips`` setting. Server Stuff diff --git a/docs/source/news.rst b/docs/source/news.rst index 431a01b2..c28f6d9c 100644 --- a/docs/source/news.rst +++ b/docs/source/news.rst @@ -5,20 +5,27 @@ Changelog 23.0.0 - unreleased =================== -* minor docs fixes (:pr:`3217`, :pr:`3089`, :pr:`3167`) -* worker_class parameter accepts a class (:pr:`3079`) -* fix deadlock if request terminated during chunked parsing (:pr:`2688`) -* permit receiving Transfer-Encodings: compress, deflate, gzip (:pr:`3261`) -* permit Transfer-Encoding headers specifying multiple encodings. note: no parameters, still (:pr:`3261`) -* sdist generation now explicitly excludes sphinx build folder (:pr:`3257`) -* decode bytes-typed status (as can be passed by gevent) as utf-8 instead of raising `TypeError` (:pr:`2336`) -* raise correct Exception when encounting invalid chunked requests (:pr:`3258`) +- minor docs fixes (:pr:`3217`, :pr:`3089`, :pr:`3167`) +- worker_class parameter accepts a class (:pr:`3079`) +- fix deadlock if request terminated during chunked parsing (:pr:`2688`) +- permit receiving Transfer-Encodings: compress, deflate, gzip (:pr:`3261`) +- permit Transfer-Encoding headers specifying multiple encodings. note: no parameters, still (:pr:`3261`) +- sdist generation now explicitly excludes sphinx build folder (:pr:`3257`) +- decode bytes-typed status (as can be passed by gevent) as utf-8 instead of raising `TypeError` (:pr:`2336`) +- raise correct Exception when encounting invalid chunked requests (:pr:`3258`) +- the SCRIPT_NAME header when received from allowed forwarders is no longer restricted for containing an underscore (:pr:`3192`) + +** NOTE ** + +- The SCRIPT_NAME change mitigates a regression that appeared first in the 22.0.0 release +- Review your ``forwarded-allow-ips`` setting if you are still not seeing the SCRIPT_NAME transmitted ** Breaking changes ** -* refuse requests where the uri field is empty (:pr:`3255`) -* refuse requests with invalid CR/LR/NUL in heade field values (:pr:`3253`) -* remove temporary `--tolerate-dangerous-framing` switch from 22.0 (:pr:`3260`) -* If any of the breaking changes affect you, be aware that now refused requests can post a security problem, especially so in setups involving request pipe-lining and/or proxies. + +- refuse requests where the uri field is empty (:pr:`3255`) +- refuse requests with invalid CR/LR/NUL in heade field values (:pr:`3253`) +- remove temporary `--tolerate-dangerous-framing` switch from 22.0 (:pr:`3260`) +- If any of the breaking changes affect you, be aware that now refused requests can post a security problem, especially so in setups involving request pipe-lining and/or proxies. 22.0.0 - 2024-04-17 =================== diff --git a/docs/source/settings.rst b/docs/source/settings.rst index 1bb90d55..67a473c5 100644 --- a/docs/source/settings.rst +++ b/docs/source/settings.rst @@ -1479,6 +1479,25 @@ Use with care and only if necessary. Deprecated; scheduled for removal in 24.0.0 .. versionadded:: 22.0.0 +.. _forwarder-headers: + +``forwarder_headers`` +~~~~~~~~~~~~~~~~~~~~~ + +**Command line:** ``--forwarder-headers`` + +**Default:** ``'SCRIPT_NAME'`` + +A list containing upper-case header field names that the front-end proxy +sets, to be used in WSGI environment. + +If headers named in this list are not present in the request, they will be ignored. + +This option can be used to transfer SCRIPT_NAME and REMOTE_USER. + +It is important that your front-end proxy configuration ensures that +the headers defined here can not be passed directly from the client. + .. _header-map: ``header_map`` @@ -1496,11 +1515,12 @@ the same environment variable will dangerously confuse applications as to which The safe default ``drop`` is to silently drop headers that cannot be unambiguously mapped. The value ``refuse`` will return an error if a request contains *any* such header. -The value ``dangerous`` matches the previous, not advisabble, behaviour of mapping different +The value ``dangerous`` matches the previous, not advisable, behaviour of mapping different header field names into the same environ name. -The (at this time, not configurable) header `SCRIPT_NAME` is permitted - without consulting this setting, if it is received from an allowed forwarder. +If the source IP is permitted by ``forwarded-allow-ips``, *and* the header name is +present in ``forwarder-headers``, the header is mapped into environment regardless of +the state of this setting. Use with care and only if necessary and after considering if your problem could instead be solved by specifically renaming or rewriting only the intended headers diff --git a/gunicorn/config.py b/gunicorn/config.py index 209d93d8..f047074b 100644 --- a/gunicorn/config.py +++ b/gunicorn/config.py @@ -9,6 +9,7 @@ import argparse import copy import grp import inspect +import ipaddress import os import pwd import re @@ -402,6 +403,17 @@ def validate_list_of_existing_files(val): return [validate_file_exists(v) for v in validate_list_string(val)] +def validate_string_to_addr_list(val): + val = validate_string_to_list(val) + + for addr in val: + if addr == "*": + continue + _vaid_ip = ipaddress.ip_address(addr) + + return val + + def validate_string_to_list(val): val = validate_string(val) @@ -1262,7 +1274,7 @@ class ForwardedAllowIPS(Setting): section = "Server Mechanics" cli = ["--forwarded-allow-ips"] meta = "STRING" - validator = validate_string_to_list + validator = validate_string_to_addr_list default = os.environ.get("FORWARDED_ALLOW_IPS", "127.0.0.1,::1") desc = """\ Front-end's IPs from which allowed to handle set secure headers. @@ -2348,6 +2360,26 @@ def validate_header_map_behaviour(val): raise ValueError("Invalid header map behaviour: %s" % val) +class ForwarderHeaders(Setting): + name = "forwarder_headers" + section = "Server Mechanics" + cli = ["--forwarder-headers"] + validator = validate_string_to_list + default = "SCRIPT_NAME" + desc = """\ + + A list containing upper-case header field names that the front-end proxy + sets, to be used in WSGI environment. + + If headers named in this list are not present in the request, they will be ignored. + + This option can be used to transfer SCRIPT_NAME and REMOTE_USER. + + It is important that your front-end proxy configuration ensures that + the headers defined here can not be passed directly from the client. + """ + + class HeaderMap(Setting): name = "header_map" section = "Server Mechanics" @@ -2363,11 +2395,12 @@ class HeaderMap(Setting): The safe default ``drop`` is to silently drop headers that cannot be unambiguously mapped. The value ``refuse`` will return an error if a request contains *any* such header. - The value ``dangerous`` matches the previous, not advisabble, behaviour of mapping different + The value ``dangerous`` matches the previous, not advisable, behaviour of mapping different header field names into the same environ name. - The (at this time, not configurable) header `SCRIPT_NAME` is permitted - without consulting this setting, if it is received from an allowed forwarder. + If the source IP is permitted by ``forwarded-allow-ips``, *and* the header name is + present in ``forwarder-headers``, the header is mapped into environment regardless of + the state of this setting. Use with care and only if necessary and after considering if your problem could instead be solved by specifically renaming or rewriting only the intended headers diff --git a/gunicorn/http/message.py b/gunicorn/http/message.py index d71ba0ab..d65aebe5 100644 --- a/gunicorn/http/message.py +++ b/gunicorn/http/message.py @@ -78,7 +78,7 @@ class Message(object): # handle scheme headers scheme_header = False secure_scheme_headers = {} - allowed_forwarder_headers = [] + forwarder_headers = [] if from_trailer: # nonsense. either a request is https from the beginning # .. or we are just behind a proxy who does not remove conflicting trailers @@ -87,7 +87,7 @@ class Message(object): not isinstance(self.peer_addr, tuple) or self.peer_addr[0] in cfg.forwarded_allow_ips): secure_scheme_headers = cfg.secure_scheme_headers - allowed_forwarder_headers = ["SCRIPT_NAME"] + forwarder_headers = cfg.forwarder_headers # Parse headers into key/value pairs paying attention # to continuation lines. @@ -146,7 +146,7 @@ class Message(object): # HTTP_X_FORWARDED_FOR = 2001:db8::ha:cc:ed,127.0.0.1,::1 # Only modify after fixing *ALL* header transformations; network to wsgi env if "_" in name: - if name in allowed_forwarder_headers: + if name in forwarder_headers: # This forwarder may override our environment pass elif self.cfg.header_map == "dangerous": diff --git a/tests/test_config.py b/tests/test_config.py index f0ab392c..9afb4e49 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -164,16 +164,32 @@ def test_str_validation(): pytest.raises(TypeError, c.set, "proc_name", 2) -def test_str_to_list_validation(): +def test_str_to_addr_list_validation(): c = config.Config() - assert c.forwarded_allow_ips == ["127.0.0.1"] - c.set("forwarded_allow_ips", "127.0.0.1,192.168.0.1") - assert c.forwarded_allow_ips == ["127.0.0.1", "192.168.0.1"] + assert c.forwarded_allow_ips == ["127.0.0.1", "::1"] + c.set("forwarded_allow_ips", "127.0.0.1,192.0.2.1") + assert c.forwarded_allow_ips == ["127.0.0.1", "192.0.2.1"] c.set("forwarded_allow_ips", "") assert c.forwarded_allow_ips == [] c.set("forwarded_allow_ips", None) assert c.forwarded_allow_ips == [] + # demand addresses are specified unambiguously pytest.raises(TypeError, c.set, "forwarded_allow_ips", 1) + # demand networks are specified unambiguously + pytest.raises(ValueError, c.set, "forwarded_allow_ips", "127.0.0") + # detect typos + pytest.raises(ValueError, c.set, "forwarded_allow_ips", "::f:") + + +def test_str_to_list(): + c = config.Config() + assert c.forwarder_headers == ["SCRIPT_NAME"] + c.set("forwarder_headers", "SCRIPT_NAME,REMOTE_USER") + assert c.forwarder_headers == ["SCRIPT_NAME", "REMOTE_USER"] + c.set("forwarder_headers", "") + assert c.forwarder_headers == [] + c.set("forwarder_headers", None) + assert c.forwarder_headers == [] def test_callable_validation():