Spring Security OAuth
  1. Spring Security OAuth
  2. SECOAUTH-56

Certain characters in signature are not validating properly

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical 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
        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
        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
        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
        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
        Ryan Heaton added a comment -

        Thanks for the report. Well take a look.

        Show
        Ryan Heaton added a comment - Thanks for the report. Well take a look.
        Hide
        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
        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
        Dave Syer added a comment -

        Merged a pull request.

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

          People

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

            Dates

            • Created:
              Updated:
              Resolved: