Uploaded image for project: 'Spring Security OAuth'
  1. Spring Security OAuth
  2. SECOAUTH-56

Certain characters in signature are not validating properly

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Critical
    • Resolution: Complete
    • Affects Version/s: 1.0.0.M3
    • Fix Version/s: 1.0.0
    • Component/s: OAuth 1
    • Labels:
      None
    • Environment:
      Mac OS X 10.6.7, Java 6

      Description

      There seems to be an issue where certain characters in the signature are creating validation to fail. Here is an example (using HMAC-SHA1 for signature method):

      Signature as generated on the consumer side (using CoreOAuthConsumerSupport to make the call): KoCR1Z1PeM/+sCoptySENMEh2xw=
      Signature as generated on the provider side: KoCR1Z1PeM/ sCoptySENMEh2xw=

      Notice how the + is missing on the provider side. My guess is this has something to do with URLEncoding/Decoding. This is difficult to replicate since the signature needs a + sign in it after generation.

        Activity

        Hide
        atberman Andrew Berman added a comment -

        A little bit more information:

        1. It happens when the authentication information is NOT put in the header and with both GET (auth info as query params) and POST (auth info as body) methods
        2. It only happens when there is a + sign in the signature

        Show
        atberman Andrew Berman added a comment - A little bit more information: 1. It happens when the authentication information is NOT put in the header and with both GET (auth info as query params) and POST (auth info as body) methods 2. It only happens when there is a + sign in the signature
        Hide
        atberman Andrew Berman added a comment -

        Ok, so after debugging more, the bug is in CoreOAuthConsumerSupport. The query string and body are not being URL escaped properly and as a result the plus sign is not being encoded to %2B (it's being left as a + sign since that character is valid in a URL) where upon calling request.getParameter in CoreOAuthProviderSupport, it yields a space instead of the + sign.

        Show
        atberman Andrew Berman added a comment - Ok, so after debugging more, the bug is in CoreOAuthConsumerSupport. The query string and body are not being URL escaped properly and as a result the plus sign is not being encoded to %2B (it's being left as a + sign since that character is valid in a URL) where upon calling request.getParameter in CoreOAuthProviderSupport, it yields a space instead of the + sign.
        Hide
        rheaton Ryan Heaton added a comment -

        Thanks for the report. Well take a look.

        Show
        rheaton Ryan Heaton added a comment - Thanks for the report. Well take a look.
        Hide
        rheaton Ryan Heaton added a comment - - edited

        Finally getting around to looking at this, but I think I'm going to need a test case or something. From the description of the problem, it's not clear to me where the fix needs to be applied. If the invalid signature only happens when using GET or POST, it might be a bug on the provider-side, parsing a '+' as a ' ' when it shouldn't. It could also be a problem at the time that the request to the provider is prepared, where the '+' needs to be encoded as a %2B.

        What's the suggested fix?

        Show
        rheaton Ryan Heaton added a comment - - edited Finally getting around to looking at this, but I think I'm going to need a test case or something. From the description of the problem, it's not clear to me where the fix needs to be applied. If the invalid signature only happens when using GET or POST, it might be a bug on the provider-side, parsing a '+' as a ' ' when it shouldn't. It could also be a problem at the time that the request to the provider is prepared, where the '+' needs to be encoded as a %2B. What's the suggested fix?
        Hide
        david_syer Dave Syer added a comment -

        Merged a pull request.

        Show
        david_syer Dave Syer added a comment - Merged a pull request.

          People

          • Assignee:
            david_syer Dave Syer
            Reporter:
            atberman Andrew Berman
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development