CouchDB
  1. CouchDB
  2. COUCHDB-1144

oauth requests with non-percent-encoded realms result in function_clause error in HTTP request

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.0.2, 1.1
    • Fix Version/s: 1.0.3, 1.1
    • Component/s: HTTP Interface
    • Labels:
      None
    • Environment:

      OSX - branch 1.1 - r1095237

      Description

      As illustrated in this Wireshark packet dump, lines 53ff, any OAuth request with a non-percent-encoded URL as the realm will return a 500 error:
      http://friendpaste.com/3vXPjHP6s7dLZjsj7DOfSH

      Erlang stacktrace is similar to:

      [error] [<0.189.0>] function_clause error in HTTP request [Wed, 27 Apr 2011 23:31:46 GMT] [info] [<0.189.0>] Stacktrace:
      [

      {oauth_uri,decode, ["://127.0.0.1:5984","ptth"]}

      ,

      {oauth_uri,param_from_header_string,1}

      ,

      {oauth_uri, '-params_from_header_string/1-lc$^0/1-0-', 1}

      ,

      {couch_httpd_oauth,serve_oauth,3}

      ,

      {couch_httpd,authenticate_request,2}

      ,

      {couch_httpd,handle_request_int,5}

      ,

      {mochiweb_http,headers,5}

      ,

      {proc_lib,init_p_do_apply,3}

      ] [Wed, 27 Apr 2011 23:31:46 GMT] [info] [<0.189.0>] 127.0.0.1 - - 'PUT'
      /test_c_project/16f74c25-d641-4710-8f38-18295a8a69b1 500

      Chatted with benoitc today and he suggested this may be because the realm is not being properly encoded before being passed on.

      By default, some OAuth libraries such as python's popular oauth2 library always set the realm of a request to the URL of the resource being accessed, such as http://localhost:5984/ . (In fact, oauth2 library does not support overriding this setting.) The OAuth spec @ http://oauth.net/core/1.0/ also shows realms specified as URLs. RFC5849 states "The OPTIONAL "realm" parameter MAY be added and interpreted per [RFC2617] section 1.2.", which in turn says the realm is any quoted-string.

      It seems that this may already be fixed in trunk simply because trunk has a newer version of the upstream erlang-oauth.

      A JS test could be added in futon to validate this by modifying function oauthRequest to accept a realm parameter, then passing that down to OAuth.getAuthorizationHeader on line 56.

      1. oauth-realm-test.patch
        1 kB
        Joan Touzet
      2. oauth_uri.erl
        3 kB
        Joan Touzet

        Activity

        Hide
        Joan Touzet added a comment -

        CouchDB only errors when the OAuth request is malformed (not percent-encoded). python-oauth2 (simplegeo) doesn't percent-encode its OAuth realm. We're pushing upstream to fix that.

        It's interesting that the newer version of erlang-oauth in trunk handles the malformed request successfully, without the stacktrace.

        I'll reduce this to a minor issue, upload a .diff to the JS tests that shows the issue, and suggest that, if possible, CouchDB be modified not to 500 on these malformed requests.

        Show
        Joan Touzet added a comment - CouchDB only errors when the OAuth request is malformed (not percent-encoded). python-oauth2 (simplegeo) doesn't percent-encode its OAuth realm. We're pushing upstream to fix that. It's interesting that the newer version of erlang-oauth in trunk handles the malformed request successfully, without the stacktrace. I'll reduce this to a minor issue, upload a .diff to the JS tests that shows the issue, and suggest that, if possible, CouchDB be modified not to 500 on these malformed requests.
        Hide
        Joan Touzet added a comment -

        Applying this patch to 1.0.2/1.1 will cause the oauth test to fail. As this patch makes OAuth "do the wrong thing" by not percent-encoding the OAuth realm parameter, it is probably not suitable for inclusion long-term.

        Show
        Joan Touzet added a comment - Applying this patch to 1.0.2/1.1 will cause the oauth test to fail. As this patch makes OAuth "do the wrong thing" by not percent-encoding the OAuth realm parameter, it is probably not suitable for inclusion long-term.
        Hide
        Joan Touzet added a comment - - edited

        After chatting with davisp in IRC, we agree that the bug is in erlang-oauth/src/oauth_uri.erl . A patch against 1.0.x / 1.1 is attached; it is the oauth_uri.erl from trunk/upstream. Paired code inspection (with davisp) shows no variance in the exported functions, so this is a dropin replacement.

        I respectfully request including this in 1.1.0.

        Show
        Joan Touzet added a comment - - edited After chatting with davisp in IRC, we agree that the bug is in erlang-oauth/src/oauth_uri.erl . A patch against 1.0.x / 1.1 is attached; it is the oauth_uri.erl from trunk/upstream. Paired code inspection (with davisp) shows no variance in the exported functions, so this is a dropin replacement. I respectfully request including this in 1.1.0.
        Hide
        Joan Touzet added a comment -

        Patch to fix this in 1.0.x/1.1.

        Show
        Joan Touzet added a comment - Patch to fix this in 1.0.x/1.1.
        Hide
        Paul Joseph Davis added a comment -

        Crux of the issue is that oauth_uri:decode in 1.1.x is much more strict than in trunk. Trunk version just de-hexes any %HH encodings, where as 1.1.x asserts that non %HH sequences match [a-zA-Z0-9-_.~]

        Show
        Paul Joseph Davis added a comment - Crux of the issue is that oauth_uri:decode in 1.1.x is much more strict than in trunk. Trunk version just de-hexes any %HH encodings, where as 1.1.x asserts that non %HH sequences match [a-zA-Z0-9-_.~]
        Hide
        Paul Joseph Davis added a comment -

        Anyone have an issue with me committing this?

        Show
        Paul Joseph Davis added a comment - Anyone have an issue with me committing this?
        Hide
        Robert Newson added a comment -

        +1. Let's get it in.

        Show
        Robert Newson added a comment - +1. Let's get it in.
        Hide
        Benoit Chesneau added a comment -

        voted with the nice button on top of this ticket

        Show
        Benoit Chesneau added a comment - voted with the nice button on top of this ticket
        Hide
        Robert Newson added a comment -

        This is happening in 1.1.0.

        Show
        Robert Newson added a comment - This is happening in 1.1.0.

          People

          • Assignee:
            Unassigned
            Reporter:
            Joan Touzet
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development