[keycloak-dev] Token's issuedAt value is the same as value of NotBeforePolicy
Stian Thorgersen
sthorger at redhat.com
Thu Apr 11 01:11:31 EDT 2019
I was mostly throwing the idea out there as an attempt to reduce amount of
manual configuration and problem solving if someone experiences problems.
I'm mostly onboard with going for 0 and making it configurable, but still
wonder if there's some safe default other than 0 that would make this
mostly a non-issue. I can see people having some confusing afternoons
trying to debug this if they do have some small time sync issues.
On Wed, 10 Apr 2019 at 09:53, Michal Hajas <mhajas at redhat.com> wrote:
> 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