Missing decode on client basic authentication

[strehle]

SECURITY NOTICE: If you have found a security problem in the UAA, please do not file a public github issue. Instead, please send an email to [email protected]

Thanks for taking the time to file an issue. You'll minimize back and forth and help us help you more effectively by answering all of the following questions as specifically and completely as you can.

What version of UAA are you running?

4.10.0

How are you deploying the UAA?

• locally only using gradlew
• using cf-release
• using cf-deployment

What did you do?

1. create an oauth client with special chars or secret with special chars
   e.g. use | and in console you need to escape it with \

uaac client add app\|client --autoapprove true --redirect_uri http://localhost:8080/uaa/app --authorized_grant_types password -s secret --authorities uaa.resource --scope openid,uaa.user

2. UAAC does not implement encode,
   uaac token owner get app|client marissa -s secret -p koala -t

works, however if you use curl with correct header

curl -i -HPOST -H"content-type: application/x-www-form-urlencoded;charset=utf-8" -H"authorization: Basic YXBwJTdDY2xpZW50OnNlY3JldA==" -H"Accept: application/json" http://localhost:8080/uaa/oauth/token -d "grant_type=password&username=marissa&password=koala"

does not work

What did you expect to see? What goal are you trying to achieve with the UAA?

successfull token request on

curl -i -HPOST -H"content-type: application/x-www-form-urlencoded;charset=utf-8" -H"authorization: Basic YXBwJTdDY2xpZW50OnNlY3JldA==" -H"Accept: application/json" http://localhost:8080/uaa/oauth/token -d "grant_type=password&username=marissa&password=koala"

What did you see instead?

{"error":"unauthorized","error_description":"Bad credentials"}

---

See:
https://tools.ietf.org/html/rfc6749#section-2.3
The authorization header needs to be
Authorization: Basic base64Encode(urlencode(client_id):urlencode(client_secret))

The go client library implements this rfc but requests fail if client_id/secret contains special chars.
https://github.com/golang/oauth2/blob/master/internal/token.go#L191

Incorrect validation is in
a) UAA code.
https://github.com/cloudfoundry/uaa/blob/master/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/ClientBasicAuthenticationFilter.java#L105
b) spring-security-web, class
BasicAuthenticationFilter , which contains similar pattern and missing urldecode

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/155465478

The labels on this github issue will be updated when the story is started.

[aeijdenberg]

Yes, I've also noted this in past with regards to the cf CLI client not correctly following the RFC in this regard, however in that PR they chose to disregard the patch we provided and instead continue the non-compliant behaviour.

[strehle]

ok, I recommend, that you reopen it. the fix should begin with uaa, but then the clients in 2nd steps, e.g. cli and uaac (see cloudfoundry/cf-uaac#50)
@sreetummidi UAA wants to get certified for OIDC and this behaviour should be done before a certification

[DennisDenuto]

Thanks for creating this issue, reading rfc6749 I think you are correct. However the uaa heavily relies on spring filters to provide Basic Auth functionality. Specifically as part of our filter chain we delegate to BasicAuthenticationFilter.

This filter in turn adheres to rfc1945 and correctly does not url-decode the base64 token (https://github.com/spring-projects/spring-security/blob/master/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java#L224)

FYI rfc1945 is dated May 1996 while rfc6749 is dated Oct 2012.

I think this issue is valid however I think it belongs in https://github.com/spring-projects/spring-security/issues . If this change is made upstream I think we should be able to upgrade uaa to also conform to rfc6749.

EDIT: I just realised that they are different specs. rfc1945 is a spec for http/1.0 and rfc6749 is a spec for oauth

[aeijdenberg]

@DennisDenuto - I'm confused by your comment.

RFC6749 does not supersede or replace RFC1945 - they both describe different layers of a stack.

e.g. RFC1945 describes how to encode a user ID and password and put into an authorization header.

RFC6749 describes pre-processing that should only apply in the context of OAuth 2.0 that should take place before applying RFC1945.

So it would be quite incorrect to change the behaviour of a common library that implements RFC1945.

Rather an application (ie UAA) that implements OAuth 2.0 needs to pass the right inputs to it (or if the API surface doesn't permit, then reimplement correctly).

This needs to be either fixed in UAA, or clearly documented as a known variation from the OAuth 2.0 specification, the security impact assessed and if appropriate operators warned accordingly.

[DennisDenuto]

@aeijdenberg Thanks for your reply. Apologies I realized my mistake after posting.

IMO ideally the uaa would leverage existing spring code from spring-security oauth to handle this. Looking at the spring TokenEndpointAuthenticationFilter that the uaa uses, It seems to imply that the oauth token filter should be used with BasicAuthenticationFilter which does not url-encode.

The uaa extending the BasicAuthenticationFilter to correctly url-encode is one option worth considering.

It also isn't clear to me what the security risks here are?

Lastly a background concern is backwards compatibility of uaa clients. i.e. the cf cli, cf uaac

@jhamon @tnwang thoughts?

[idoru]

Hi, we just encountered this problem, while using the om library (from pivotal-cf/om) which basically uses the golang oauth library (golang.org/x/oauth2).

This library assumes that the server implements RFC6749 faithfully, in particular 2.3.1 where the client id and secret are URL encoded before being later Base64 encoded at the RFC1945 layer.

It only avoids doing this for the special cases which are detected by URL https://github.com/golang/oauth2/blob/3d292e4d0cdc3a0113e6d207bb137145ef1de42f/internal/token.go#L93

It's probably worth noting the list of domains at the linked code is a pretty clear indictment of how many folks are in non-compliance. I wouldn't consider this a justification for not doing things the right way, and adding to this list isn't a viable workaround since UAA could be at any address.

This could preclude users of UAA from using standard oauth libraries to communicate with UAA, increasing the complexity of communicating with UAA which reduces the value of using it as an OAuth implementation. For example, om would likely need to re-implement it's authentication to be workable with UAA.

/cc @drich10

[heikoettelbruecksap]

I just stumbled across this issue while evaluating usage of UAA for login to Kubernetes, e.g. via https://github.com/int128/kubelogin. kubelogin, as well as probably other OIDC tools for Kubernetes implemented in go, use the https://github.com/coreos/go-oidc library, which internally the golang oauth library that idoru mentioned above.
=> In our case, we have generated client secrets (and IDs) with special characters (like "=" as padding in base64 encoded client secrets), so this issue in UAA (or its usage of Spring libs) is actually a blocker. I assume basically it's a general blocker for usage of UAA for OIDC login to Kubernetes, as soon as it's not possible/feasible to restrict client IDs and secret to non-special characters.
/cc @strehle FYI, just learned that you already reported this quite some time ago

[strehle]

@heikoettelbruecksap yes and this is a long story.

CF tools by default use authorization header - also cli uses this and the different encoding of authorization header breaks authentication, but cli does not use the standard oauth2 implementation.
... standard means use of "golang.org/x/oauth2"

kube tools rely on standard oauth2 implementation library and here by default the authorization header is encoded in different way, or better decribed in the correct way ;)

However, the workaround would be that authentication is done via POST body.golang.org/x/oauth2 library in go has an option for this, see call oauth2.RegisterBrokenAuthHeaderProvider, e.g. another PR
https://github.com/cloudfoundry-incubator/uaa-cli/blob/c2690b0bc4ba5d154e8175dc87b9fdb3be9d408d/cmd/get_client_credentials_token.go#L34
usese this.

So may its faster to provide an option in https://github.com/coreos/go-oidc/blob/v2/oidc.go#L84-L89 for switching to body parameters instead of authorization header.

[aeijdenberg]

It's interesting - until encountering UAA I wasn't even aware that the OAuth spec contemplated passing the client ID/secret via an Authorization header. All previous usages I'd seen had alway been by doing so in the body, which has the advantage of well-defined and consistent escaping semantics.

I'll note also a few weeks ago that Google managed to break their OAuth token issuance, but only for those doing so via the Authorization header (see: https://twitter.com/AdamEijdenberg/status/1045197676759769088) suggesting this is not a terribly common pattern for their consumers also.

So yes, I'd suggest simply passing via the body - that seems to be a more standard approach to OAuth outside of the CF / UAA space.

[aeijdenberg]

and yes, UAA should still fix this.

[heikoettelbruecksap]

I proposed a respective change in the oauth2 library for go, switching from basic authentication header to POST parameters, see golang/oauth2#320.

[shamus]

Closing due to inactivity.