werkzeug ProxyFix is configured to ignore all proxy values (was: Issues with admin and https)

[Konzertheld]

In master, the admin login form on /admin sends the login credentials to http even when it is called on https. Also, css and svg files in admin won't load for me because of the mixed protocol. Apparently Firefox blocks loading unsecure resources on secured sites. I will investigate further and keep this ticket updated.

Relevant lines (serving as notes for myself and everyone investigating not familiar with the code yet): https://github.com/posativ/isso/blob/master/isso/views/comments.py#L1099

[Konzertheld]

Workaround: Setting server.public-endpoint to https://comments.example.org without trailing slash

[Konzertheld]

I figured that the issue is in werkzeug.local that is called in the mentioned line as a fallback for public-endpoint. The returned value for local.host has http as protocol even if the host defined in the config has https and the page is called via https.

[Konzertheld]

Got it. local.host is set on dispatch, which uses the host() function in wsgi.py, precisely this line is responsible: https://github.com/posativ/isso/blob/faaff1d4acaad42fc6485ce2090875ade265ea43/isso/wsgi.py#L33

And the url_scheme referenced there comes down to http or https based on a variable ssl_context being set or not in the server class, which seems to be always unset, based on this line in the same file: https://github.com/posativ/isso/blob/faaff1d4acaad42fc6485ce2090875ade265ea43/isso/wsgi.py#L214

If I do SSL termination with Nginx reverse proxy, I use X-Forwarded-Proto which should be respected somewhere but is not.

[Konzertheld]

To me this looks like the appropriate point where the X_FORWARDED_PROTO should be taken care of is in werkzeug. There are too many places where values set by werkzeug are used by isso to fix this in isso itself.

[Konzertheld]

And indeed werkzeug does look at X_FORWARDED_PROTO but ignores it because it is set to trust 0 proxies. At which point I am out because I have no idea how to configure that in isso.

[seyuf]

Thanks @Konzertheld for the workaround 👍 . However, even after setting the public endpoint, the admin seems to be redirected from https to http after the login ...

[Konzertheld]

@seyuf Did you include the https in your public-endpoint and host? I did, and I am not redirected.

[ix5]

@Konzertheld this might give you an idea:

https://github.com/posativ/isso/blob/e6dbd21819f8fdca9fdace447e9ba01276e8d6a9/isso/init.py#L82-L87

https://werkzeug.palletsprojects.com/en/2.0.x/middleware/proxy_fix/

The reason for not setting all the ProxyFix vars to True is explained by Werkzeug's authors:

You must tell the middleware how many proxies set each header so it knows what values to trust. It is a security issue to trust values that came from the client rather than a proxy.

The issue for Isso is that we have little way of knowing what setups users configure. The defaults in the docs (See: Quickstart: Running Isso tell people to use the following:

    location / {
        proxy_pass http://localhost:8080;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header Host $host;
        proxy_set_header X-Forwarded-Proto $scheme;
    }

The docs for Multiple Sites will e.g. omit the protocol:

server {
    listen [::]:80;
    server_name comments.example;

    location / {
        proxy_pass http://localhost:8080;
    }
}

So while the fix for your issue (apart from not using the fallback and instead setting public-endpoint) would lie in __init__.py to set x_proto=1, existing proxy server configs might not pass on that info. So it'd also be great to update and bring the docs up to a common standard.

As for introducing a security risk by allowing clients to set the X-Forwarded-{Host,Port,...} headers, I don't quite understand the scope there.