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

Digest Authorization header causes bad request error.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.1.4
    • Fix Version/s: 3.3.1
    • Component/s: publisher
    • Labels:
      None

      Description

      If Apache is used to perform authentication, the Authorization header still gets
      passed through to mod_python.publisher. Unfortunately, mod_python.publisher
      authentication code in process_auth() will attempt to decode the contents of the
      Authorization header even if there are no _auth_ or _access_ hooks defined
      for authentication and access control within the published code itself.

      The consequence of this is that if Digest authentication is used for AuthType
      at level of Apache authentication, the process_auth() code will raise a bad request
      error as it assumes Authorization header is always in format for Basic authentication
      type and when it can't decode it, it raises an error.

      What should happen is that any decoding of Authorization should only be done
      if there is a _auth_ or _access_ hook that actually requires it. That way, if some
      one uses Digest authentication at Apache configuration file level, provided that no
      _auth_ or _access_ hooks are provided, there wouldn't be a problem.

      See:

      http://www.modpython.org/pipermail/mod_python/2005-April/017911.html
      http://www.modpython.org/pipermail/mod_python/2005-April/017912.html

      for additional information.

      1. MP47_20060307_grahamd_1.diff
        0.8 kB
        Graham Dumpleton
      2. MP47_20060309_grahamd_2.diff
        2 kB
        Graham Dumpleton

        Activity

        Hide
        grahamd Graham Dumpleton added a comment -

        This issue was also raised a few years ago on the mailing list. Relevant mailing list
        post at the time with suggested fix was:

        http://www.modpython.org/pipermail/mod_python/2002-September/013071.html

        The suggested fix at the time was to set user,passwd to None,None with similar
        check as initially suggested this time round.

        In vampire::publisher where similar problem existed, was found to be better to
        defer parsing of Authorization until determined it was needed though as it then
        mean't you got a hard error if actually trying to use _auth_ or _access_ instead
        of it being silently ignored. Ie., error would occur still where you were trying to
        mix two different authorisation types.

        Show
        grahamd Graham Dumpleton added a comment - This issue was also raised a few years ago on the mailing list. Relevant mailing list post at the time with suggested fix was: http://www.modpython.org/pipermail/mod_python/2002-September/013071.html The suggested fix at the time was to set user,passwd to None,None with similar check as initially suggested this time round. In vampire::publisher where similar problem existed, was found to be better to defer parsing of Authorization until determined it was needed though as it then mean't you got a hard error if actually trying to use _ auth _ or _ access _ instead of it being silently ignored. Ie., error would occur still where you were trying to mix two different authorisation types.
        Hide
        grahamd Graham Dumpleton added a comment -

        As usual there is nearly always a way to fudge things. You could still use Apache HTTP digest authentication (managed by Apache) and still use mod_python.publisher by having an authenhandler() or earlier content handler which deleted the "Authorization" header so that mod_python.publisher didn't find it and therefore didn't barf.

        def authenhandler(req):

        if req.headers_in.has_key("Authorization"):
        del req.headers_in["Authorization"]

        ... etc.

        I haven't tried this, but it should work.

        Show
        grahamd Graham Dumpleton added a comment - As usual there is nearly always a way to fudge things. You could still use Apache HTTP digest authentication (managed by Apache) and still use mod_python.publisher by having an authenhandler() or earlier content handler which deleted the "Authorization" header so that mod_python.publisher didn't find it and therefore didn't barf. def authenhandler(req): if req.headers_in.has_key("Authorization"): del req.headers_in ["Authorization"] ... etc. I haven't tried this, but it should work.
        Hide
        grahamd Graham Dumpleton added a comment -

        The simplest way of fixing this problem may be that after changes related to MODPYTHON-124 are made that the publisher simply not try and authenticate users if req.ap_auth_type is not None.

        In other words, if AuthType directive has been defined assume that something else is handling authentication and that publisher doesn't have to worry about it. This will mean publisher will not redundantly decode authorisation header if AuthType was Basic and was being handled explicitly by mod_auth in Apache and outside of the publisher.

        Thus, insert at start of process_auth():

        if req.ap_auth_type:
        return realm, user, passwd

        Show
        grahamd Graham Dumpleton added a comment - The simplest way of fixing this problem may be that after changes related to MODPYTHON-124 are made that the publisher simply not try and authenticate users if req.ap_auth_type is not None. In other words, if AuthType directive has been defined assume that something else is handling authentication and that publisher doesn't have to worry about it. This will mean publisher will not redundantly decode authorisation header if AuthType was Basic and was being handled explicitly by mod_auth in Apache and outside of the publisher. Thus, insert at start of process_auth(): if req.ap_auth_type: return realm, user, passwd
        Hide
        grahamd Graham Dumpleton added a comment -

        Attached "MP47_20060307_grahamd_1.diff" with proposed patch. Feedback on this one appreciated.

        Show
        grahamd Graham Dumpleton added a comment - Attached "MP47_20060307_grahamd_1.diff" with proposed patch. Feedback on this one appreciated.
        Hide
        jgallacher Jim Gallacher added a comment -

        I'm don't really like this patch (MP47_20060307_grahamd_1.diff) as it bypasses any authorization or access checks in the object being published.

        Authentication != authorization

        I'm not sure what the answer is, but I think we all know that the problem lies in the fact that publisher is too tightly bound to basic authentication.

        Show
        jgallacher Jim Gallacher added a comment - I'm don't really like this patch (MP47_20060307_grahamd_1.diff) as it bypasses any authorization or access checks in the object being published. Authentication != authorization I'm not sure what the answer is, but I think we all know that the problem lies in the fact that publisher is too tightly bound to basic authentication.
        Hide
        grahamd Graham Dumpleton added a comment -

        Attached alternate suggested fix as "MP47_20060309_grahamd_2.diff".

        I should have just done it this way to begin with.

        What this does is simply not try and do anything with the Authorization header unless either _auth_ or _access are actually found. Ie., the fix I originally described, rather than my latter obsession with req.ap_auth_type.

        This means that if one has _auth_ or _access_ and Apache configuration still uses digest authentication it will fail. But if one knows one is using digest authentication support in mod_auth, you would not be using the publisher auth support anyway as they would conflict.

        I agree that authentication should not be part of publisher, but can't do anything about that now. Apache has a really good concept of phases yet just about everything merely uses mod_python as a hopping off point at content handler phase and does everything in that one phase.

        Show
        grahamd Graham Dumpleton added a comment - Attached alternate suggested fix as "MP47_20060309_grahamd_2.diff". I should have just done it this way to begin with. What this does is simply not try and do anything with the Authorization header unless either _ auth _ or _ access are actually found. Ie., the fix I originally described, rather than my latter obsession with req.ap_auth_type. This means that if one has _ auth _ or _ access _ and Apache configuration still uses digest authentication it will fail. But if one knows one is using digest authentication support in mod_auth, you would not be using the publisher auth support anyway as they would conflict. I agree that authentication should not be part of publisher, but can't do anything about that now. Apache has a really good concept of phases yet just about everything merely uses mod_python as a hopping off point at content handler phase and does everything in that one phase.
        Hide
        jgallacher Jim Gallacher added a comment -

        The second patch is likely the best compromise.

        I think part of the problem with process_auth() is the uncertainty of meaning associated with auth and _auth. Does it mean authenticate or authorize? If it's authorize, then there is no reason to call __auth_ with the password. Likewise, you shouldn't need the user name when calling _access_. One could certainly come up with access rules which are not tied to a specific user.

        Maybe we should consider refactoring process_auth() to look for 2 new attributes such as _authen_ and _author, and deprecate the use of __auth_. If somebody wants to do authentication in publisher then their code would be completely responsible for returning either a user or raising an error. (Further discussion of this should be shifted out of this particular issue).

        Show
        jgallacher Jim Gallacher added a comment - The second patch is likely the best compromise. I think part of the problem with process_auth() is the uncertainty of meaning associated with auth and _ auth . Does it mean authenticate or authorize? If it's authorize, then there is no reason to call __auth _ with the password. Likewise, you shouldn't need the user name when calling _ access _. One could certainly come up with access rules which are not tied to a specific user. Maybe we should consider refactoring process_auth() to look for 2 new attributes such as _ authen _ and _ author , and deprecate the use of __auth _. If somebody wants to do authentication in publisher then their code would be completely responsible for returning either a user or raising an error. (Further discussion of this should be shifted out of this particular issue).

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development