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

Tue Apr 9 11:31:55 EDT 2019

-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
>>>
>>
>>
>