Merge pull request #3192 from pajod/patch-allowed-script-name

22.0.0 regression: We need a better default treatment of SCRIPT_NAME

docs/source/deploy.rst

@@ -246,13 +246,14 @@ 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
+    # this user can be transiently created by systemd
-    # DynamicUser=yes
+    # DynamicUser=true
-    # see http://0pointer.net/blog/dynamic-users-with-systemd.html
     RuntimeDirectory=gunicorn
     WorkingDirectory=/home/someuser/applicationroot
     ExecStart=/usr/bin/gunicorn applicationname.wsgi
@@ -260,6 +261,8 @@ to the newly created unix socket:
     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 +275,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
+    # inherits the file descriptor by socket activation.
-    # only the nginx daemon will need access to the socket
+    # Only the nginx daemon will need access to the socket:
     SocketUser=www-data
-    # Optionally restrict the socket permissions even more.
+    SocketGroup=www-data
-    # SocketMode=600
+    # Once the user/group is correct, restrict the permissions:
+    SocketMode=0660
 
     [Install]
     WantedBy=sockets.target

docs/source/faq.rst

@@ -11,8 +11,14 @@ 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 :ref:`forwarded-allow-ips` setting.
 
+.. note::
+
+   If your application should appear in a subfolder, your ``SCRIPT_NAME``
+   would typically start with single slash but contain no trailing slash.
 
 Server Stuff
 ============

docs/source/news.rst

@@ -5,20 +5,29 @@ Changelog
 23.0.0 - unreleased
 ===================
 
-* minor docs fixes (:pr:`3217`, :pr:`3089`, :pr:`3167`)
+- minor docs fixes (:pr:`3217`, :pr:`3089`, :pr:`3167`)
-* worker_class parameter accepts a class (:pr:`3079`)
+- worker_class parameter accepts a class (:pr:`3079`)
-* fix deadlock if request terminated during chunked parsing (:pr:`2688`)
+- fix deadlock if request terminated during chunked parsing (:pr:`2688`)
-* permit receiving Transfer-Encodings: compress, deflate, gzip (:pr:`3261`)
+- permit receiving Transfer-Encodings: compress, deflate, gzip (:pr:`3261`)
-* permit Transfer-Encoding headers specifying multiple encodings. note: no parameters, still (: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`)
+- 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`)
+- 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`)
+- raise correct Exception when encounting invalid chunked requests (:pr:`3258`)
+- the SCRIPT_NAME and PATH_INFO headers, when received from allowed forwarders, are no longer restricted for containing an underscore (:pr:`3192`)
+- include IPv6 loopback address ``[::1]`` in default for :ref:`forwarded-allow-ips` and :ref:`proxy-allow-ips` (:pr:`3192`)
+
+** NOTE **
+
+- The SCRIPT_NAME change mitigates a regression that appeared first in the 22.0.0 release
+- Review your :ref:`forwarded-allow-ips` setting if you are still not seeing the SCRIPT_NAME transmitted
+- Review your :ref:`forwarder-headers` setting if you are missing headers after upgrading from a version prior to 22.0.0
 
 ** 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`)
+- refuse requests where the uri field is empty (:pr:`3255`)
-* remove temporary `--tolerate-dangerous-framing` switch from 22.0 (:pr:`3260`)
+- refuse requests with invalid CR/LR/NUL in heade field values (:pr:`3253`)
-* 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.
+- 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
 ===================

docs/source/settings.rst

@@ -1208,7 +1208,7 @@ temporary directory.
 
 A dictionary containing headers and values that the front-end proxy
 uses to indicate HTTPS requests. If the source IP is permitted by
-``forwarded-allow-ips`` (below), *and* at least one request header matches
+:ref:`forwarded-allow-ips` (below), *and* at least one request header matches
 a key-value pair listed in this dictionary, then Gunicorn will set
 ``wsgi.url_scheme`` to ``https``, so your application can tell that the
 request is secure.
@@ -1232,17 +1232,23 @@ the headers defined here can not be passed directly from the client.
 
 **Command line:** ``--forwarded-allow-ips STRING``
 
-**Default:** ``'127.0.0.1'``
+**Default:** ``'127.0.0.1,::1'``
 
 Front-end's IPs from which allowed to handle set secure headers.
-(comma separate).
+(comma separated).
 
-Set to ``*`` to disable checking of Front-end IPs (useful for setups
+Set to ``*`` to disable checking of front-end IPs. This is useful for setups
-where you don't know in advance the IP address of Front-end, but
+where you don't know in advance the IP address of front-end, but
-you still trust the environment).
+instead have ensured via other means that only your
+authorized front-ends can access Gunicorn.
 
 By default, the value of the ``FORWARDED_ALLOW_IPS`` environment
-variable. If it is not defined, the default is ``"127.0.0.1"``.
+variable. If it is not defined, the default is ``"127.0.0.1,::1"``.
+
+.. note::
+
+    This option does not affect UNIX socket connections. Connections not associated with
+    an IP address are treated as allowed, unconditionally.
 
 .. note::
 
@@ -1369,13 +1375,19 @@ Example for stunnel config::
 
 **Command line:** ``--proxy-allow-from``
 
-**Default:** ``'127.0.0.1'``
+**Default:** ``'127.0.0.1,::1'``
 
-Front-end's IPs from which allowed accept proxy requests (comma separate).
+Front-end's IPs from which allowed accept proxy requests (comma separated).
 
-Set to ``*`` to disable checking of Front-end IPs (useful for setups
+Set to ``*`` to disable checking of front-end IPs. This is useful for setups
-where you don't know in advance the IP address of Front-end, but
+where you don't know in advance the IP address of front-end, but
-you still trust the environment)
+instead have ensured via other means that only your
+authorized front-ends can access Gunicorn.
+
+.. note::
+
+    This option does not affect UNIX socket connections. Connections not associated with
+    an IP address are treated as allowed, unconditionally.
 
 .. _raw-paste-global-conf:
 
@@ -1498,6 +1510,26 @@ 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,PATH_INFO'``
+
+A list containing upper-case header field names that the front-end proxy
+(see :ref:`forwarded-allow-ips`) sets, to be used in WSGI environment.
+
+This option has no effect for headers not present in the request.
+
+This option can be used to transfer ``SCRIPT_NAME``, ``PATH_INFO``
+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``
@@ -1515,9 +1547,13 @@ 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.
 
+If the source is permitted as explained in :ref:`forwarded-allow-ips`, *and* the header name is
+present in :ref:`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
 on a proxy in front of Gunicorn.

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)
 
@@ -1238,7 +1250,7 @@ class SecureSchemeHeader(Setting):
 
         A dictionary containing headers and values that the front-end proxy
         uses to indicate HTTPS requests. If the source IP is permitted by
-        ``forwarded-allow-ips`` (below), *and* at least one request header matches
+        :ref:`forwarded-allow-ips` (below), *and* at least one request header matches
         a key-value pair listed in this dictionary, then Gunicorn will set
         ``wsgi.url_scheme`` to ``https``, so your application can tell that the
         request is secure.
@@ -1262,18 +1274,24 @@ 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")
+    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.
-        (comma separate).
+        (comma separated).
 
-        Set to ``*`` to disable checking of Front-end IPs (useful for setups
+        Set to ``*`` to disable checking of front-end IPs. This is useful for setups
-        where you don't know in advance the IP address of Front-end, but
+        where you don't know in advance the IP address of front-end, but
-        you still trust the environment).
+        instead have ensured via other means that only your
+        authorized front-ends can access Gunicorn.
 
         By default, the value of the ``FORWARDED_ALLOW_IPS`` environment
-        variable. If it is not defined, the default is ``"127.0.0.1"``.
+        variable. If it is not defined, the default is ``"127.0.0.1,::1"``.
+
+        .. note::
+
+            This option does not affect UNIX socket connections. Connections not associated with
+            an IP address are treated as allowed, unconditionally.
 
         .. note::
 
@@ -2062,14 +2080,20 @@ class ProxyAllowFrom(Setting):
     name = "proxy_allow_ips"
     section = "Server Mechanics"
     cli = ["--proxy-allow-from"]
-    validator = validate_string_to_list
+    validator = validate_string_to_addr_list
-    default = "127.0.0.1"
+    default = "127.0.0.1,::1"
     desc = """\
-        Front-end's IPs from which allowed accept proxy requests (comma separate).
+        Front-end's IPs from which allowed accept proxy requests (comma separated).
 
-        Set to ``*`` to disable checking of Front-end IPs (useful for setups
+        Set to ``*`` to disable checking of front-end IPs. This is useful for setups
-        where you don't know in advance the IP address of Front-end, but
+        where you don't know in advance the IP address of front-end, but
-        you still trust the environment)
+        instead have ensured via other means that only your
+        authorized front-ends can access Gunicorn.
+
+        .. note::
+
+            This option does not affect UNIX socket connections. Connections not associated with
+            an IP address are treated as allowed, unconditionally.
         """
 
 
@@ -2368,6 +2392,27 @@ 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,PATH_INFO"
+    desc = """\
+
+        A list containing upper-case header field names that the front-end proxy
+        (see :ref:`forwarded-allow-ips`) sets, to be used in WSGI environment.
+
+        This option has no effect for headers not present in the request.
+
+        This option can be used to transfer ``SCRIPT_NAME``, ``PATH_INFO``
+        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"
@@ -2383,9 +2428,13 @@ 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.
 
+        If the source is permitted as explained in :ref:`forwarded-allow-ips`, *and* the header name is
+        present in :ref:`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
         on a proxy in front of Gunicorn.

gunicorn/http/message.py

@@ -78,6 +78,7 @@ class Message(object):
         # handle scheme headers
         scheme_header = False
         secure_scheme_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
@@ -86,6 +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
+            forwarder_headers = cfg.forwarder_headers
 
         # Parse headers into key/value pairs paying attention
         # to continuation lines.
@@ -147,7 +149,10 @@ 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 self.cfg.header_map == "dangerous":
+                if name in forwarder_headers or "*" in forwarder_headers:
+                    # This forwarder may override our environment
+                    pass
+                elif self.cfg.header_map == "dangerous":
                     # as if we did not know we cannot safely map this
                     pass
                 elif self.cfg.header_map == "drop":

tests/test_config.py

@@ -164,16 +164,33 @@ 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"]
+    assert c.proxy_allow_ips == ["127.0.0.1", "::1"]
-    c.set("forwarded_allow_ips", "127.0.0.1,192.168.0.1")
+    assert c.forwarded_allow_ips == ["127.0.0.1", "::1"]
-    assert c.forwarded_allow_ips == ["127.0.0.1", "192.168.0.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", "PATH_INFO"]
+    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():