9:21 a.m.
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...
1:47 p.m.
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
https://lists.jboss.org/mailman/listinfo/keycloak-dev
1:50 p.m.
On 09/04/2019 13:47, Marek Posolda 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 .
Sorry, that sentence should be: token is valid if it 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
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
1:53 p.m.
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()
On Tue, 9 Apr 2019 at 13:50, Marek Posolda <mposolda(a)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
> 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
3:16 p.m.
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@lists.jboss.org>
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org <mailto:keycloak-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
3:38 p.m.
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> 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@lists.jboss.org>
> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev(a)lists.jboss.org <mailto:keycloak-dev@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
3:42 p.m.
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
<mailto:mposolda@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(a)redhat.com
<mailto:mposolda@redhat.com>
> <mailto:mposolda@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@lists.jboss.org>
<mailto:keycloak-dev@lists.jboss.org
<mailto:keycloak-dev@lists.jboss.org>>
> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev(a)lists.jboss.org
<mailto:keycloak-dev@lists.jboss.org>
<mailto:keycloak-dev@lists.jboss.org
<mailto:keycloak-dev@lists.jboss.org>>
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org <mailto:keycloak-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
4:14 p.m.
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> 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> 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@lists.jboss.org
> >
> > > https://lists.jboss.org/mailman/listinfo/keycloak-dev
> >
> >
> > _______________________________________________
> > keycloak-dev mailing list
> > keycloak-dev(a)lists.jboss.org <mailto:keycloak-dev@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
>
4:38 p.m.
+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
<mailto:mposolda@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(a)redhat.com
> <mailto:mposolda@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(a)redhat.com <mailto:mposolda@redhat.com>
> > <mailto:mposolda@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@lists.jboss.org>
> <mailto:keycloak-dev@lists.jboss.org
> <mailto:keycloak-dev@lists.jboss.org>>
> > > https://lists.jboss.org/mailman/listinfo/keycloak-dev
> >
> >
> > _______________________________________________
> > keycloak-dev mailing list
> > keycloak-dev(a)lists.jboss.org
> <mailto:keycloak-dev@lists.jboss.org>
> <mailto:keycloak-dev@lists.jboss.org
> <mailto:keycloak-dev@lists.jboss.org>>
> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
> >
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev(a)lists.jboss.org
> <mailto:keycloak-dev@lists.jboss.org>
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
5:31 p.m.
-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> 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> 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> 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>
>> > > https://lists.jboss.org/mailman/listinfo/keycloak-dev
>> >
>> >
>> > _______________________________________________
>> > keycloak-dev mailing list
>> > keycloak-dev(a)lists.jboss.org
<mailto:keycloak-dev@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
>>
>
>
7:18 p.m.
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> 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> 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> 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>
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>
>>> > > 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>
>>> > 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>
9:53 a.m.
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(a)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(a)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-fo...
>
> On Tue, Apr 9, 2019 at 4:38 PM Marek Posolda <mposolda(a)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(a)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(a)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(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>
> >>> > > 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>
> >>> > 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
7:11 a.m.
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(a)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(a)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(a)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-fo...
> >
> > On Tue, Apr 9, 2019 at 4:38 PM Marek Posolda <mposolda(a)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(a)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(a)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(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>
> > >>> > >
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>
> > >>> > 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
1841
days inactive
1843
days old
12 comments
5 participants
participants (5)
-
Hynek Mlnarik -
Marek Posolda -
Michal Hajas -
Stefan Guilhen -
Stian Thorgersen