Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

22.0.0 regression: We need a better default treatment of SCRIPT_NAME #3192

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 10 additions & 1 deletion .github/workflows/tox.yml
Expand Up @@ -14,7 +14,11 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest] # All OSes pass except Windows because tests need Unix-only fcntl, grp, pwd, etc.
os:
- ubuntu-latest
# not defaulting to macos-latest: Python <= 3.9 was missing from macos-14 @ arm64
- macos-13
# Not testing Windows, because tests need Unix-only fcntl, grp, pwd, etc.
python-version:
# CPython <= 3.7 is EoL since 2023-06-27
- "3.7"
Expand All @@ -26,6 +30,11 @@ jobs:
# PyPy <= 3.8 is EoL since 2023-06-16
- "pypy-3.9"
- "pypy-3.10"
include:
# Note: potentially "universal2" release
# https://github.com/actions/runner-images/issues/9741
- os: macos-latest
python-version: "3.12"
steps:
- uses: actions/checkout@v4
- name: Using Python ${{ matrix.python-version }}
Expand Down
23 changes: 14 additions & 9 deletions docs/source/deploy.rst
Expand Up @@ -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=/home/someuser/applicationroot
ExecStart=/usr/bin/gunicorn applicationname.wsgi
WorkingDirectory=~
# using the @ prefix, you can set venv path via argv[0]
ExecStart=@/usr/bin/python3 ${HOME}/bin/python3 -m gunicorn --chdir appdir 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
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion docs/source/faq.rst
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions docs/source/news.rst
Expand Up @@ -2,6 +2,16 @@
Changelog
=========

22.0.1 - 2TBDTBDTBD
===================

- the SCRIPT_NAME header when received from allowed forwarders is no longer restricted for containing an underscore

*** NOTE ***

- This 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

22.0.0 - 2024-04-17
===================

Expand Down
49 changes: 43 additions & 6 deletions gunicorn/config.py
Expand Up @@ -9,6 +9,7 @@
import copy
import grp
import inspect
import ipaddress
import os
import pwd
import re
Expand Down Expand Up @@ -400,6 +401,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)

Expand Down Expand Up @@ -1260,18 +1272,19 @@ class ForwardedAllowIPS(Setting):
section = "Server Mechanics"
cli = ["--forwarded-allow-ips"]
meta = "STRING"
validator = validate_string_to_list
default = os.environ.get("FORWARDED_ALLOW_IPS", "127.0.0.1")
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.
(comma separate).

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
you still trust the environment).
instead have ensured via other means that none other than 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::

Expand Down Expand Up @@ -2340,6 +2353,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"
Expand All @@ -2355,9 +2388,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 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
on a proxy in front of Gunicorn.
Expand Down
7 changes: 6 additions & 1 deletion gunicorn/http/message.py
Expand Up @@ -77,6 +77,7 @@ def parse_headers(self, data, from_trailer=False):
# 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
Expand All @@ -85,6 +86,7 @@ def parse_headers(self, data, from_trailer=False):
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.
Expand Down Expand Up @@ -140,7 +142,10 @@ def parse_headers(self, data, from_trailer=False):
# 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":
Expand Down
24 changes: 20 additions & 4 deletions tests/test_config.py
Expand Up @@ -147,16 +147,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():
Expand Down