[keycloak-dev] Refresh tokens no longer reusable

[Stian Thorgersen]

That was my initial idea as well, but then again it already works with our
adapters, we already regenerate the tokens, so why not add this extra layer
of defence? End of the day as refresh tokens can be stored on the client
side how well they are secured can vary. If users want long sessions or
even worse with offline tokens it makes sense to add this that enables
users to at least notice something is going wrong. The issue is that you
may not notice that a client has been compromised, but if all tokens stop
working you will.

I don't have an issue with setting it to true by default (so refresh tokens
are reusable), but since our adapters already work and I can't see any big
side effects of preventing refresh token reuse I set it to false by default.

On 15 October 2015 at 01:27, Bill Burke <bburke at redhat.com> wrote:

>
>
> On 10/14/2015 5:49 PM, Marek Posolda wrote:
> > On 14/10/15 20:24, Stian Thorgersen wrote:
> >> Refresh tokens are no longer reusable. This is done by setting the
> >> client sessions timestamp when a new refresh token is issued. If the
> >> refresh tokens iat value is less than the client sessions timestamp
> >> it's not permitted.
> >>
> >> If anyone has time I'd appreciate a review of the changes:
> >> <https://github.com/keycloak/keycloak/pull/1732>
> https://github.com/keycloak/keycloak/pull/1732
> >>
> >> For anyone that runs into issues with this policy there's an option to
> >> disable it in the admin console in the realms token settings.
> >>
> >> This does not apply to offline tokens (at least yet). We need to add
> >> it to offline tokens as well though as it's even more important for
> >> those. There's two problems with offline tokens though, firstly the
> >> setTimestamp is not permitted on offline client sessions. Secondly if
> >> we allow setting it we would have to persist it, unless someone can
> >> come up with something clever.
> > I think we don't need to persist, but just save clientSession with
> > updated timestamp into infinispan/memory. Then during startup, the
> > timestamp of clientSessions will be updated to startup time similarly
> > like we have for lastSessionRefresh of user sessions. The refresh will
> > be allowed if (iat == clientSession.timestamp OR startupTime ==
> > clientSession.timestamp) . In other words, first refresh after server
> > restart will be always allowed.
> >
> > There is some chance that there  can be same refresh token used two
> > times (if attacker will do second refresh after server restart). But
> > then clientSession timestamp will be updated and regular user won't be
> > allowed to refresh his token and will recognize error.
> >
> >
> > But question is, do we really want refreshTokenReusable to be disabled
> > by default? For offline tokens, people will often need to save the
> > offline token into their database on application side. With
> > refreshTokenReusable disabled, they will need to always write into their
> > DB and save new offline token after each refresh.
> >
>
> My own personal opinion is that we are making this fix to pass some
> random company's security audit that I don't particularly agree with.
> If a client has been compromised, then its offline tokens should be
> revoked and a revocation not-before policy should be pushed out.  As it
> is, the only reason we need to regenerate the refresh token is to update
> its timestamp for idle checks.
>
>
> --
> Bill Burke
> JBoss, a division of Red Hat
> http://bill.burkecentral.com
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/keycloak-dev/attachments/20151015/5867dc95/attachment.html