[keycloak-dev] Token's issuedAt value is the same as value of NotBeforePolicy
Michal Hajas
mhajas at redhat.com
Wed Apr 10 03:53:17 EDT 2019
I agree with Hynek too, I think it is not a big effort to make it
configurable (defaulting to 0/1), however, it makes sense for an
administrator to have this possibility.
I introduced an issue for this:
https://issues.jboss.org/browse/KEYCLOAK-10021 feel free to update or
comment.
FYI: I introduced also https://issues.jboss.org/browse/KEYCLOAK-10013 which
will be fixed ASAP as I am blocked by this with stabilization of Spring
Boot tests.
Michal
On Tue, Apr 9, 2019 at 7:23 PM Stefan Guilhen <sguilhen at redhat.com> wrote:
> Agree with Hynek on this one. I'm also not convinced about the default
> value of 1 and I really believe the skew should be configurable.
>
> On Tue, Apr 9, 2019 at 1:08 PM Hynek Mlnarik <hmlnarik at redhat.com> wrote:
>
> > -1
> >
> > This should be administrator's known decision, based on their knowledge
> of
> > how much precise they need.
> >
> > See e.g. [1]: "Tolerances of 3–5 minutes are a reasonable default, but
> > allowing for configurability is a suggested practice."
> >
> > We can add a default of 1 second (not convinced about that either, IMO
> the
> > default should be 0 since it already works for the most deployments in
> the
> > wild) but the value has to be configurable, even if in a fine-grained
> > configuration.
> >
> > [1]
> >
> >
> https://docs.kantarainitiative.org/fi/rec-saml2-implementation-profile-for-fedinterop.html
> >
> > On Tue, Apr 9, 2019 at 4:38 PM Marek Posolda <mposolda at redhat.com>
> wrote:
> >
> > > +1
> > >
> > > We can revisit later if more fine-grained clock skew configuration is
> > > needed.
> > >
> > > Marek
> > >
> > > On 09/04/2019 16:14, Stian Thorgersen wrote:
> > >
> > > I wonder if we could just set it to 1 sec and not make it configurable?
> > >
> > > On Tue, 9 Apr 2019 at 15:42, Marek Posolda <mposolda at redhat.com>
> wrote:
> > >
> > >> Ah, ok. Forgot that we already added such option to identity
> providers,
> > >> so seems that people really have issues on servers too. +1 for add the
> > >> option then.
> > >>
> > >> Thanks,
> > >> Marek
> > >>
> > >>
> > >> On 09/04/2019 15:38, Hynek Mlnarik wrote:
> > >>
> > >> Clock time skew was asked for in SAML adapter [1] (number of watchers
> is
> > >> 6), mostly because inability to sync the clocks up to millisecond
> > precision
> > >>
> > >> I am in favor of including this option. We should support the
> > >> configuration both in adapters and in identity provider configuration
> > >> (these bits can be provided separately).
> > >>
> > >> [1] https://issues.jboss.org/browse/KEYCLOAK-8104
> > >>
> > >> On Tue, Apr 9, 2019 at 3:31 PM Marek Posolda <mposolda at redhat.com>
> > wrote:
> > >>
> > >>> On 09/04/2019 13:53, Stian Thorgersen wrote:
> > >>> > Not before should be reject tokens before that time, so changing to
> > >>> > the following as suggested would be the correct behaviour:
> > >>> >
> > >>> > this.token.getIssuedAt() >= deployment.getNotBefore
> > >>> >
> > >>> > Further, all adapters should allow a configurable allowed time sync
> > >>> > here. We should introduce a new property allowedTimeSkew with
> default
> > >>> > set to 1 second. The above would then be changed to:
> > >>> >
> > >>> > this.token.getIssuedAt() >= deployment.getNotBefore() -
> > >>> > deployment.getAllowedTimeSkew()
> > >>>
> > >>> That's the question whether we need another configuration option?
> Can't
> > >>> recall if someone asks for this? And we have lots of switches already
> > :)
> > >>>
> > >>> I see the point in having clock skew in keycloak.js, which is
> executed
> > >>> on user's machines and there is quite high chance of having time
> > >>> slightly out of sync. On the other hand, java adapter is mostly
> > executed
> > >>> on servers where apps are deployed and server administrators usually
> > pay
> > >>> attention to have time in sync to avoid various issues?
> > >>>
> > >>> Marek
> > >>>
> > >>> >
> > >>> > On Tue, 9 Apr 2019 at 13:50, Marek Posolda <mposolda at redhat.com
> > >>> > <mailto:mposolda at redhat.com>> wrote:
> > >>> >
> > >>> > My vote is to go with (2). It may turn to be more work as it
> > needs
> > >>> to
> > >>> > check all the adapters (node.js, maybe also gatekeeper etc).
> > >>> >
> > >>> > But strictly said, if not-before is set for example to 10:0000,
> > >>> > then my
> > >>> > understanding of not-before semantics is to check that: token
> is
> > >>> > valid
> > >>> > if it was issued not before 10:00:00 .
> > >>> > Which translates to: token is valid if it was wasn't issued
> > before
> > >>> > 10:00:00 .
> > >>> >
> > >>> > In other words, if not-before is 10:00:00 then token issued at
> > >>> > 9:59:59
> > >>> > shouldn't be valid, but token issued at 10:00:00 should be
> valid
> > >>> IMO.
> > >>> >
> > >>> > Marek
> > >>> >
> > >>> > On 09/04/2019 09:21, Michal Hajas wrote:
> > >>> > > Hi,
> > >>> > >
> > >>> > > I found out that when you do logout-all (in this step
> > >>> > realm.notBefore value
> > >>> > > is set) and subsequent login very quickly it may happen that
> > >>> > Keycloak
> > >>> > > returns tokens with an issuedAt value which is the same as
> the
> > >>> > value of the
> > >>> > > NotBeforePolicy.
> > >>> > >
> > >>> > > Such tokens are considered invalid in adapter due to this
> check
> > >>> [1].
> > >>> > >
> > >>> > > My question is, should we prevent such state? If yes what is
> > >>> correct
> > >>> > > behavior?
> > >>> > >
> > >>> > > 1. Do not generate tokens with the same issuedAt value as
> > >>> > NotBefore policy.
> > >>> > > For example, in TokenManager [2] check NotBefore value and
> > change
> > >>> > > issuedAt for all tokens to (NotBefore + 1) in case they are
> > same.
> > >>> > >
> > >>> > > or
> > >>> > >
> > >>> > > 2. Change condition [2]:
> > >>> > > .... && this.token.getIssuedAt() > deployment.getNotBefore();
> > >>> > > to:
> > >>> > > .... && this.token.getIssuedAt() >=
> deployment.getNotBefore();
> > >>> > >
> > >>> > > The later will probably require to also check other non-java
> > >>> > adapters
> > >>> > > whether such check is present or not.
> > >>> > >
> > >>> > > Best regards,
> > >>> > > Michal
> > >>> > >
> > >>> > > [1]
> > >>> > >
> > >>> >
> > >>>
> >
> https://github.com/keycloak/keycloak/blob/master/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/RefreshableKeycloakSecurityContext.java#L79
> > >>> > >
> > >>> > > [2]
> > >>> > >
> > >>> >
> > >>>
> >
> https://github.com/keycloak/keycloak/blob/master/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java#L796
> > >>> > > _______________________________________________
> > >>> > > keycloak-dev mailing list
> > >>> > > keycloak-dev at lists.jboss.org <mailto:
> > >>> keycloak-dev at lists.jboss.org>
> > >>> > > https://lists.jboss.org/mailman/listinfo/keycloak-dev
> > >>> >
> > >>> >
> > >>> > _______________________________________________
> > >>> > keycloak-dev mailing list
> > >>> > keycloak-dev at lists.jboss.org <mailto:
> > keycloak-dev at lists.jboss.org>
> > >>> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
> > >>> >
> > >>>
> > >>> _______________________________________________
> > >>> keycloak-dev mailing list
> > >>> keycloak-dev at lists.jboss.org
> > >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> > >>>
> > >>
> > >>
> > >
> > _______________________________________________
> > keycloak-dev mailing list
> > keycloak-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
>
>
> --
>
> STEFAN GUILHEN
>
> PRINCIPAL SOFTWARE ENGINEER
>
> Red Hat <https://www.redhat.com/>
>
> sguilhen at redhat.com M: +55-11-98117-7332
> <https://red.ht/sig>
> @RedHat <https://twitter.com/redhat> Red Hat
> <https://www.linkedin.com/company/red-hat> Red Hat
> <https://www.facebook.com/RedHatInc>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
More information about the keycloak-dev
mailing list