FinalsClub / karmaworld

KarmaNotes.org v3.0
GNU Affero General Public License v3.0
7 stars 6 forks source link

nginx and Django don't share SSL status #322

Closed btbonval closed 10 years ago

btbonval commented 10 years ago

When I go to https://localhost:6659/ or https://localhost:6659/admin/, the request claims I'm not secure and then redirects:

(Pdb) view_kwargs
{}
(Pdb) list
 13     class SSLRedirect(object):
 14     
 15         def process_view(self, request, view_func, view_args, view_kwargs):
 16             import pdb
 17             pdb.set_trace()
 18  ->         if SSL in view_kwargs:
 19                 secure = view_kwargs[SSL]
 20                 del view_kwargs[SSL]
 21             else:
 22                 secure = False
 23     
(Pdb) request.is_secure()
False

Here's what it looks like if I'm using http://localhost:16659/:

> /home/vagrant/karmaworld/karmaworld/utils/SSLRedirect.py(18)process_view()
-> if SSL in view_kwargs:
(Pdb) view_kwargs
{}
(Pdb) request.is_secure()
False

At least on my machine, nginx is managing security, and SSLRedirect has no idea whether I'm already using HTTPS.

btbonval commented 10 years ago

Running through everything in requests, there is no difference whether I use HTTPS or HTTP. It seems to indicate the connection between nginx and Django? There should be a way to either pass those HTTPS headers through (nginx setting) or that information is available and I'm looking in the wrong place.

btbonval commented 10 years ago

nginx on Beta is passing headers through:

        location / {
                proxy_pass         http://django-beta;
                proxy_redirect     off;
                proxy_set_header   Host             $host;
                proxy_set_header   X-Real-IP        $remote_addr;
                proxy_set_header   X-Forwarded-For  $proxy_add_x_forwarded_for;
        }

compare to VM nginx:

    location / {
        # pass traffic through to gunicorn
        proxy_pass http://127.0.0.1:8000;
    }

Let's try forwarding some headers so maybe Django can see what's going down.

btbonval commented 10 years ago

I only see these problems when I'm signed in. I have never signed into Beta, and even so, if Beta were to forward on every single page, nobody would notice without debugging. It just happens to be a very visible problem for the VM interaction because forwarding is unsupported.

btbonval commented 10 years ago

The right solution appears to be setting headers for X-Forwarded-Proto, X-Forwarded-Protocol and X-Forwarded-SSL, and checking for one of them. SSLRedirect does check one. https://code.djangoproject.com/ticket/14597

nginx has to be setup on the VM, Beta, and Prod to forward at least one of them. It doesn't forward this on any of them, so I must assume Beta is redirecting every page once logged in, because Beta's Django has no way to tell the connection to nginx is SSL.

btbonval commented 10 years ago

Instead of the SSLRedirect._is_secure, someone noted there is a Django setting that may be used instead. https://yuji.wordpress.com/2008/08/15/django-nginx-making-ssl-work-on-django-behind-a-reverse-proxy/#comment-1258

btbonval commented 10 years ago

Alright from everything I've read, I can no longer keep ssl and plaintext in the same config to support header forwarding. Here is what all (VM, staging, prod) have right now for nginx:

server {
     listen 80;
     listen 443 ssl;
     server_name beta.karmanotes.org ;
     ssl_certificate     thing.cert;
     ssl_certificate_key thing.key;

But they will have to be split up with a lot of copy/paste to support all of this properly. sad pandas are sad :(

btbonval commented 10 years ago

renamed ticket to reflect the real problem here.

btbonval commented 10 years ago

oooo maybe I can be lazy and not split them. Pandas can make babies and not be sad now?

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

http://wiki.nginx.org/HttpCoreModule#.24scheme

Here's what Django request says now on HTTPS connection:

(Pdb) request.META['HTTP_X_FORWARDED_SSL']
'https'
(Pdb) request.META['HTTP_X_FORWARDED_PROTOCOL']
'https'
(Pdb) request.META['HTTP_X_FORWARDED_PROTO']
'https'

Here's what Django request says on HTTP connection:

(Pdb) request.META['HTTP_X_FORWARDED_SSL']
'http'
(Pdb) request.META['HTTP_X_FORWARDED_PROTOCOL']
'http'
(Pdb) request.META['HTTP_X_FORWARDED_PROTO']
'http'

So the one problem is that SSLRequest currently checks HTTP_X_FORWARDED_SSL which should be on or off, not http or https. Let's see if nginx can do some neat tricks there! Like an if $scheme=='https' then 'on' else 'off' thing.

btbonval commented 10 years ago

Conditional handling: http://wiki.nginx.org/HttpRewriteModule

        if ($scheme ~ http) {
            set $ssl 'off';
        }
        if ($scheme ~ https) {
            set $ssl 'on';
        }
        proxy_set_header X-Forwarded-SSL $ssl;

HTTP:

(Pdb) request.META['HTTP_X_FORWARDED_SSL']
'off'
(Pdb) request.META['HTTP_X_FORWARDED_PROTOCOL']
'http'
(Pdb) request.META['HTTP_X_FORWARDED_PROTO']
'http'

HTTPS:

(Pdb) request.META['HTTP_X_FORWARDED_SSL']
'on'
(Pdb) request.META['HTTP_X_FORWARDED_PROTOCOL']
'https'
(Pdb) request.META['HTTP_X_FORWARDED_PROTO']
'https'
btbonval commented 10 years ago

SSLRedirect._is_secure now works:

(Pdb) self._is_secure(request)
True

While I'm at it, might not be bad to look into this: https://docs.djangoproject.com/en/1.5/ref/settings/#secure-proxy-ssl-header "A tuple representing a HTTP header/value combination that signifies a request is secure. This controls the behavior of the request object’s is_secure() method."

btbonval commented 10 years ago

might as well use this one shown in the example SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTO', 'https'), and then request.is_secure() will return True right out.

btbonval commented 10 years ago

Well that's weird. logged in and it forwarded to http://127.0.0.1:8000/admin/. I didn't set port 8000 anywhere on my system.

btbonval commented 10 years ago

WOOP I can see the HTTPS stuff when logged in. Problem solved. This will be one of many commits coming out.

Hey @charlesconnell , did you hard code a forward to 127.0.0.1:8000 somewhere? My database Sites is set to localhost:6659 and I was accessing localhost:16659. 127.0.0.1:8000 really came out of nowhere as far as my settings are concerned.

btbonval commented 10 years ago

@charlesconnell nevermind. found it. copy/paste error in nginx.

btbonval commented 10 years ago

Docs are updated as is the Vagrant file. Will push after a bunch of other related problems are fixed and tested.

AndrewMagliozzi commented 10 years ago

Sweet!

On Feb 6, 2014, at 6:33 PM, Bryan Bonvallet notifications@github.com wrote:

Docs are updated as is the Vagrant file. Will push after a bunch of other related problems are fixed and tested.

— Reply to this email directly or view it on GitHub.