Re: [keycloak-dev] Token's issuedAt value is the same as value of NotBeforePolicy

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(a)redhat.com&gt; 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-fo... > > On Tue, Apr 9, 2019 at 4:38 PM Marek Posolda <mposolda(a)redhat.com&gt; 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(a)redhat.com&gt; 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(a)redhat.com&gt; > 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(a)redhat.com > >>> > <mailto:mposolda@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-co... > >>> > > > >>> > > [2] > >>> > > > >>> > > >>> > https://github.com/keycloak/keycloak/blob/master/services/src/main/java/o... > >>> > > _______________________________________________ > >>> > > keycloak-dev mailing list > >>> > > keycloak-dev(a)lists.jboss.org <mailto: > >>> keycloak-dev(a)lists.jboss.org&gt; > >>> > > https://lists.jboss.org/mailman/listinfo/keycloak-dev > >>> > > >>> > > >>> > _______________________________________________ > >>> > keycloak-dev mailing list > >>> > keycloak-dev(a)lists.jboss.org <mailto: > keycloak-dev(a)lists.jboss.org&gt; > >>> > https://lists.jboss.org/mailman/listinfo/keycloak-dev > >>> > > >>> > >>> _______________________________________________ > >>> keycloak-dev mailing list > >>> keycloak-dev(a)lists.jboss.org > >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev > >>> > >> > >> > > > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-dev -- STEFAN GUILHEN PRINCIPAL SOFTWARE ENGINEER Red Hat <https://www.redhat.com/> sguilhen(a)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(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev