Skip to content
New issue

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

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

Already on GitHub? # to your account

Argument validation for allowedClockSkewSeconds #601

Merged
merged 2 commits into from
Jun 11, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion api/src/main/java/io/jsonwebtoken/JwtParser.java
Original file line number Diff line number Diff line change
@@ -191,13 +191,15 @@ public interface JwtParser {
* @param seconds the number of seconds to tolerate for clock skew when verifying {@code exp} or {@code nbf} claims.
* @return the parser for method chaining.
* @since 0.7.0
* @throws IllegalArgumentException if {@code seconds} is a value greater than {@code Long.MAX_VALUE / 1000} as
* any such value would cause numeric overflow when multiplying by 1000 to obtain a millisecond value.
* @deprecated see {@link JwtParserBuilder#setAllowedClockSkewSeconds(long)}.
* To construct a JwtParser use the corresponding builder via {@link Jwts#parserBuilder()}. This will construct an
* immutable JwtParser.
* <p><b>NOTE: this method will be removed before version 1.0</b>
*/
@Deprecated
JwtParser setAllowedClockSkewSeconds(long seconds);
JwtParser setAllowedClockSkewSeconds(long seconds) throws IllegalArgumentException;

/**
* Sets the signing key used to verify any discovered JWS digital signature. If the specified JWT string is not
4 changes: 3 additions & 1 deletion api/src/main/java/io/jsonwebtoken/JwtParserBuilder.java
Original file line number Diff line number Diff line change
@@ -147,8 +147,10 @@ public interface JwtParserBuilder {
*
* @param seconds the number of seconds to tolerate for clock skew when verifying {@code exp} or {@code nbf} claims.
* @return the parser builder for method chaining.
* @throws IllegalArgumentException if {@code seconds} is a value greater than {@code Long.MAX_VALUE / 1000} as
* any such value would cause numeric overflow when multiplying by 1000 to obtain a millisecond value.
*/
JwtParserBuilder setAllowedClockSkewSeconds(long seconds);
JwtParserBuilder setAllowedClockSkewSeconds(long seconds) throws IllegalArgumentException;

/**
* Sets the signing key used to verify any discovered JWS digital signature. If the specified JWT string is not
Original file line number Diff line number Diff line change
@@ -180,7 +180,8 @@ public JwtParser setClock(Clock clock) {
}

@Override
public JwtParser setAllowedClockSkewSeconds(long seconds) {
public JwtParser setAllowedClockSkewSeconds(long seconds) throws IllegalArgumentException {
Assert.isTrue(seconds <= DefaultJwtParserBuilder.MAX_CLOCK_SKEW_MILLIS, DefaultJwtParserBuilder.MAX_CLOCK_SKEW_ILLEGAL_MSG);
this.allowedClockSkewMillis = Math.max(0, seconds * MILLISECONDS_PER_SECOND);
return this;
}
Original file line number Diff line number Diff line change
@@ -39,6 +39,16 @@ public class DefaultJwtParserBuilder implements JwtParserBuilder {

private static final int MILLISECONDS_PER_SECOND = 1000;

/**
* To prevent overflow per <a href="https://github.com/jwtk/jjwt/issues/583">Issue 583</a>.
*
* Package-protected on purpose to allow use in backwards-compatible {@link DefaultJwtParser} implementation.
* TODO: enable private modifier on these two variables when deleting DefaultJwtParser
*/
static final long MAX_CLOCK_SKEW_MILLIS = Long.MAX_VALUE / MILLISECONDS_PER_SECOND;
static final String MAX_CLOCK_SKEW_ILLEGAL_MSG = "Illegal allowedClockSkewMillis value: multiplying this " +
"value by 1000 to obtain the number of milliseconds would cause a numeric overflow.";

private byte[] keyBytes;

private Key key;
@@ -130,7 +140,8 @@ public JwtParserBuilder setClock(Clock clock) {
}

@Override
public JwtParserBuilder setAllowedClockSkewSeconds(long seconds) {
public JwtParserBuilder setAllowedClockSkewSeconds(long seconds) throws IllegalArgumentException {
Assert.isTrue(seconds <= MAX_CLOCK_SKEW_MILLIS, MAX_CLOCK_SKEW_ILLEGAL_MSG);
this.allowedClockSkewMillis = Math.max(0, seconds * MILLISECONDS_PER_SECOND);
return this;
}
Original file line number Diff line number Diff line change
@@ -75,4 +75,21 @@ class DefaultJwtParserBuilderTest {

assertEquals 'bar', p.setSigningKey(key).build().parseClaimsJws(jws).getBody().get('foo')
}

@Test
void testMaxAllowedClockSkewSeconds() {
long max = Long.MAX_VALUE / 1000 as long
new DefaultJwtParserBuilder().setAllowedClockSkewSeconds(max) // no exception should be thrown
}

@Test
void testExceededAllowedClockSkewSeconds() {
long value = Long.MAX_VALUE / 1000 as long
value = value + 1L
try {
new DefaultJwtParserBuilder().setAllowedClockSkewSeconds(value)
} catch (IllegalArgumentException expected) {
assertEquals DefaultJwtParserBuilder.MAX_CLOCK_SKEW_ILLEGAL_MSG, expected.message
}
}
}
Original file line number Diff line number Diff line change
@@ -131,4 +131,21 @@ class DefaultJwtParserTest {

new DefaultJwtParser().setSigningKey(key).parseClaimsJws(invalidJws)
}

@Test
void testMaxAllowedClockSkewSeconds() {
long max = Long.MAX_VALUE / 1000 as long
new DefaultJwtParser().setAllowedClockSkewSeconds(max) // no exception should be thrown
}

@Test
void testExceededAllowedClockSkewSeconds() {
long value = Long.MAX_VALUE / 1000 as long
value = value + 1L
try {
new DefaultJwtParser().setAllowedClockSkewSeconds(value)
} catch (IllegalArgumentException expected) {
assertEquals DefaultJwtParserBuilder.MAX_CLOCK_SKEW_ILLEGAL_MSG, expected.message
}
}
}