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

detecting scheme=='https' is broken if bound to unix domain socket #1766

Closed
zt-initech opened this Issue Apr 30, 2018 · 50 comments

Comments

Projects
None yet
10 participants
@zt-initech

zt-initech commented Apr 30, 2018

commit b07532b "Forbid contradictory secure scheme headers" broke "request.scheme" and "wsgi.url_scheme" when gunicorn is bound to unix domain socket. When gunicorn is bound to tcp socket everything works fine.

When used with nginx in front of gunicorn 19.8, nginx is passing HTTP_X_FORWARDED_PROTO == https, but in gunicorn/http/message.py in line remote_addr = self.unreader.sock.getpeername() in case of unix domain socket remote_addr is empty string and then secure_scheme_headers is empty dict and scheme ends up being "http" although the web server is https-only and passed all the headers.

I'm using gevent worker with latest everything on python 3.6 but I don't think it matters.

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Apr 30, 2018

@zt-initech thanks for the report. I'll take a look immediately.

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Apr 30, 2018

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Apr 30, 2018

I'm not sure who else might be around to review it, but it's a pretty simple patch and I tested it locally. If you can verify it, I'll merge and make a release.

@zt-initech

This comment has been minimized.

zt-initech commented Apr 30, 2018

@tilgovi your patch fixed the issue for me. But I would have done this instead:

            if self.unreader.sock.family == socket.AF_UNIX:
                secure_scheme_headers = cfg.secure_scheme_headers
            else:
                remote_addr = self.unreader.sock.getpeername()
                if isinstance(remote_addr, tuple):
                    remote_host = remote_addr[0]
                    if remote_host in cfg.forwarded_allow_ips:
                        secure_scheme_headers = cfg.secure_scheme_headers

and I am not an expert on networking so I don't know if relying on getpeername() returning a string is ok.

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Apr 30, 2018

19.8.1 is released

@tilgovi tilgovi closed this Apr 30, 2018

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Apr 30, 2018

@zt-initech Ah, yes. At least what I did is consistent with what we've had in the past, but I think you're right that would have been better.

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Apr 30, 2018

It would have been clearer with AF_UNIX, but we can rely on string type:

The address of an AF_UNIX socket bound to a file system node is represented as a string

Source: https://docs.python.org/3/library/socket.html

@danielskun

This comment has been minimized.

danielskun commented Jun 14, 2018

I have nginx in front of gunicorn, and 19.8.1 still results in "Contradictory scheme headers" error even if I have set proxy_set_header X-Forwarded-Proto $scheme; in nginx conf.

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Jun 14, 2018

@danielskun is there another reverse proxy (like a load balancer) in front of nginx setting a different header?

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Jun 14, 2018

If you can make a small repo that reproduces the problem I can take a look.

@alorence

This comment has been minimized.

alorence commented Jun 29, 2018

Hi,

Like @danielskun I fall in that case yesterday. After a standard deployment of my django app, I noticed an infinite 301 redirect loop when connecting to its https version.

This was caused by SECURE_SSL_REDIRECT security option, and the cause was the update of gunicorn from 19.7.1 to 19.8.1

With 19.7.1, with SECURE_SSL_REDIRECT=True, Django correctly detected the website is visited with https scheme. The environment wsgi.url_scheme was correctly set. Now, with gunicorn 19.8.1, wsgi.url_scheme contains http.

My wsgi app run behind nginx reverse proxy (no other front app, no load balancer) and is connected via a unix socket (on disk).

I don't know how to debug this, since I develop on windows, I usually can't simulate https connection on my dev machine. But I think I can setup a mini wsgi/django application on my server to easily demonstrate the issue. With an open-source code, you may look by yourself. Of course, I am ready to assist you to debug this, I just don't know how to proceed yet ;)

I will probably publish this mini app tomorrow, let me know if you need more info. Have a nice day

@alorence

This comment has been minimized.

alorence commented Jun 30, 2018

Hi

As promised, I quickly deployed on a test server a very simple django application to demonstrate the issue is still present.

You can display it from these urls: http://gunicorn-test.exige.info and https://gunicorn-test.exige.info
The sources are available on https://github.com/alorence/gunicorn-test

Please note on most browsers, when you first visited https:// link it is not possible to go back to http://. Reaching the page with curl works well indeed.

As you can see, with gunicorn 19.8.1, with a unix socket used to provide a connection between nginx reverse proxy and gunicorn instance, the wsgi.url_scheme is not correctly set.

I'm okay to give more information if you tell me what you need ;)

@danielskun

This comment has been minimized.

danielskun commented Jun 30, 2018

Thanks @alorence, my configuration is the same as yours. @tilgovi I have nothing else in front of nginx.

@mbeacom

This comment has been minimized.

mbeacom commented Jul 5, 2018

I have encountered the same issue attempting to update from 19.17.1 to 19.9.0 under similar conditions as @alorence

@g10f

This comment has been minimized.

g10f commented Jul 18, 2018

I have the same issue. Until 19.7.1 it worked with unix sockets. With 19.8.0, 19.8.1 and 19.9.0 the scheme is set to http, although X-Forwarded-Proto is set to https.

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Jul 19, 2018

When I wrote #1767 and released 19.8.1, I definitely tested this locally. If anyone can make something simple to reproduce the issue it would save me some investigative time! Maybe a repo with a Dockerfile with nginx and gunicorn set up? Otherwise, I'll look at this when I can and I'm happy to review any pull requests. I'm sorry for the disruption.

@benoitc

This comment has been minimized.

Owner

benoitc commented Jul 19, 2018

is this issue coming with django only? which version?

@alorence

This comment has been minimized.

alorence commented Jul 19, 2018

@benoitc This can probably be reproduced with any version of Django. I personally saw this with Django 1.11.14 and Django 2.0.7.

@tilgovi I already pushed online a very simple demonstration of the issue. The code is available from https://github.com/alorence/gunicorn-test, It is not a Docker based setup, but it contains:

The project has been deployed online, you can reach it from http://gunicorn-test.exige.info and https://gunicorn-test.exige.info. Note that you may be unable to reach http version once you already opened https version in recent browsers.

The most important thing to note is wsgi.url_scheme environment is set to http even when the website is visited from https url.

The nginx config file:

server {
    listen 80;
    listen [::]:80;
    listen 443 ssl;
    listen [::]:443 ssl;
    server_name gunicorn-test.exige.info;

    access_log /var/log/nginx/gunicorn-test.access.log;
    error_log  /var/log/nginx/gunicorn-test.error.log;

    location / {
        include proxy_params;
        proxy_pass http://unix:/var/tmp/gunicorn_tests.sock;
    }

    # SSL Configuration
    ssl_certificate /etc/letsencrypt/live/exige.info/fullchain.pem;
    ssl_certificate_key /etc/letsencrypt/live/exige.info/privkey.pem;

    # Common SSL config
    include common_lets_encrypt;
}

Startup script

#!/usr/bin/env bash

source venv/bin/activate
python manage.py migrate
gunicorn --bind unix:/var/tmp/gunicorn_tests.sock gunicorn_https.wsgi:application
@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Jul 19, 2018

@alorence what is in your proxy_params.conf?

@alorence

This comment has been minimized.

alorence commented Jul 19, 2018

@tilgovi

~ ᐅ cat /etc/nginx/proxy_params
proxy_set_header Host $http_host;
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
~ ᐅ sudo nginx -v
nginx version: nginx/1.10.3
@g10f

This comment has been minimized.

g10f commented Jul 19, 2018

benzkji added a commit to benzkji/django-layout that referenced this issue Jul 27, 2018

@benzkji

This comment has been minimized.

benzkji commented Jul 27, 2018

running into the same error, but the above commit seems to help for me, ie setting proxy_set_header X-Forwarded-Proto $scheme;

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Aug 3, 2018

@alorence thank you for the repo. I was able to run it locally under my own nginx with the same proxy params but on localhost. To get it to work I had to generate a self-signed certificate and add localhost to ALLOWED_HOSTS, but I get the right result. wsgi.url_scheme is set and request.is_secure() returns True.

@predatell

This comment has been minimized.

predatell commented Aug 7, 2018

I have this issue and I don't know how to fix it. How many people also have this problem? I have the same configuration as @alorence sent (except django==1.11.15 and python 2.4 if it matters)

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Aug 7, 2018

To be clear: I have not reproduced this :-/. If anyone can share steps to reproduce, please do.

@tisdall

This comment has been minimized.

tisdall commented Aug 20, 2018

I just pushed stuff to production and ran into this issue myself. I had to downgrade back to 19.7.1 .

Here's the versions I was using:
gunicorn 19.9.0
python 2.7.6
django 1.11.15
nginx 1.0.15

My nginx config has the following:

        proxy_set_header X-Forwarded-Proto $scheme;
        proxy_set_header X-Forwarded-Protocol $scheme;

I don't have https in my testing environment, but I'll see if I can set it up and track down the issue.

At least I was able to quickly find this issue when I did a search for "Contradictory scheme headers" and was able to quickly downgrade to fix the issue. I think this issue should probably be re-opened since it seems to affect several people.

@alorence

This comment has been minimized.

alorence commented Aug 20, 2018

I agree that the bug may be re-opened, or maybe a new one could be created, since @zt-initech confirmed his original issue was fixed by PR #1767

@benoitc

This comment has been minimized.

Owner

benoitc commented Aug 20, 2018

@tisdall what is the wsgi environment returned? Also how do you launch gunicorn ? Can you share the config and command line arguments you're using?

@alorence not sure to follow, but if a bug is still existing , please open a ticket for it :)

@predatell

This comment has been minimized.

predatell commented Aug 20, 2018

@alorence has already shared his config in messages above and I have the same lines in my config.

I launch gunicorn through supervisor, command line arguments:

gunicorn app.wsgi:application --bind 127.0.0.1:8000 -w12 --max-requests 10000 --timeout 180

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Aug 20, 2018

@predatell so your issue is at least different in that you are not binding to a UNIX socket.

Can everyone else please provide their gunicorn launch command and any other configuration that you think may be helpful? If there's an issue here I would very much like to fix it.

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Aug 20, 2018

Any details about load balancers and proxies could help, too. For instance, if you're deploying on Heroku, or on AWS, or on some other service. Or even if you have an orchestrator with routing like k8s. Anything that might affect the headers gunicorn receives. Also, I can't tell whether everyone is using nginx or not, as well.

Please, as many details as you can provide. If possible, a whole example repo with deploy instructions would be amazing.

@tisdall

This comment has been minimized.

tisdall commented Aug 20, 2018

I have the issue alive in well in my testing environment now so I can attempt different fixes and see how that works.

gunicorn command (altered for privacy): gunicorn -c /data/web/gunicorn_config.py -b unix:/tmp/wsgi_application.sock wsgi:application

The gunicorn_config.py:

workers = 4
limit_request_line = 8190
max_requests = 8000  # number of requests before restart worker
max_requests_jitter = 500

It's running behind nginx as a proxy with the following config:

        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header Host $http_host;
        proxy_set_header X-NginX-Proxy true;
        proxy_pass http://unix:/tmp/wsgi_application.sock/;
        proxy_redirect off;
        proxy_headers_hash_bucket_size 96;
        proxy_set_header X-Forwarded-Proto $scheme;
        proxy_set_header X-Forwarded-Protocol $scheme;

@benoitc - I'm not sure what you mean by "the wsgi environment returned" or how I report that... Can you clarify where I could find that info?

@tisdall

This comment has been minimized.

tisdall commented Aug 20, 2018

I turned on debugging and noticed this: secure_scheme_headers: {'X-FORWARDED-PROTOCOL': 'ssl', 'X-FORWARDED-PROTO': 'https', 'X-FORWARDED-SSL': 'on'}

However, the nginx config is setting X-Forwarded-Protocol to https instead of ssl. Could this be the cause?

@tisdall

This comment has been minimized.

tisdall commented Aug 20, 2018

Okay, that's definitely the issue... I changed my gunicorn_config.py and added the following to temporarily fix it:

secure_scheme_headers = {'X-FORWARDED-PROTOCOL': 'https', 'X-FORWARDED-PROTO': 'https', 'X-FORWARDED-SSL': 'on'}
@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Aug 20, 2018

Also commented here: #1857 (comment)

The analysis from @tisdall is correct and explains the issue for @predatell as well.

I'm curious if either of you is familiar with software that sets X-Forwarded-Protocol by default and what values it expects. I assume that our default secure_scheme_headers are set up to expect ssl because some software in the wild does that.

@tilgovi

This comment has been minimized.

Collaborator

tilgovi commented Aug 20, 2018

Note that it's not necessary to set more than one of these headers, so you could both remove X-Forwarded-Protocol and leave X-Forwarded-Proto.

The "Contradictory scheme headers" error is meant to detect cases where your reverse proxy only sets some of the secure_scheme_headers but a malicious client attempts to spoof the secure status by setting another one on the request. Unless you expect another reverse proxy, outside of your control, to specify another header there's no need to set more than one of them in nginx.

@alorence

This comment has been minimized.

alorence commented Aug 21, 2018

I don't have proxy_set_header X-Forwarded-Protocol in my nginx config, so in my case, I don't think this is the root cause of the issue. But following @benoitc advice, I will create a new issue ASAP with a summary of the problem and investigations, to sanitize the discussion.

Thanks all for your help

@predatell

This comment has been minimized.

predatell commented Aug 21, 2018

@tilgovi and @tisdall thank you for help. I had problem with this in my nginx:

proxy_set_header X-Forwarded-Protocol $scheme;

@tisdall

This comment has been minimized.

tisdall commented Aug 21, 2018

@alorence - Does changing secure_scheme_headers to {'X-FORWARDED-PROTO': 'https'} solve your issue?

@alorence

This comment has been minimized.

alorence commented Aug 21, 2018

Unfortunately, no. See https://gunicorn-test.exige.info/

The current config file is

secure_scheme_headers = {
#    'X-FORWARDED-PROTOCOL': 'ssl',
    'X-FORWARDED-PROTO': 'https',
#    'X-FORWARDED-SSL': 'on'
}

and the updated command line to run gunicorn is: gunicorn --bind unix:/var/tmp/gunicorn_tests.sock -c ./gunicorn_config.py gunicorn_https.wsgi:application

@tisdall

This comment has been minimized.

tisdall commented Aug 21, 2018

@alorence - What's in gunicorn_https/wsgi.py?

@tisdall

This comment has been minimized.

tisdall commented Aug 21, 2018

@alorence - BTW, I'm able to load https://gunicorn-test.exige.info/ successfully...

@alorence

This comment has been minimized.

alorence commented Aug 21, 2018

This application is still available on github. Here is the content of wsgi.py:

import os

from django.core.wsgi import get_wsgi_application

os.environ.setdefault("DJANGO_SETTINGS_MODULE", "gunicorn_https.settings")

application = get_wsgi_application()

I don't understand your next remark. I can open https://gunicorn-test.exige.info/ too, but the content is invalid:

request.is_secure: False
request.environ['wsgi.url_scheme']: http

We should have request.is_secure = True and request.environ['wsgi.url_scheme']: https

@alorence

This comment has been minimized.

alorence commented Aug 21, 2018

For the record, this is a small application I quickly wrote and put online on 1 of my tests server to demonstrate the issue. More information in #1766 (comment)

@tisdall

This comment has been minimized.

tisdall commented Aug 21, 2018

Oh, sorry.. I thought at one point I saw it giving the "Contradictory scheme headers" error instead of content. I also forgot you mentioning the code was on github.

Yeah, I checked and mine is correctly setting request.is_secure to True and request.environ['wsgi.url_scheme'] to 'https'.

What if you explicitly add proxy_set_header X-Forwarded-Proto $scheme; to the nginx config? I think nginx may not automatically use what's in proxy_params and you have to actually include with a include proxy_params; in that location / block.

@tisdall

This comment has been minimized.

tisdall commented Aug 21, 2018

uh.. you have the include statement there.. I think I need a rest. ^_^

@alorence

This comment has been minimized.

alorence commented Aug 21, 2018

I pushed these modifications to nginx config file, and restarted the app. The result remains the same. https://gunicorn-test.exige.info/

server {
    listen 80;
    listen [::]:80;
    listen 443 ssl;
    listen [::]:443 ssl;
    server_name gunicorn-test.exige.info;

    access_log /var/log/nginx/gunicorn-test.access.log;
    error_log  /var/log/nginx/gunicorn-test.error.log;

    location / {
        proxy_set_header Host $http_host;
        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header X-Forwarded-Proto $scheme;
        proxy_pass http://unix:/var/tmp/gunicorn_tests.sock;
    }

    # SSL Configuration
    ssl_certificate /etc/letsencrypt/live/exige.info/fullchain.pem;
    ssl_certificate_key /etc/letsencrypt/live/exige.info/privkey.pem;

    # Common SSL config
    include common_lets_encrypt;
}
@tisdall

This comment has been minimized.

tisdall commented Aug 21, 2018

frustrating! Any clues in request.META?

@alorence

This comment has been minimized.

alorence commented Aug 21, 2018

I have to update project's sources to print content of request.META in this environment. Unfortunately, I won't be able to do this before 2 days. Sorry about that. As soon as possible, I will update the project, restart it on the test server, and maybe open a new issue so we can continue the discussion in an open ticket... Thank you very much for your help

@tisdall

This comment has been minimized.

tisdall commented Aug 21, 2018

Okay. Tag me on the new issue and I'll take a look when you've changed it...

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