Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why limit expiration #40

Merged
merged 3 commits into from Jul 14, 2017
Merged

Why limit expiration #40

merged 3 commits into from Jul 14, 2017

Conversation

martinthomson
Copy link
Collaborator

@martinthomson martinthomson commented Jun 28, 2017

From the secdir review by Robert Sparks.

an attacker and can increase the difficulty of stealing a token.
an attacker and can increase the difficulty of stealing a token. Placing a
hard upper limit on the "exp" claim in the JWT allows a push service to reject
tokens that have excessively long expiration times.
Copy link
Collaborator

@beverloo beverloo Jun 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely convinced about the value of this addition. It's normatively required (the MUST NOT on line 145) and already implied by the sentence directly above ("applying narrow limits"), as it would not make sense if the PS was free to ignore this requirement. It's clear though, so SGTM from that point of view.

Copy link
Collaborator Author

@martinthomson martinthomson Jul 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm going to cut this one.

@martinthomson martinthomson merged commit c64b3dd into master Jul 14, 2017
1 check passed
@martinthomson martinthomson deleted the why_limit_expiration branch Jul 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants