Opened 17 years ago
Closed 17 years ago
#4710 closed (fixed)
ModPythonRequest.is_secure(): accept greater range of HTTPS env. settings
Reported by: | anonymous | Owned by: | nobody |
---|---|---|---|
Component: | Uncategorized | Version: | dev |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Hi,
In django/core/handlers/modpython.py, the method
ModPythonRequest.is_secure() method checks the setting of the HTTPS
environment variable. Currently it assumes that this will be set to
the string "on" if SSL is active. However, it is common to use other
values, in particular "1" - do a google search for `apache "SetEnv
HTTPS 1"' to see examples of this. The current version of is_secure
will incorrectly return False in this case.
The submitted patch fixes this issue. In addition, it removes case
insensitivity, and makes it easy to extend, if it turns out other
values (like "true" or something) need to be accepted.
Thanks,
Aaron Maxwell
SnapLogic - Open Source Data Integration
http://SnapLogic.org
Attachments (2)
Change History (9)
by , 17 years ago
Attachment: | is_secure.diff added |
---|
comment:1 by , 17 years ago
Just to clarify, which version of Apache are you using?
In Apache 2.X, the HTTPS variable is set in the request subprocess environment by the SSL engine in Apache.
/* * Annotate the SSI/CGI environment with standard SSL information */ /* the always present HTTPS (=HTTP over SSL) flag! */ apr_table_setn(env, "HTTPS", "on");
The mod_python request handler will inherit this value. There should never be a need for a user to explicitly use SetEnv to set the value of HTTP when using mod_python, at least on Apache 2.X.
So perhaps indicate which version of Apache and mod_python you are using so the scope of the problem can be determined.
comment:2 by , 17 years ago
Apache version is Apache/2.0.52, with mod_python 3.3.1. The httpd came
with RHEL 4; we compiled the mod_python from source.
There is a VirtualHost declaration for the site, in which we had been
explicitly setting the value with SetEnv. I tried just not setting
HTTPS (everywhere in the apache config, not just in that vhost), to
see if our apache would set it, but it appears not.
Interesting issue here, not sure why the HTTPS variable is not being
set automatically for us. Does the version of mod_ssl have anything
to do with it? Perhaps there is something else in our configuration.
Thanks,
Aaron
http://SnapLogic.org
comment:3 by , 17 years ago
The addition of HTTPS is done in the Apache fixup phase, ie., last phase before actual response handler is called. This should mean it will always be set before Django mod_python code is triggered from mod_python PythonHandler if SSL is enabled. It shouldn't matter whether a VirtualHost is being used, or even some other specialised means for managing virtual hosts such as the various vhost modules or rewrite rules.
By rights where the mod_python code currently has:
def is_secure(self): # Note: modpython 3.2.10+ has req.is_https(), but we need to support previous versions return self._req.subprocess_env.has_key('HTTPS') and self._req.subprocess_env['HTTPS'] == 'on'
it should probably use:
def is_secure(self): try: return self._req.is_https() except: return self._req.subprocess_env.has_key('HTTPS') and self._req.subprocess_env['HTTPS'] == 'on'
There is no reason for it not to try the better is_https() method and fall back to looking for HTTPS instead.
If you make this change instead, does that work?
If that doesn't pickup a secure connection is present, can only assume that your web server isn't actually receiving an SSL connection.
Is this web server the actual facing SSL enabled server, or does it sit behind another web server using mod_proxy or similar?
As a side note, because HTTPS is only set in the fixup phase it will be after the authen phase and thus anyone hooking in with PythonAuthenHandler to use Django for authentication would not pick up things as a secure connection if relying on HTTPS to be present. The is_https() should be okay though.
Back to the change, a combination of your change and is_https() check would probably be the best. Thus:
def is_secure(self): try: return self._req.is_https() except: return self._req.subprocess_env.has_key('HTTPS') and self._req.subprocess_env['HTTPS'].lower() in ['on', '1']
Still be interesting to know why HTTPS not being set. I don't think I am wrong in believing it should be.
comment:4 by , 17 years ago
Yep, that does it. Tried both (the first try/except, and the second,
combined one), with no SetEnv HTTPS in the apache conf, and in both
cases the request responds properly. Experimenting a bit verified that
it's the self._req.is_https() returning here in our case.
For the record, this server is the actual facing server, not behind a
proxy or anything.
Don't know at all about the HTTPS var, but if I find out (probably not
likely) I'll post here.
Thanks
Aaron
comment:5 by , 17 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
by , 17 years ago
Attachment: | is_secure.patch added |
---|
patch taking in Graham's suggestion of trying is_https()
first
comment:6 by , 17 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:7 by , 17 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
patch