Code

Opened 7 years ago

Closed 7 years ago

#4710 closed (fixed)

ModPythonRequest.is_secure(): accept greater range of HTTPS env. settings

Reported by: anonymous Owned by: nobody
Component: Uncategorized Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

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)

is_secure.diff (646 bytes) - added by amax@… 7 years ago.
patch
is_secure.patch (877 bytes) - added by SmileyChris 7 years ago.
patch taking in Graham's suggestion of trying is_https() first

Download all attachments as: .zip

Change History (9)

Changed 7 years ago by amax@…

patch

comment:1 Changed 7 years ago by Graham.Dumpleton@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 7 years ago by amax@…

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 Changed 7 years ago by Graham.Dumpleton@…

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 Changed 7 years ago by amax@…

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 Changed 7 years ago by SmileyChris

  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Changed 7 years ago by SmileyChris

patch taking in Graham's suggestion of trying is_https() first

comment:6 Changed 7 years ago by SmileyChris

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

comment:7 Changed 7 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [6359]) Fixed #4710 -- Improved mod_python HTTPS checking. Thanks, Aaron Maxwell, SmileyChris and Graham Dumpleton.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.