Skip to content
This repository was archived by the owner on Jul 16, 2025. It is now read-only.

Add 1 minute clock skew when verifying time constraint#57

Merged
qiwzhang merged 1 commit intogoogle:masterfrom
qiwzhang:time-skew
Nov 3, 2020
Merged

Add 1 minute clock skew when verifying time constraint#57
qiwzhang merged 1 commit intogoogle:masterfrom
qiwzhang:time-skew

Conversation

@qiwzhang
Copy link
Contributor

@qiwzhang qiwzhang commented Nov 2, 2020

Signed-off-by: Wayne Zhang qiwzhang@google.com

This is to fix :GoogleCloudPlatform/esp-v2#369

This is to on-par with grpc jwt_validator

Signed-off-by: Wayne Zhang <qiwzhang@google.com>
@qiwzhang qiwzhang requested review from a team and nareddyt and removed request for a team November 2, 2020 23:04
@yangminzhu
Copy link

@qiwzhang should we make this configurable? I'm not sure 1 minute is good for all cases, sometime we have short-lived JWT token that expires in ~5 minutes (or even shorter), and 1 minute skew means at least 20% difference.

@qiwzhang
Copy link
Contributor Author

qiwzhang commented Nov 4, 2020

Well grpc is hardcoded to 1 minute jwt_validator

I think we should be fine.

lizan pushed a commit to envoyproxy/envoy that referenced this pull request Nov 16, 2020
When verifying Jwt clock constraint,  it is recommend to use some clock skew.   grpc is using 1 minute clock [skew](https://github.com/grpc/grpc/blob/4645da201ae2c7d0b15fe56d86b41354fa4af0ca/src/core/lib/security/credentials/jwt/jwt_verifier.cc#L388-L389).

[jwt_verify_lib](google/jwt_verify_lib#57) has been updated to add 1 minute clock skew.  

In the old code,  time constraint verification is done in jwt_authn filter, and jwt_verify_lib::verifyJwt() is doing time constraint verification again.

Change  jwt_verify_lib to split the time constraint verification to Jwt class so it can be called separately. And call verify() without the time checking.

Risk Level: None
Testing:  unit-test is done in jwt_verify_lib repo
Docs Changes: None
Release Notes: Added

Signed-off-by: Wayne Zhang <qiwzhang@google.com>
lizan pushed a commit to envoyproxy/data-plane-api that referenced this pull request Nov 16, 2020
When verifying Jwt clock constraint,  it is recommend to use some clock skew.   grpc is using 1 minute clock [skew](https://github.com/grpc/grpc/blob/4645da201ae2c7d0b15fe56d86b41354fa4af0ca/src/core/lib/security/credentials/jwt/jwt_verifier.cc#L388-L389).

[jwt_verify_lib](google/jwt_verify_lib#57) has been updated to add 1 minute clock skew.

In the old code,  time constraint verification is done in jwt_authn filter, and jwt_verify_lib::verifyJwt() is doing time constraint verification again.

Change  jwt_verify_lib to split the time constraint verification to Jwt class so it can be called separately. And call verify() without the time checking.

Risk Level: None
Testing:  unit-test is done in jwt_verify_lib repo
Docs Changes: None
Release Notes: Added

Signed-off-by: Wayne Zhang <qiwzhang@google.com>

Mirrored from https://github.com/envoyproxy/envoy @ cd684e76bda80e140ab90573815f1990ec6f2a6f
andreyprezotto pushed a commit to andreyprezotto/envoy that referenced this pull request Nov 24, 2020
…oxy#13872)

When verifying Jwt clock constraint,  it is recommend to use some clock skew.   grpc is using 1 minute clock [skew](https://github.com/grpc/grpc/blob/4645da201ae2c7d0b15fe56d86b41354fa4af0ca/src/core/lib/security/credentials/jwt/jwt_verifier.cc#L388-L389).

[jwt_verify_lib](google/jwt_verify_lib#57) has been updated to add 1 minute clock skew.  

In the old code,  time constraint verification is done in jwt_authn filter, and jwt_verify_lib::verifyJwt() is doing time constraint verification again.

Change  jwt_verify_lib to split the time constraint verification to Jwt class so it can be called separately. And call verify() without the time checking.

Risk Level: None
Testing:  unit-test is done in jwt_verify_lib repo
Docs Changes: None
Release Notes: Added

Signed-off-by: Wayne Zhang <qiwzhang@google.com>
qqustc pushed a commit to qqustc/envoy that referenced this pull request Nov 24, 2020
…oxy#13872)

When verifying Jwt clock constraint,  it is recommend to use some clock skew.   grpc is using 1 minute clock [skew](https://github.com/grpc/grpc/blob/4645da201ae2c7d0b15fe56d86b41354fa4af0ca/src/core/lib/security/credentials/jwt/jwt_verifier.cc#L388-L389).

[jwt_verify_lib](google/jwt_verify_lib#57) has been updated to add 1 minute clock skew.

In the old code,  time constraint verification is done in jwt_authn filter, and jwt_verify_lib::verifyJwt() is doing time constraint verification again.

Change  jwt_verify_lib to split the time constraint verification to Jwt class so it can be called separately. And call verify() without the time checking.

Risk Level: None
Testing:  unit-test is done in jwt_verify_lib repo
Docs Changes: None
Release Notes: Added

Signed-off-by: Wayne Zhang <qiwzhang@google.com>
Signed-off-by: Qin Qin <qqin@google.com>
rexengineering pushed a commit to rexengineering/istio-envoy that referenced this pull request Oct 15, 2021
When verifying Jwt clock constraint,  it is recommend to use some clock skew.   grpc is using 1 minute clock [skew](https://github.com/grpc/grpc/blob/4645da201ae2c7d0b15fe56d86b41354fa4af0ca/src/core/lib/security/credentials/jwt/jwt_verifier.cc#L388-L389).

[jwt_verify_lib](google/jwt_verify_lib#57) has been updated to add 1 minute clock skew.  

In the old code,  time constraint verification is done in jwt_authn filter, and jwt_verify_lib::verifyJwt() is doing time constraint verification again.

Change  jwt_verify_lib to split the time constraint verification to Jwt class so it can be called separately. And call verify() without the time checking.

Risk Level: None
Testing:  unit-test is done in jwt_verify_lib repo
Docs Changes: None
Release Notes: Added

Signed-off-by: Wayne Zhang <qiwzhang@google.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants