Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In OIDC verify that the JWT token type is "Bearer" #643

Closed
mayorova opened this issue Mar 1, 2018 · 13 comments
Closed

In OIDC verify that the JWT token type is "Bearer" #643

mayorova opened this issue Mar 1, 2018 · 13 comments

Comments

@mayorova
Copy link
Contributor

mayorova commented Mar 1, 2018

Currently APIcast will just check the type of token in the Authorization header, so requests like this will fail:

curl -v "http://localhost:8080/test" -H "Authorization: Refresh <JWT Access Token>"

However, it is possible to pass a token which is NOT a bearer token like this:

curl -v "http://localhost:8080/test" -H "Authorization: Bearer <JWT Refresh Token>"

The request will succeed, as the refresh token has the claims required by APIcast. The same with ID token. However, this doesn't sound right.

Red Hat Single Sign-On includes the claim "typ" to identify the type of token

It doesn't seem to be included in the specifications:

However, Red Hat Single Sign-On itself validates the type of the token, and in case it is not correct, it returns a:

{
    "error": "invalid_token",
    "error_description": "Invalid type of token"
}

I think APIcast should also check that the value of the "typ" claim is Bearer and the token is send as Bearer in the Authorization header.

@mikz
Copy link
Contributor

mikz commented Mar 1, 2018

Would be good to provide examples for those tokens (decoded and sanitized are fine).

Also I'll need a spec/RFC to follow. If this is not specified anywhere, then how other implementations work? We need to follow some specified standard.

@mayorova
Copy link
Contributor Author

mayorova commented Mar 1, 2018

Example tokens:

Bearer

eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJiTTFLVFFCeHhoMmRaYTVLLXFzZ1ZZVk1tZTJkLUhSU28zVGNmNDRacVE0In0.eyJqdGkiOiJlYmE2ZTNiNi0xM2M3LTRmNmUtYTFiMC0yZWJiMmJiZmI5NmEiLCJleHAiOjE1MTk5MTQxMjQsIm5iZiI6MCwiaWF0IjoxNTE5OTEzODI0LCJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjgwODAvYXV0aC9yZWFsbXMvb2lkYyIsImF1ZCI6InRlc3QiLCJzdWIiOiJhMTY2N2ZhMy1jMTczLTQ1Y2QtYWE2Mi1iZDlhN2M5ZjczNDAiLCJ0eXAiOiJCZWFyZXIiLCJhenAiOiJ0ZXN0IiwiYXV0aF90aW1lIjowLCJzZXNzaW9uX3N0YXRlIjoiZjNiOGZlZmMtNGViMy00M2VlLWE0NGUtMWJiZmU4ZTAwMzc0IiwiYWNyIjoiMSIsImNsaWVudF9zZXNzaW9uIjoiYjJmMDdlZjctN2UzNS00NjFlLTk1MTgtODg4YTNkYjUzZTJkIiwiYWxsb3dlZC1vcmlnaW5zIjpbXSwicmVhbG1fYWNjZXNzIjp7InJvbGVzIjpbInVtYV9hdXRob3JpemF0aW9uIl19LCJyZXNvdXJjZV9hY2Nlc3MiOnsiYWNjb3VudCI6eyJyb2xlcyI6WyJtYW5hZ2UtYWNjb3VudCIsInZpZXctcHJvZmlsZSJdfX0sImNsaWVudElkIjoidGVzdCIsImNsaWVudEhvc3QiOiIxNzIuMTcuMC4xIiwibmFtZSI6IiIsInByZWZlcnJlZF91c2VybmFtZSI6InNlcnZpY2UtYWNjb3VudC10ZXN0IiwiY2xpZW50QWRkcmVzcyI6IjE3Mi4xNy4wLjEiLCJlbWFpbCI6InNlcnZpY2UtYWNjb3VudC10ZXN0QHBsYWNlaG9sZGVyLm9yZyJ9.Nd-d2xIm3oHOypkny99RgpUudq5jlZu9Qu9KCp65x3ML0yO3_spE646SpmfD54joSLpH3KOm2LK9hRjrqLCclvYhW5B-dNLe0vDNvcIgKJ1g9HJUtMCC42Bd6WiNR6zu5LSynXBsFnOHgyg4uEUnTCW_i6gAnwjeaobJsTzqHjvOyVUMRwItdFNtKgUPUkmYDPS49iSziQvFWPw9GKb1Q0d42Yt3pdAwCCDDcwPvlbrIlgHhDPgy7ORXW3JY-WlZUaRmiMp979kHOUqKkBarCjKNjggExQm_xsB8rbK44_FnmfM5fm8Ztg28TnnNgw7_4CRe1_Jr9ObsH9zwoiC0rA

Refresh:

eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJiTTFLVFFCeHhoMmRaYTVLLXFzZ1ZZVk1tZTJkLUhSU28zVGNmNDRacVE0In0.eyJqdGkiOiJmYjI3NTJlNC02NDc3LTQwODgtODg4NC1hYThlNTY5ZDVlMjEiLCJleHAiOjE1MTk5MTU2MjQsIm5iZiI6MCwiaWF0IjoxNTE5OTEzODI0LCJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjgwODAvYXV0aC9yZWFsbXMvb2lkYyIsImF1ZCI6InRlc3QiLCJzdWIiOiJhMTY2N2ZhMy1jMTczLTQ1Y2QtYWE2Mi1iZDlhN2M5ZjczNDAiLCJ0eXAiOiJSZWZyZXNoIiwiYXpwIjoidGVzdCIsImF1dGhfdGltZSI6MCwic2Vzc2lvbl9zdGF0ZSI6ImYzYjhmZWZjLTRlYjMtNDNlZS1hNDRlLTFiYmZlOGUwMDM3NCIsImNsaWVudF9zZXNzaW9uIjoiYjJmMDdlZjctN2UzNS00NjFlLTk1MTgtODg4YTNkYjUzZTJkIiwicmVhbG1fYWNjZXNzIjp7InJvbGVzIjpbInVtYV9hdXRob3JpemF0aW9uIl19LCJyZXNvdXJjZV9hY2Nlc3MiOnsiYWNjb3VudCI6eyJyb2xlcyI6WyJtYW5hZ2UtYWNjb3VudCIsInZpZXctcHJvZmlsZSJdfX19.fHSom4IJIOCJST0S_GPpP7C-XN_8FRzxUPagzZPVxgY8I4CVgPZSCxlXXia15fWAcYgeSAWyyapu8qmgQv_ZduLtuWV-TnAGegf-ofq0U80kVy100ZNm5IztkO3oeJLgOSeYImXuZp5PHnf3Na0Pqv9z-sJT6LZ6o_J5aA7e_ud5O_jqBCNT5qgHk7uamwUt6Q5MzuNd5LUQ6aRCEaNTqYUnQlyr6JTCY6lLB4CAVOQSjwzvzaP79GuNuyr3lkynZZlkPPJdQ4takrbheu8bnuJYbrfWMfm2DYP-f7RbKvQ5cgtm_ePo8x3ECXgog4N9N4WTlj8mQTLT0sH60TIOzQ

ID:

eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJiTTFLVFFCeHhoMmRaYTVLLXFzZ1ZZVk1tZTJkLUhSU28zVGNmNDRacVE0In0.eyJqdGkiOiJhM2M4ZGM0MC1lMzU4LTRiMDQtODU4Mi1kMDVkNWQzY2YzZDciLCJleHAiOjE1MTk5MTQxMjQsIm5iZiI6MCwiaWF0IjoxNTE5OTEzODI0LCJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjgwODAvYXV0aC9yZWFsbXMvb2lkYyIsImF1ZCI6InRlc3QiLCJzdWIiOiJhMTY2N2ZhMy1jMTczLTQ1Y2QtYWE2Mi1iZDlhN2M5ZjczNDAiLCJ0eXAiOiJJRCIsImF6cCI6InRlc3QiLCJhdXRoX3RpbWUiOjAsInNlc3Npb25fc3RhdGUiOiJmM2I4ZmVmYy00ZWIzLTQzZWUtYTQ0ZS0xYmJmZThlMDAzNzQiLCJhY3IiOiIxIiwiY2xpZW50SWQiOiJ0ZXN0IiwiY2xpZW50SG9zdCI6IjE3Mi4xNy4wLjEiLCJuYW1lIjoiIiwicHJlZmVycmVkX3VzZXJuYW1lIjoic2VydmljZS1hY2NvdW50LXRlc3QiLCJjbGllbnRBZGRyZXNzIjoiMTcyLjE3LjAuMSIsImVtYWlsIjoic2VydmljZS1hY2NvdW50LXRlc3RAcGxhY2Vob2xkZXIub3JnIn0.CnB7pUQEJ4dwcmVN_yu5MtMkvzQVI6BhAq4eSlIIjiMtWPjSly7q8sE_IeoXjGnI9cPYVBu214AYg2XQ9OLJc0fX_ShGEPCSj3IAyHExkNXpu9NG85PK-yCL2DreDYY45ItFaEcYTEL5ansjih5uUV4oWn4Cn0gnNQ5As-MhZZ9yczlRnbdrtwqOV6d-zCOX3Ky-6vRLPnHHzN8p63JThIZRowzJjUablQQOj6Vb1NdU3KpYI7Wx6USXg4aE68hL2LIZQ4JTtsJnCiWtXVKR2ro9y5A3b9OAwJGiXriCkL5SaznojdUG88bb3zRmxSEYcryq2kPyKk-PU2fsxX57lg

@eye0fra
Copy link

eye0fra commented Mar 2, 2018

@mikz you have to add the following validation inside the jwt_claims on oidc.lua

typ = jwt_validators.equals('Bearer')

@mikz
Copy link
Contributor

mikz commented Mar 5, 2018

So I did some research and I think we should be using aud field.

The typ does not appear in any of the OIDC/OAuth2 specs:
http://openid.net/specs/openid-connect-core-1_0.html
https://tools.ietf.org/html/rfc6750
https://tools.ietf.org/html/draft-ietf-oauth-jwt-bearer-12
https://tools.ietf.org/html/draft-ietf-oauth-assertions-18

But I got the impression that aud field should be used from:
https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-token-and-claims
https://auth0.com/docs/api-auth/tutorials/adoption/api-tokens
https://stackoverflow.com/questions/28418360/jwt-json-web-token-audience-aud-versus-client-id-whats-the-difference

edit:
However, because it is not a standard some IDPs use different fields:
https://developer.okta.com/blog/2017/08/01/oidc-primer-part-3 (token_type is Bearer aud is the client id) (very useful blog series btw)

That makes me thing if we should validate the tokens by calling the IDP introspect endpoint. Because looks like the access token does not have to be a JWT (by the OIDC spec) and it should be treated as opaque string.

@mayorova
Copy link
Contributor Author

mayorova commented Mar 5, 2018

@mikz but aud claim includes the client_id in most of IdPs (Keycloak included). How would you verify the claim type with this field?

It seems indeed that there is no standard way to validate the type of token, but it looks like we can safely assume that Keycloak validates based on typ claim. In Okta it would be token_type instead.

The only standard and reliable way would of course be token instrospection, but I think we could rely on this non-standard typ for Keycloak to save us from the introspection calls...

@mikz
Copy link
Contributor

mikz commented Mar 5, 2018

@mayorova I don't want this to be Keycloak specific. Support for more IDPs is going to come in next version and I don't want to implement a workaround for every IDP.

So if we need to do this then we need one way for all of them. Either configuring the IDP to use aud claim and document that or using the introspection.

Edit:
looks like there should be a workaround for Keycloak to not expose client id: https://issues.jboss.org/browse/KEYCLOAK-5223

@kevprice83
Copy link
Member

I agree this should be done as the spec defines it. So firstly we should be validating the correct claim in the JWT which for an access_token would be the azp claim. However, as our only supported integration is with Keycloak currently I think we might need to handle this exception until that has been implemented correctly. We can still check the azp claim and fall back to the aud claim if necessary, this can then be deprecated once the next release of Keycloak is available with the proper fix. Unless we feel that the workaround is not too cumbersome as mentioned in the issue, I think it will cause too many problems personally.

As for verifying the type of token there is nothing in the spec that states we should reject certain types of tokens. However, I still think it is correct/makes sense to reject Refresh & Id tokens as they are meant for the authorisation server and client respectively. Neither of which APIcast proxy passes requests to so this should just be a well-documented behaviour of the gateway.

@mayorova
Copy link
Contributor Author

mayorova commented Mar 7, 2018

@kevprice83

We can still check the azp claim and fall back to the aud claim if necessary

We are already doing it :)
https://github.com/3scale/apicast/blob/02868a40189e0f15b16fd372b1dae10ce1ea5d56/gateway/src/apicast/oauth/oidc.lua#L129

@mikz
Copy link
Contributor

mikz commented Mar 7, 2018

My proposal for this release is to modify the Token introspection policy to:

  • allow caching and option to validate the token just once
  • use the oidc configuration from oidc discovery endpoint

And then the workaround should be just be just adding that policy to validate the token by token introspection.

For the next release I'd go with offering customization for the oidc authorization process so customers can define how the access token should be verified to support cases when aud is the client_id or the API endpoint.

@kevprice83
Copy link
Member

kevprice83 commented Mar 8, 2018

@mayorova yeah that's my point, we can continue to do that. There's no reason why it should just be a validation of that one claim at least whilst keycloak is doing it this way. I guess we are in agreement on this point then :)

@mikz yes I think this is going to be the majority of use cases rather than introspection on every call. I assume that the config used is the "main" apicast config then and not the oidc.config ?

@mikz
Copy link
Contributor

mikz commented Mar 12, 2018

@kevprice83 I probably don't get you question.

The Token introspection policy should not require configuration when OIDC is used, because it can get its configuration through OIDC discovery the same way as APIcast Policy does.

@mikz
Copy link
Contributor

mikz commented Mar 16, 2018

So I've tried to use the Token introspection endpoint of Keycloak and managed to get a response.
Wrong one, unfortunately.

Keycloak stubbornly returns {"active": false} even for just created tokens. This moves the token introspection solution out of the table for now.

Guess the next best solution would be to add jwt validator that verifies typ of the JWT payload (null or Bearer).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants