envoyproxy
Oauth2 lifetime of refresh token
Commit Message:
OAuth2 filter: fixing the lifetime of the refresh token. The refresh token is not expired together with the access token. So, once the access token is expired, the oauth2 filter can update the access token using by the refresh token.
Additional Description:
The expiration time of the access token and refresh token is the same. So, once the access token is expired oauth filter cannot update it using by refresh token because the refresh token is expired too.
The refresh token has an exp claim which specifes a lifetime of the token. So now the expiration time of the refresh token is equaled of the value in the exp claim. This value is more than a lifetime of the access token. So once the access token is expired the oauth filter can update the access token using by the refresh token.
Besides that I cannot sure that every jwt contains the exp claim. So I have provided a new option default_refresh_token_expires_in that is specified a default lifetime in seconds of the refresh token, if the exp claim is omitted in the refresh token or the refresh token is not JWT. Default value is 604800 seconds (a week)
Fix for issue #30053
Risk Level: Low Testing: Unit Tests Docs Changes: Needs Release Notes: Needs
Platform Specific Features: [Optional Fixes #30053]
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).
:cat:
Caused by: https://github.com/envoyproxy/envoy/pull/32278 was opened by Alexcei88.
![repokitteh-read-only[bot] avatar](https://avatars.githubusercontent.com/in/16729?v=4 "Published by a pub.dev verified publisher"){kind=small}
Should we allow setting the default expiration to unlimited via some magical number? Given 0 is already used for immediate expiration, I guess -1 is the next available option.
Thoughts?
@loewenstein, As far as I know according this if value is less than or equal to zero then cookie expires immediately. Maybe I am wrong, honestly I have not checked.
@loewenstein, As far as I know according this if value is less than or equal to zero then cookie expires immediately. Maybe I am wrong, honestly I have not checked.
Understood. Would not setting Max-Age at all then be an alternative we should consider users to be able to configure?
@loewenstein, As far as I know according this if value is less than or equal to zero then cookie expires immediately. Maybe I am wrong, honestly I have not checked.
Understood. Would not setting
Max-Ageat all then be an alternative we should consider users to be able to configure?
According to https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#expiresdate -
If unspecified, the cookie becomes a session cookie. A session finishes when the client shuts down, after which the session cookie is removed.
I would see this as closely similar to not supporting refresh token at all (which is a current configuration parameter).
I don't think there's any unlimited value for cookie lifetimes, just various options of "far into the future", with different browsers supporting different max values. Chrome itself currently caps the maximum lifetime to 400 days, there's not much value in having a default value higher than this: https://chromestatus.com/feature/4887741241229312
@Alexcei88:
The refresh token has an exp claim which specifes a lifetime of the token
This is only in cases where the refresh token is a JWT. I'm not familiar with any providers that do this, but as it's defined as an opaque identifier, there are probably providers who do. (e.g. Okta and Google provide refresh tokens which are not JWTs).
But I think in the most cases the value is not needed to configure.
All the refresh tokens I remember seeing in the wild cannot be parsed in this way, so a default is necessary (but also, I think, not especially important, a fairly long duration will suffice; the provider will reply with whether the refresh token worked or not).
@fiadliel , thank you for information. I always face with refresh token that is JWT. So I wonder sometimes it is not so. What default value are you suggesting? 24 hours?
Understood. Would not setting
Max-Ageat all then be an alternative we should consider users to be able to configure?
I have had the same thought. But in the cases when we can figure out exactly the lifetime of the token it's best to use this value. So I decided to make so. I cannot come up cases when this behaviour user wishes to override.
@fiadliel , thank you for information. I always face with refresh token that is JWT. So I wonder sometimes it is not so. What default value are you suggesting? 24 hours?
It's the auth provider that validates the refresh token, this is different to how the access token is validated. So it's not a security issue to extend the token on the client side.
The tradeoffs (in my mind) on refresh token cookie lifetime:
Shorter lifetime advantages:
- invalid refresh token doesn't use up bandwidth or cookie storage
- attempt to use invalid refresh token makes the latency of call longer
Longer lifetime advantages:
- if not sure if token is invalid, you might avoid a new login cycle
The advantages of a short cookie lifetime are minor, and the advantage of a longer lifetime is a good potential quality of life improvement for the client.
Some example refresh tokens validity policies with Okta:
- 1 day
- 1 week
- Unlimited, but will expire if not used every 7 days
Personally, I would err on the side of a longer cookie lifetime (e.g. 1 year). The cost is fairly low, a failed attempt to refresh the cookie, followed by redirect to login.
Default value of the default_refresh_token_expires_in = 604800 seconds (a week)
I stumbled across this PR because I ran into a related issue: https://github.com/envoyproxy/envoy/issues/32485. If I am not mistaken, the implementation of reading the expiry from the JWT could be reused to solve https://github.com/envoyproxy/envoy/issues/32485 as well.
@michaelsauter , Yes, if IdToken is JWT then the expiration time of the that can be taken from itself JWT. It's easy.
I am using Keycloak server that is pretty popular Identity Provider. it provides the refresh token as JWT. So I consider it's common.
How common the refresh token is a JWT? I'm a bit worried about pulling JWT dependencies in the OAuth filter.
@lizan Please also have a look at https://github.com/envoyproxy/envoy/issues/32485. For that issue, the JWT dependencies would also be needed as an ID token is always represented as a JWT, see https://openid.net/specs/openid-connect-core-1_0.html#IDToken.
I didn't test this, but there might be an issue with the HMAC over the cookie values. The CookieValidator should not be able to successfully validate the cookies if the only surviving cookie is the refresh token cookie since the HMAC is done with all tokens. I think the refresh token should have it's own HMAC cookie with the same Max-Age.
Did I miss something?
Once the access token is expired, the HMAC become invalid too. It's expected behaviour. And it' s ok that Cookie validator cannot validate cookies, we cannot skip authorization in this moment because there is no access token. The refresh token is accessory token. So once next request is received it triggers updating the access token using the refresh token and HMAC is recalculated again. The HMAC will be valid again until the access token expires. So no reason for saving separated HMAC for refresh token.
OAuth2 itself has nothing to say about JWTs (e.g. search for the phrase in https://datatracker.ietf.org/doc/html/rfc6749). The ID token is part of OpenID Connect Core, which does require JWTs for full support.
@Alexcei88 Thank you for your explanation, now I see that it will work. But isn't it an issue, that the refresh token is now unsecured? An attacker may now send bogus refresh tokens, causing the filter to issue requests to the authentication server. Those request may cause undesired load to the authentication server or even worse open a new attack vector to the authentication server and envoy.
I don't want to add one more cookie. There are too many of them.... Do you have any idea without adding one more HMAC cookie?
The oauth provider validates token in any case. And I think it's a not secured issue. The best solution would be saving the refresh token in some other place which is more secured than as cookie.
Do you have any idea without adding one more HMAC cookie?
Well the HMAC does not need to be in a separate Cookie. You could exclude the refresh token cookie from the original HMAC and change the refresh token cookie value to a combination of expiry + HMAC(expiry).
@ergonjona , I am not sure I catch your idea. Could you explain please what you suggest?
@Alexcei88, Excuse me, I mixed things up a little bit. The idea is to write the refresh token and its HMAC into one cookie. So there would be just four cookies:
- bearer_token
- id_token
- oauth_hmac (of bearer_token and id_token)
- refresh_token (with its own HMAC joined to the token value)
I don't consider that sending an invalid refresh token to identity provider server is a critical secured issue. I expect that Identity provider server is ready for it. So, I am looking forward for review. What do you think?
