Skip to content

Commit

Permalink
Fixing issue #3 and adding tests to confirm.
Browse files Browse the repository at this point in the history
  • Loading branch information
robotdan committed May 2, 2018
1 parent 8d73248 commit abb0d47
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 7 deletions.
12 changes: 9 additions & 3 deletions src/main/java/org/primeframework/jwt/JWTDecoder.java
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,11 @@ private byte[] base64Decode(byte[] bytes) {
}

private JWT decode(String encodedJWT, Header header, String[] parts, Verifier verifier) {
int index = encodedJWT.lastIndexOf(".");
// The message comprises the first two segments of the entire JWT, the signature is the last segment.
byte[] message = encodedJWT.substring(0, index).getBytes(StandardCharsets.UTF_8);
// The callers of this decode will have already handled 'none' if it was deemed to be valid based upon
// the provided verifiers. At this point, if we have a 'none' algorithm specified in the header, it is invalid.
if (header.algorithm == Algorithm.none) {
throw new MissingVerifierException("No Verifier has been provided for verify a signature signed using [" + header.algorithm.getName() + "]");
}

// If a signature is provided and verifier must be provided.
if (parts.length == 3 && verifier == null) {
Expand All @@ -165,6 +167,10 @@ private JWT decode(String encodedJWT, Header header, String[] parts, Verifier ve
throw new InvalidJWTSignatureException();
}

int index = encodedJWT.lastIndexOf(".");
// The message comprises the first two segments of the entire JWT, the signature is the last segment.
byte[] message = encodedJWT.substring(0, index).getBytes(StandardCharsets.UTF_8);

if (parts.length == 3) {
// Verify the signature before de-serializing the payload.
byte[] signature = base64Decode(parts[2].getBytes(StandardCharsets.UTF_8));
Expand Down
29 changes: 25 additions & 4 deletions src/test/java/org/primeframework/jwt/VulnerabilityTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ public void test_encodedJwtWithSignatureRemoved() throws Exception {

String hackedJWT = encodedJWT.substring(0, encodedJWT.lastIndexOf("."));

expectException(InvalidJWTException.class, ()
-> JWT.getDecoder().decode(hackedJWT, HMACVerifier.newVerifier("secret")));
expectException(InvalidJWTException.class, () -> JWT.getDecoder().decode(hackedJWT, HMACVerifier.newVerifier("secret")));
}

@Test
Expand All @@ -71,8 +70,30 @@ public void test_noVerification() throws Exception {
JWT jwt = new JWT().setSubject("art");
String encodedJWT = JWT.getEncoder().encode(jwt, HMACSigner.newSHA256Signer("secret"));

expectException(MissingVerifierException.class, ()
-> JWT.getDecoder().decode(encodedJWT));
expectException(MissingVerifierException.class, () -> JWT.getDecoder().decode(encodedJWT));
}

@Test
public void test_unsecuredJWT_validation() throws Exception {
JWT jwt = new JWT().setSubject("123456789");
Signer signer = new UnsecuredSigner();
Verifier hmacVerifier = HMACVerifier.newVerifier("too many secrets");

String encodedUnsecuredJWT = JWTEncoder.getInstance().encode(jwt, signer);

// Ensure that attempting to decode an un-secured JWT fails when we provide a verifier
expectException(MissingVerifierException.class, () -> JWT.getDecoder().decode(encodedUnsecuredJWT, hmacVerifier));

String encodedUnsecuredJWT_withKid = JWTEncoder.getInstance().encode(jwt, signer, (header) -> header.set("kid", "abc"));
String encodedUnsecuredJWT_withoutKid = JWTEncoder.getInstance().encode(jwt, signer);

Map<String, Verifier> verifierMap = new HashMap<>();
verifierMap.put(null, hmacVerifier);
verifierMap.put("abc", hmacVerifier);

// Ensure that attempting to decode an un-secured JWT fails when we provide a verifier with or without using a kid
expectException(MissingVerifierException.class, () -> JWT.getDecoder().decode(encodedUnsecuredJWT_withKid, verifierMap));
expectException(MissingVerifierException.class, () -> JWT.getDecoder().decode(encodedUnsecuredJWT_withoutKid, verifierMap));
}

@Test
Expand Down

0 comments on commit abb0d47

Please sign in to comment.