Uploaded image for project: 'mod_python'
  1. mod_python
  2. MODPYTHON-94

Calling APR optional functions provided by mod_ssl

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.2.10, 3.3.1
    • Component/s: core
    • Labels:
      None
    • Environment:
      Apache 2

      Description

      mod_python is not able to invoke APR Optional Functions. There are
      some cases however where this could be of great benifit.

      For example, consider writing an authentication or authorization handler
      which needs to determine SSL properties (even if to just answer the
      simple question: is the connection SSL encrypted). The normal way of
      looking in the subprocess_env for SSL_* variables does not work in those
      early handler phases because those variables are not set until the fixup phase.

      The mod_ssl module though does provide both a ssl_is_https() and
      ssl_var_lookup() optional functions which can be used in earlier
      phases. For example look at how mod_rewrite calls those; using
      the APR_DECLARE_OPTIONAL_FN and APR_RETRIEVE_OPTIONAL_FN
      macros.

      I can see how it might be very hard to support optional functions in
      general because of the C type linkage issue, but perhaps a select few
      could be coded directly into mod_python.

      1. modpython4.tex.patch
        3 kB
        Deron Meranda
      2. requestobject.c.patch
        3 kB
        Deron Meranda

        Activity

        Hide
        dmeranda Deron Meranda added a comment -

        change to core component

        Show
        dmeranda Deron Meranda added a comment - change to core component
        Hide
        dmeranda Deron Meranda added a comment -

        This is a suggested patch against 3.2.5b which adds the ability to
        access a couple mod_ssl optional functions.

        I added two new methods to the request object.

        req.is_https()
        req.ssl_var()

        which will invoke the mod_ssl optional functions ssl_is_https()
        and ssl_var_lookup(). They safely handle the case where
        mod_ssl is not loaded or not available.

        They also work perfectly fine from say the PythonAccessHandler
        phase, whereas using subprocess_env does not.

        Some of the code was inspired from the mod_rewrite invocation
        of these same mod_ssl optional functions. Especially the part about
        declaring the function rather than including modssl.h.

        Show
        dmeranda Deron Meranda added a comment - This is a suggested patch against 3.2.5b which adds the ability to access a couple mod_ssl optional functions. I added two new methods to the request object. req.is_https() req.ssl_var() which will invoke the mod_ssl optional functions ssl_is_https() and ssl_var_lookup(). They safely handle the case where mod_ssl is not loaded or not available. They also work perfectly fine from say the PythonAccessHandler phase, whereas using subprocess_env does not. Some of the code was inspired from the mod_rewrite invocation of these same mod_ssl optional functions. Especially the part about declaring the function rather than including modssl.h.
        Hide
        dmeranda Deron Meranda added a comment -

        This is a documentation patch which goes with the previously attached code patch.
        Made against 3.2.5b.

        Show
        dmeranda Deron Meranda added a comment - This is a documentation patch which goes with the previously attached code patch. Made against 3.2.5b.
        Hide
        grahamd Graham Dumpleton added a comment -

        Can Apache be built without HTTPS support? Does this patch still work and does it gracefully fail or give negative results if this is the case? I would not want to see changes which require Apache to be built in a certain way.

        Also, it has previously been show how to provide this functionality as a separate Python loadable module so that patches would not be required to the mod_python core. See:

        http://www.modpython.org/pipermail/mod_python/2005-November/019609.html

        No time to look at this issue further at the moment, but recording my current concerns.

        Show
        grahamd Graham Dumpleton added a comment - Can Apache be built without HTTPS support? Does this patch still work and does it gracefully fail or give negative results if this is the case? I would not want to see changes which require Apache to be built in a certain way. Also, it has previously been show how to provide this functionality as a separate Python loadable module so that patches would not be required to the mod_python core. See: http://www.modpython.org/pipermail/mod_python/2005-November/019609.html No time to look at this issue further at the moment, but recording my current concerns.
        Hide
        davidfraser David Fraser added a comment -

        I wonder whether it would be possible to use a module like ctypes to connect to Apache functions. That way the linking issue is not a problem...

        Show
        davidfraser David Fraser added a comment - I wonder whether it would be possible to use a module like ctypes to connect to Apache functions. That way the linking issue is not a problem...
        Hide
        grahamd Graham Dumpleton added a comment -

        I thought about the ctypes approach when I proposed the first code I referenced. The problem was how you dealt with the complexity of setting up a call which had to refer to data within the internal mod_python request object C structure which wasn't otherwise visible at the Python layer through the mod_python Python request object. Eg.

        variable_value = ssl_var_lookup(
        request_object->request_rec->pool,
        request_object->request_rec->server,
        request_object->request_rec->connection,
        request_object->request_rec,
        variable_name);

        It may well be possible, but it seemed to me to be quite messy and quite prone to bugs/problems due to the disconnect between the C code and Python code. Ie., you change the C structure to add/change stuff and you have to remember to change any Python code which is somehow using ctypes and defining offsets within the structure. Seemed more trouble that it was worth.

        Show
        grahamd Graham Dumpleton added a comment - I thought about the ctypes approach when I proposed the first code I referenced. The problem was how you dealt with the complexity of setting up a call which had to refer to data within the internal mod_python request object C structure which wasn't otherwise visible at the Python layer through the mod_python Python request object. Eg. variable_value = ssl_var_lookup( request_object->request_rec->pool, request_object->request_rec->server, request_object->request_rec->connection, request_object->request_rec, variable_name); It may well be possible, but it seemed to me to be quite messy and quite prone to bugs/problems due to the disconnect between the C code and Python code. Ie., you change the C structure to add/change stuff and you have to remember to change any Python code which is somehow using ctypes and defining offsets within the structure. Seemed more trouble that it was worth.
        Hide
        dmeranda Deron Meranda added a comment -

        Graham said: "Can Apache be built without HTTPS support? Does this patch still work and does it gracefully fail or give negative results if this is the case? I would not want to see changes which require Apache to be built in a certain way. "

        Yes, the attached patches do reasonable things if mod_ssl is not available.
        Nothing else in Apache or other modules is affected. When mod_ssl is
        not available, then req.is_https() will return False, and the req.ssl_var() will
        always return None.

        As for any other way to do this in general, that would be great!

        However I had at least a real need to get to these specific SSL functions because
        there just is no other way to do this purely within mod_python; and I think it a
        reasonable expectation that an AuthHandler could at least determine if the
        connection is over SSL or not (I don't consider checking the port number a
        sufficient check).

        For completeness I also raised this issue with the mod_ssl folks to see if it could
        be solved on their side. They rejected it--see
        http://issues.apache.org/bugzilla/show_bug.cgi?id=37551

        Show
        dmeranda Deron Meranda added a comment - Graham said: "Can Apache be built without HTTPS support? Does this patch still work and does it gracefully fail or give negative results if this is the case? I would not want to see changes which require Apache to be built in a certain way. " Yes, the attached patches do reasonable things if mod_ssl is not available. Nothing else in Apache or other modules is affected. When mod_ssl is not available, then req.is_https() will return False, and the req.ssl_var() will always return None. As for any other way to do this in general, that would be great! However I had at least a real need to get to these specific SSL functions because there just is no other way to do this purely within mod_python; and I think it a reasonable expectation that an AuthHandler could at least determine if the connection is over SSL or not (I don't consider checking the port number a sufficient check). For completeness I also raised this issue with the mod_ssl folks to see if it could be solved on their side. They rejected it--see http://issues.apache.org/bugzilla/show_bug.cgi?id=37551
        Hide
        grahamd Graham Dumpleton added a comment -

        If we are going to do this, can we not truncate the ssl_var_lookup() function name?

        Ie., use req.ssl_var_lookup() and not req.ssl_var().

        Feel that the use of the exact name will make it more obvious to those who have used low level APIs of Apache previously as to what the intent is. The use of "lookup" also perhaps better suggests it is a read only value than a reference of some form that might able to be changed.

        Show
        grahamd Graham Dumpleton added a comment - If we are going to do this, can we not truncate the ssl_var_lookup() function name? Ie., use req.ssl_var_lookup() and not req.ssl_var(). Feel that the use of the exact name will make it more obvious to those who have used low level APIs of Apache previously as to what the intent is. The use of "lookup" also perhaps better suggests it is a read only value than a reference of some form that might able to be changed.
        Hide
        dmeranda Deron Meranda added a comment -

        Keeping the same mod_ssl name for this function is fine with me.

        Show
        dmeranda Deron Meranda added a comment - Keeping the same mod_ssl name for this function is fine with me.
        Hide
        grahamd Graham Dumpleton added a comment -

        The attached patch uses Python native bool type.

        + is_https = optfn_is_https && optfn_is_https(self->request_rec->connection);
        +
        + result = is_https ? Py_True : Py_False;
        +
        + Py_INCREF(result);
        + return result;

        If mod_python is to still support Python 2.2, which it looks like we are still because of Nokia work, then can't use the Python bool type yet as that was only added to Python 2.3.

        http://www.python.org/peps/pep-0285.html

        Show
        grahamd Graham Dumpleton added a comment - The attached patch uses Python native bool type. + is_https = optfn_is_https && optfn_is_https(self->request_rec->connection); + + result = is_https ? Py_True : Py_False; + + Py_INCREF(result); + return result; If mod_python is to still support Python 2.2, which it looks like we are still because of Nokia work, then can't use the Python bool type yet as that was only added to Python 2.3. http://www.python.org/peps/pep-0285.html
        Hide
        grahamd Graham Dumpleton added a comment -

        Changes as contained in requestobject.c.patch and modpython4.tex.patch applied to mod_python SVN trunk in revision 378837.

        Note though that req.ssl_var_lookup() used instead of req.ssl_var() as commented on already and req.is_https() changed to return integer instead of bool. Latter is in part because of lack of bool support in Python 2.2, but also because all other wrappers for Apache functions returning true/false still return an integer. Thus have made it consistent and if we are going to starting using bool type all should perhaps be changed to make them consistent.

        Finally, change allowed argument type for req.ssl_var_lookup() to string and disallowed passing of None. If None was allowed, the null pointer may cause internal Apache function to crash.

        No specific test added to test suite, as can't reasonably produce a generic test because of need to have SSL compiled into Apache and possible need to have a client side SSL certificate. I don't know this stuff, but if someone else can come up with a test then great.

        Show
        grahamd Graham Dumpleton added a comment - Changes as contained in requestobject.c.patch and modpython4.tex.patch applied to mod_python SVN trunk in revision 378837. Note though that req.ssl_var_lookup() used instead of req.ssl_var() as commented on already and req.is_https() changed to return integer instead of bool. Latter is in part because of lack of bool support in Python 2.2, but also because all other wrappers for Apache functions returning true/false still return an integer. Thus have made it consistent and if we are going to starting using bool type all should perhaps be changed to make them consistent. Finally, change allowed argument type for req.ssl_var_lookup() to string and disallowed passing of None. If None was allowed, the null pointer may cause internal Apache function to crash. No specific test added to test suite, as can't reasonably produce a generic test because of need to have SSL compiled into Apache and possible need to have a client side SSL certificate. I don't know this stuff, but if someone else can come up with a test then great.

          People

          • Assignee:
            grahamd Graham Dumpleton
            Reporter:
            dmeranda Deron Meranda
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development