Vasanko/sanic
2
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Forwarded headers and otherwise improved proxy handling (#1638)

Browse Source 
* Added support for HTTP Forwarded header and combined parsing of other proxy headers.

- Accessible via request.forwarded that tries parse_forwarded and then parse_xforwarded
- parse_forwarded uses the Forwarded header, if config.FORWARDED_SECRET is provided and a matching header field is found
- parse_xforwarded uses X-Real-IP and X-Forwarded-* much alike the existing implementation
- This commit does not change existing request properties that still use the old code and won't make use of Forwarded headers.

* Use req.forwarded in req properties server_name, server_port, scheme and remote_addr.

X-Scheme handling moved to parse_xforwarded.

* Cleanup and fix req.server_port; no longer reports socket port if any forwards headers are used.

* Update docstrings to incidate that forwarded header is used first.

* Remove testing function.

* Fix tests and linting.

- One test removed due to change of semantics - no socket port will be used if any forwarded headers are in effect.
- Other tests augmented with X-Forwarded-For, to allow the header being tested take effect (shouldn't affect old implementation).

* Try to workaround buggy tools complaining about incorrect ordering of imports.

* Cleanup forwarded processing, add comments. secret is now also returned.

* Added tests, fixed quoted string handling, cleanup.

* Further tests for full coverage.

* Try'n make linter happy.

* Add support for multiple Forwarded headers. Unify parse_forwarded parameters with parse_xforwarded.

* Implement multiple headers support for X-Forwarded-For.

- Previously only the first header was used, so this BUGFIX may affect functionality.

* Bugfix for request.server_name: strip port and other parts.

- request.server_name docs claim that it returns the hostname only (no port).
- config.SERVER_NAME may be full URL, so strip scheme, port and path
- HTTP Host and consequently forwarded Host may include port number, so
  strip that also for forwarded hosts (previously only done for HTTP Host).
- Possible performance benefit of limiting to one split.

* Fallback to app.url_for and let it handle SERVER_NAME if defined (until a proper solution is implemented).

* Revise previous commit. Only fallback for full URL SERVER_NAMEs; allows host to be defined and proxied information still being used.

* Heil lintnazi.

* Modify testcase not to use underscores in URLs. Use hyphens which the spec allows for.

* Forwarded and Host header parsing improved.

- request.forwarded lowercases hosts, separates host:port into their own fields and lowercases addresses
- forwarded.parse_host helper function added and used for parsing all host-style headers (IPv6 cannot be simply split(":")).
- more tests fixed not to use underscores in hosts as those are no longer accepted and lead to the field being rejected

* Fixed typo in docstring.

* Added IPv6 address tests for Host header.

* Fix regex.

* Further tests and stricter forwarded handling.

* Fix merge commit

* Linter

* Linter

* Linter

* Add  to avoid re-using the  variable. Make a few raw strings non-raw.

* Remove unnecessary or

* Updated docs (work in progress).

* Enable REAL_IP_HEADER parsing irregardless of PROXIES_COUNT setting.

- Also cleanup and added comments

* New defaults for PROXIES_COUNT and REAL_IP_HEADER, updated tests.

* Remove support for PROXIES_COUNT=-1.

* Linter errors.

- This is getting ridiculous: cannot fit an URL on one line, linter requires
  splitting the string literal!

* Add support for by=_proxySecret, updated docs, updated tests.

* Forwarded headers' semantics tuning.

- Forwarded host is now preserved in original format
- request.host now returns a forwarded host if available, else the Host header
- Forwarded options are preserved in original order, and later keys override earlier ones
- Forwarded path is automatically URL-unquoted
- Forwarded 'by' and 'for' are omitted if their value is unknown
- Tests modified accordingly
- Cleanup and improved documentation

* Add ASGI test.

* Linter

* Linter #2
This commit is contained in:
L. Kärkkäinen
2019-09-02 16:50:56 +03:00
committed by Stephen Sadowski
parent ae91852cd5
commit 1e4b1c4d1a
6 changed files with 545 additions and 166 deletions
Download Patch File Download Diff File Expand all files Collapse all files

356
tests/test_requests.py
View File

@@ -401,8 +401,232 @@ async def test_content_type_asgi(app):
assert response.text == "application/json"
def test_standard_forwarded(app):
@app.route("/")
async def handler(request):
return json(request.forwarded)
# Without configured FORWARDED_SECRET, x-headers should be respected
app.config.PROXIES_COUNT = 1
app.config.REAL_IP_HEADER = "x-real-ip"
headers = {
"Forwarded": (
'for=1.1.1.1, for=injected;host="'
', for="[::2]";proto=https;host=me.tld;path="/app/";secret=mySecret'
',for=broken;;secret=b0rked'
', for=127.0.0.3;scheme=http;port=1234'
),
"X-Real-IP": "127.0.0.2",
"X-Forwarded-For": "127.0.1.1",
"X-Scheme": "ws",
}
request, response = app.test_client.get("/", headers=headers)
assert response.json == { "for": "127.0.0.2", "proto": "ws" }
assert request.remote_addr == "127.0.0.2"
assert request.scheme == "ws"
assert request.server_port == 80
app.config.FORWARDED_SECRET = "mySecret"
request, response = app.test_client.get("/", headers=headers)
assert response.json == {
"for": "[::2]",
"proto": "https",
"host": "me.tld",
"path": "/app/",
"secret": "mySecret"
}
assert request.remote_addr == "[::2]"
assert request.server_name == "me.tld"
assert request.scheme == "https"
assert request.server_port == 443
# Empty Forwarded header -> use X-headers
headers["Forwarded"] = ""
request, response = app.test_client.get("/", headers=headers)
assert response.json == { "for": "127.0.0.2", "proto": "ws" }
# Header present but not matching anything
request, response = app.test_client.get("/", headers={"Forwarded": "."})
assert response.json == {}
# Forwarded header present but no matching secret -> use X-headers
headers = {
"Forwarded": 'for=1.1.1.1;secret=x, for=127.0.0.1',
"X-Real-IP": "127.0.0.2"
}
request, response = app.test_client.get("/", headers=headers)
assert response.json == {"for": "127.0.0.2"}
assert request.remote_addr == "127.0.0.2"
# Different formatting and hitting both ends of the header
headers = {"Forwarded": 'Secret="mySecret";For=127.0.0.4;Port=1234'}
request, response = app.test_client.get("/", headers=headers)
assert response.json == {
"for": "127.0.0.4",
"port": 1234,
"secret": "mySecret"
}
# Test escapes (modify this if you see anyone implementing quoted-pairs)
headers = {"Forwarded": 'for=test;quoted="\\,x=x;y=\\";secret=mySecret'}
request, response = app.test_client.get("/", headers=headers)
assert response.json == {
"for": "test",
"quoted": '\\,x=x;y=\\',
"secret": "mySecret"
}
# Secret insulated by malformed field #1
headers = {"Forwarded": 'for=test;secret=mySecret;b0rked;proto=wss;'}
request, response = app.test_client.get("/", headers=headers)
assert response.json == {"for": "test", "secret": "mySecret"}
# Secret insulated by malformed field #2
headers = {"Forwarded": 'for=test;b0rked;secret=mySecret;proto=wss'}
request, response = app.test_client.get("/", headers=headers)
assert response.json == {"proto": "wss", "secret": "mySecret"}
# Unexpected termination should not lose existing acceptable values
headers = {"Forwarded": 'b0rked;secret=mySecret;proto=wss'}
request, response = app.test_client.get("/", headers=headers)
assert response.json == {"proto": "wss", "secret": "mySecret"}
# Field normalization
headers = {
"Forwarded": 'PROTO=WSS;BY="CAFE::8000";FOR=unknown;PORT=X;HOST="A:2";'
'PATH="/With%20Spaces%22Quoted%22/sanicApp?key=val";SECRET=mySecret'
}
request, response = app.test_client.get("/", headers=headers)
assert response.json == {
"proto": "wss",
"by": "[cafe::8000]",
"host": "a:2",
"path": '/With Spaces"Quoted"/sanicApp?key=val',
"secret": "mySecret",
}
# Using "by" field as secret
app.config.FORWARDED_SECRET = "_proxySecret"
headers = {"Forwarded": 'for=1.2.3.4; by=_proxySecret'}
request, response = app.test_client.get("/", headers=headers)
assert response.json == {"for": "1.2.3.4", "by": "_proxySecret"}
@pytest.mark.asyncio
async def test_standard_forwarded_asgi(app):
@app.route("/")
async def handler(request):
return json(request.forwarded)
# Without configured FORWARDED_SECRET, x-headers should be respected
app.config.PROXIES_COUNT = 1
app.config.REAL_IP_HEADER = "x-real-ip"
headers = {
"Forwarded": (
'for=1.1.1.1, for=injected;host="'
', for="[::2]";proto=https;host=me.tld;path="/app/";secret=mySecret'
',for=broken;;secret=b0rked'
', for=127.0.0.3;scheme=http;port=1234'
),
"X-Real-IP": "127.0.0.2",
"X-Forwarded-For": "127.0.1.1",
"X-Scheme": "ws",
}
request, response = await app.asgi_client.get("/", headers=headers)
assert response.json() == { "for": "127.0.0.2", "proto": "ws" }
assert request.remote_addr == "127.0.0.2"
assert request.scheme == "ws"
assert request.server_port == 80
app.config.FORWARDED_SECRET = "mySecret"
request, response = await app.asgi_client.get("/", headers=headers)
assert response.json() == {
"for": "[::2]",
"proto": "https",
"host": "me.tld",
"path": "/app/",
"secret": "mySecret"
}
assert request.remote_addr == "[::2]"
assert request.server_name == "me.tld"
assert request.scheme == "https"
assert request.server_port == 443
# Empty Forwarded header -> use X-headers
headers["Forwarded"] = ""
request, response = await app.asgi_client.get("/", headers=headers)
assert response.json() == { "for": "127.0.0.2", "proto": "ws" }
# Header present but not matching anything
request, response = await app.asgi_client.get("/", headers={"Forwarded": "."})
assert response.json() == {}
# Forwarded header present but no matching secret -> use X-headers
headers = {
"Forwarded": 'for=1.1.1.1;secret=x, for=127.0.0.1',
"X-Real-IP": "127.0.0.2"
}
request, response = await app.asgi_client.get("/", headers=headers)
assert response.json() == {"for": "127.0.0.2"}
assert request.remote_addr == "127.0.0.2"
# Different formatting and hitting both ends of the header
headers = {"Forwarded": 'Secret="mySecret";For=127.0.0.4;Port=1234'}
request, response = await app.asgi_client.get("/", headers=headers)
assert response.json() == {
"for": "127.0.0.4",
"port": 1234,
"secret": "mySecret"
}
# Test escapes (modify this if you see anyone implementing quoted-pairs)
headers = {"Forwarded": 'for=test;quoted="\\,x=x;y=\\";secret=mySecret'}
request, response = await app.asgi_client.get("/", headers=headers)
assert response.json() == {
"for": "test",
"quoted": '\\,x=x;y=\\',
"secret": "mySecret"
}
# Secret insulated by malformed field #1
headers = {"Forwarded": 'for=test;secret=mySecret;b0rked;proto=wss;'}
request, response = await app.asgi_client.get("/", headers=headers)
assert response.json() == {"for": "test", "secret": "mySecret"}
# Secret insulated by malformed field #2
headers = {"Forwarded": 'for=test;b0rked;secret=mySecret;proto=wss'}
request, response = await app.asgi_client.get("/", headers=headers)
assert response.json() == {"proto": "wss", "secret": "mySecret"}
# Unexpected termination should not lose existing acceptable values
headers = {"Forwarded": 'b0rked;secret=mySecret;proto=wss'}
request, response = await app.asgi_client.get("/", headers=headers)
assert response.json() == {"proto": "wss", "secret": "mySecret"}
# Field normalization
headers = {
"Forwarded": 'PROTO=WSS;BY="CAFE::8000";FOR=unknown;PORT=X;HOST="A:2";'
'PATH="/With%20Spaces%22Quoted%22/sanicApp?key=val";SECRET=mySecret'
}
request, response = await app.asgi_client.get("/", headers=headers)
assert response.json() == {
"proto": "wss",
"by": "[cafe::8000]",
"host": "a:2",
"path": '/With Spaces"Quoted"/sanicApp?key=val',
"secret": "mySecret",
}
# Using "by" field as secret
app.config.FORWARDED_SECRET = "_proxySecret"
headers = {"Forwarded": 'for=1.2.3.4; by=_proxySecret'}
request, response = await app.asgi_client.get("/", headers=headers)
assert response.json() == {"for": "1.2.3.4", "by": "_proxySecret"}
def test_remote_addr_with_two_proxies(app):
app.config.PROXIES_COUNT = 2
app.config.REAL_IP_HEADER = "x-real-ip"
@app.route("/")
async def handler(request):
@@ -443,6 +667,7 @@ def test_remote_addr_with_two_proxies(app):
@pytest.mark.asyncio
async def test_remote_addr_with_two_proxies_asgi(app):
app.config.PROXIES_COUNT = 2
app.config.REAL_IP_HEADER = "x-real-ip"
@app.route("/")
async def handler(request):
@@ -480,57 +705,6 @@ async def test_remote_addr_with_two_proxies_asgi(app):
assert response.text == "127.0.0.1"
def test_remote_addr_with_infinite_number_of_proxies(app):
app.config.PROXIES_COUNT = -1
@app.route("/")
async def handler(request):
return text(request.remote_addr)
headers = {"X-Real-IP": "127.0.0.2", "X-Forwarded-For": "127.0.1.1"}
request, response = app.test_client.get("/", headers=headers)
assert request.remote_addr == "127.0.0.2"
assert response.text == "127.0.0.2"
headers = {"X-Forwarded-For": "127.0.1.1"}
request, response = app.test_client.get("/", headers=headers)
assert request.remote_addr == "127.0.1.1"
assert response.text == "127.0.1.1"
headers = {
"X-Forwarded-For": "127.0.0.5, 127.0.0.4, 127.0.0.3, 127.0.0.2, 127.0.0.1"
}
request, response = app.test_client.get("/", headers=headers)
assert request.remote_addr == "127.0.0.5"
assert response.text == "127.0.0.5"
@pytest.mark.asyncio
async def test_remote_addr_with_infinite_number_of_proxies_asgi(app):
app.config.PROXIES_COUNT = -1
@app.route("/")
async def handler(request):
return text(request.remote_addr)
headers = {"X-Real-IP": "127.0.0.2", "X-Forwarded-For": "127.0.1.1"}
request, response = await app.asgi_client.get("/", headers=headers)
assert request.remote_addr == "127.0.0.2"
assert response.text == "127.0.0.2"
headers = {"X-Forwarded-For": "127.0.1.1"}
request, response = await app.asgi_client.get("/", headers=headers)
assert request.remote_addr == "127.0.1.1"
assert response.text == "127.0.1.1"
headers = {
"X-Forwarded-For": "127.0.0.5, 127.0.0.4, 127.0.0.3, 127.0.0.2, 127.0.0.1"
}
request, response = await app.asgi_client.get("/", headers=headers)
assert request.remote_addr == "127.0.0.5"
assert response.text == "127.0.0.5"
def test_remote_addr_without_proxy(app):
app.config.PROXIES_COUNT = 0
@@ -634,15 +808,16 @@ def test_forwarded_scheme(app):
async def handler(request):
return text(request.remote_addr)
app.config.PROXIES_COUNT = 1
request, response = app.test_client.get("/")
assert request.scheme == "http"
request, response = app.test_client.get(
"/", headers={"X-Forwarded-Proto": "https"}
"/", headers={"X-Forwarded-For": "127.1.2.3", "X-Forwarded-Proto": "https"}
)
assert request.scheme == "https"
request, response = app.test_client.get("/", headers={"X-Scheme": "https"})
request, response = app.test_client.get("/", headers={"X-Forwarded-For": "127.1.2.3", "X-Scheme": "https"})
assert request.scheme == "https"
@@ -1688,9 +1863,19 @@ def test_request_server_name_in_host_header(app):
return text("OK")
request, response = app.test_client.get(
"/", headers={"Host": "my_server:5555"}
"/", headers={"Host": "my-server:5555"}
)
assert request.server_name == "my_server"
assert request.server_name == "my-server"
request, response = app.test_client.get(
"/", headers={"Host": "[2a00:1450:400f:80c::200e]:5555"}
)
assert request.server_name == "[2a00:1450:400f:80c::200e]"
request, response = app.test_client.get(
"/", headers={"Host": "mal_formed"}
)
assert request.server_name == None # For now (later maybe 127.0.0.1)
def test_request_server_name_forwarded(app):
@@ -1698,11 +1883,12 @@ def test_request_server_name_forwarded(app):
def handler(request):
return text("OK")
app.config.PROXIES_COUNT = 1
request, response = app.test_client.get(
"/",
headers={"Host": "my_server:5555", "X-Forwarded-Host": "your_server"},
headers={"Host": "my-server:5555", "X-Forwarded-For": "127.1.2.3", "X-Forwarded-Host": "your-server"},
)
assert request.server_name == "your_server"
assert request.server_name == "your-server"
def test_request_server_port(app):
@@ -1710,7 +1896,7 @@ def test_request_server_port(app):
def handler(request):
return text("OK")
request, response = app.test_client.get("/", headers={"Host": "my_server"})
request, response = app.test_client.get("/", headers={"Host": "my-server"})
assert request.server_port == app.test_client.port
@@ -1720,18 +1906,29 @@ def test_request_server_port_in_host_header(app):
return text("OK")
request, response = app.test_client.get(
"/", headers={"Host": "my_server:5555"}
"/", headers={"Host": "my-server:5555"}
)
assert request.server_port == 5555
request, response = app.test_client.get(
"/", headers={"Host": "[2a00:1450:400f:80c::200e]:5555"}
)
assert request.server_port == 5555
request, response = app.test_client.get(
"/", headers={"Host": "mal_formed:5555"}
)
assert request.server_port == app.test_client.port
def test_request_server_port_forwarded(app):
@app.get("/")
def handler(request):
return text("OK")
app.config.PROXIES_COUNT = 1
request, response = app.test_client.get(
"/", headers={"Host": "my_server:5555", "X-Forwarded-Port": "4444"}
"/", headers={"Host": "my-server:5555", "X-Forwarded-For": "127.1.2.3", "X-Forwarded-Port": "4444"}
)
assert request.server_port == 4444
@@ -1746,6 +1943,23 @@ def test_request_form_invalid_content_type(app):
assert request.form == {}
def test_server_name_and_url_for(app):
@app.get("/foo")
def handler(request):
return text("ok")
app.config.SERVER_NAME = "my-server"
assert app.url_for("handler", _external=True) == "http://my-server/foo"
request, response = app.test_client.get("/foo")
assert request.url_for("handler") == f"http://my-server:{app.test_client.port}/foo"
app.config.SERVER_NAME = "https://my-server/path"
request, response = app.test_client.get("/foo")
url = f"https://my-server/path/foo"
assert app.url_for("handler", _external=True) == url
assert request.url_for("handler") == url
def test_url_for_with_forwarded_request(app):
@app.get("/")
def handler(request):
@@ -1755,32 +1969,24 @@ def test_url_for_with_forwarded_request(app):
def view_name(request):
return text("OK")
app.config.SERVER_NAME = "my-server"
app.config.PROXIES_COUNT = 1
request, response = app.test_client.get(
"/", headers={"X-Forwarded-Proto": "https"}
)
assert app.url_for("view_name") == "/another_view"
assert app.url_for("view_name", _external=True) == "http:///another_view"
assert request.url_for(
"view_name"
) == "https://127.0.0.1:{}/another_view".format(app.test_client.port)
app.config.SERVER_NAME = "my_server"
request, response = app.test_client.get(
"/", headers={"X-Forwarded-Proto": "https", "X-Forwarded-Port": "6789"}
"/", headers={"X-Forwarded-For": "127.1.2.3", "X-Forwarded-Proto": "https", "X-Forwarded-Port": "6789"}
)
assert app.url_for("view_name") == "/another_view"
assert (
app.url_for("view_name", _external=True)
== "http://my_server/another_view"
== "http://my-server/another_view"
)
assert (
request.url_for("view_name") == "https://my_server:6789/another_view"
request.url_for("view_name") == "https://my-server:6789/another_view"
)
request, response = app.test_client.get(
"/", headers={"X-Forwarded-Proto": "https", "X-Forwarded-Port": "443"}
"/", headers={"X-Forwarded-For": "127.1.2.3", "X-Forwarded-Proto": "https", "X-Forwarded-Port": "443"}
)
assert request.url_for("view_name") == "https://my_server/another_view"
assert request.url_for("view_name") == "https://my-server/another_view"
@pytest.mark.asyncio