Skip to content

Using ProxyHeadersMiddleware with client on trusted proxy results in invalid client information #1068

@jchristgit

Description

@jchristgit

Checklist

  • The bug is reproducible against the latest release and/or master.
  • There are no similar issues or pull requests to fix it yet.

Describe the bug

Using ProxyHeadersMiddleware results in the client being [None, 0] when the original client runs on a trusted proxy.

In our setup, we have a client service which runs on one of our proxy servers. The proxy servers run on HAProxy with option forwardfor to add the X-Forwarded-For header. With today's upgrade to uvicorn 0.14.0, the client fields are no longer present for requests coming from the mentioned server.

To reproduce

This addition to the tests should illustrate the problem:

diff --git a/tests/middleware/test_proxy_headers.py b/tests/middleware/test_proxy_headers.py
index aacf789..b453892 100644
--- a/tests/middleware/test_proxy_headers.py
+++ b/tests/middleware/test_proxy_headers.py
@@ -27,6 +27,8 @@ async def app(scope, receive, send):
         ("127.0.0.1, 10.0.0.1", "Remote: https://1.2.3.4:0"),
         # request from untrusted proxy
         ("192.168.0.1", "Remote: http://127.0.0.1:123"),
+        # request from client running on proxy server
+        (["127.0.0.1", "1.2.3.4"], "Remote: https://1.2.3.4:0"),
     ],
 )
 async def test_proxy_headers_trusted_hosts(trusted_hosts, response_text):

which results in:

>       assert response.text == response_text
E       AssertionError: assert 'Remote: https://None:0' == 'Remote: https://1.2.3.4:0'
E         - Remote: https://1.2.3.4:0
E         ?                 ^^^^^^^
E         + Remote: https://None:0
E         ?                 ^^^^

Expected behavior

The proper client address should be forwarded as before.

Actual behavior

The client is set to [None, 0].

Debugging material

I assume this is a regression of #591, more specifically, see the fallthrough discussion on https://github.com/encode/uvicorn/pull/591/files#r438008416. That said, I am not entirely sure what the best way to fix this would be - based on the discussion, the original code seems to be security-related. Would returning the last entry in x_forwarded_for be sufficient?

Environment

Running uvicorn 0.14.0 with CPython 3.9.1 on Linux

I can provide proxy & detailed uvicorn configuration if needed, but I will need to sanitise it, that said, I am not sure if it's needed for this issue.

Important

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions