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

middleware/proxy_headers.py: Inconsistent x-forwarded-proto for websockets #1935

Closed
Kludex opened this issue Apr 9, 2023 Discussed in #1933 · 1 comment · Fixed by #2043
Closed

middleware/proxy_headers.py: Inconsistent x-forwarded-proto for websockets #1935

Kludex opened this issue Apr 9, 2023 Discussed in #1933 · 1 comment · Fixed by #2043

Comments

@Kludex
Copy link
Sponsor Member

Kludex commented Apr 9, 2023

Discussed in #1933

Originally posted by 1997cui April 9, 2023
Hi,
I recently noticed a potential bug in the proxy_headers.py middleware:

The problem

Upstream reverse proxy (i.e. Caddy or Nginx Check scheme variable ) always puts http or https in x-forwarded-proto, even if the request has an Upgrade header and is a WebSocket. This behavior complies with the protocol

The proxy_headers.py passes the value along into the scope["scheme"], even if it is a Websocket. This is not correct. According to the spec, the scheme should be either ws or wss. https or http in websocket type cause routing errors.

A minimal proof of concept

app.py

from quart import Quart, websocket

app = Quart(__name__)

@app.websocket('/test')
async def test():
    await websocket.send('Hello World!')

Caddyfile

example.com {
    route /test {
        reverse_proxy http://localhost:5000 {
            header_up X-Forwarded-For {remote_host}
            header_up X-Forwarded-Proto {scheme}
        }
    }
}

Run it

  • We accept the upstream proxy header
gunicorn app:app --worker-class uvicorn.workers.UvicornWorker --bind 127.0.0.1:5000 --forwarded-allow-ips="*"

Result

wscat -c "wss://slu.t.cuitian1.com/test"
error: Unexpected server response: 403
  • We ignore the upstream header
gunicorn app:app --worker-class uvicorn.workers.UvicornWorker --bind 127.0.0.1:5000 --forwarded-allow-ips=""

result

wscat -c "wss://slu.t.cuitian1.com/test"
Connected (press CTRL+C to quit)
< Hello World!
Disconnected (code: 1000, reason: "")
@1997cui
Copy link

1997cui commented Apr 11, 2023

I tried to add a pull request #1934 just let please let me know if changes are needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants