Skip to content

Commit

Permalink
fix ECDSA signature encoding
Browse files Browse the repository at this point in the history 
Use a copy of the ECDSA tests with BouncyCastle provider
  • Loading branch information
@lbalmaceda
lbalmaceda committed Nov 3, 2017
1 parent 34aee16 commit d396396
Show file tree
Hide file tree
Showing 7 changed files with 1,573 additions and 67 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Temporary Items

# IntelliJ
/out/
/lib/out/

# mpeltonen/sbt-idea plugin
.idea_modules/
Expand Down
4 changes: 2 additions & 2 deletions lib/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ compileJava {
dependencies {
compile 'com.fasterxml.jackson.core:jackson-databind:2.8.4'
compile 'commons-codec:commons-codec:1.10'
compile 'org.bouncycastle:bcprov-jdk15on:1.55'
testCompile 'org.bouncycastle:bcprov-jdk15on:1.58'
testCompile 'junit:junit:4.12'
testCompile 'net.jodah:concurrentunit:0.4.2'
testCompile 'org.hamcrest:hamcrest-library:1.3'
testCompile 'org.hamcrest:java-hamcrest:2.0.0.0'
testCompile 'org.mockito:mockito-core:2.2.8'
}

Expand Down
97 changes: 78 additions & 19 deletions lib/src/main/java/com/auth0/jwt/algorithms/ECDSAAlgorithm.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,7 @@ public void verify(DecodedJWT jwt) throws SignatureVerificationException {
if (publicKey == null) {
throw new IllegalStateException("The given Public Key is null.");
}
if (!isDERSignature(signatureBytes)) {
signatureBytes = JOSEToDER(signatureBytes);
}
boolean valid = crypto.verifySignatureFor(getDescription(), publicKey, contentBytes, signatureBytes);
boolean valid = crypto.verifySignatureFor(getDescription(), publicKey, contentBytes, JOSEToDER(signatureBytes));

if (!valid) {
throw new SignatureVerificationException(this);
Expand All @@ -64,7 +61,8 @@ public byte[] sign(byte[] contentBytes) throws SignatureGenerationException {
if (privateKey == null) {
throw new IllegalStateException("The given Private Key is null.");
}
return crypto.createSignatureFor(getDescription(), privateKey, contentBytes);
byte[] signature = crypto.createSignatureFor(getDescription(), privateKey, contentBytes);
return DERToJOSE(signature);
} catch (NoSuchAlgorithmException | SignatureException | InvalidKeyException | IllegalStateException e) {
throw new SignatureGenerationException(this, e);
}
Expand All @@ -75,15 +73,60 @@ public String getSigningKeyId() {
return keyProvider.getPrivateKeyId();
}

private boolean isDERSignature(byte[] signature) {
//Visible for testing
byte[] DERToJOSE(byte[] derSignature) throws SignatureException {
// DER Structure: http://crypto.stackexchange.com/a/1797
// Should begin with 0x30 and have exactly the expected length
return signature[0] == 0x30 && signature.length != ecNumberSize * 2;
boolean derEncoded = derSignature[0] == 0x30 && derSignature.length != ecNumberSize * 2;
if (!derEncoded) {
throw new SignatureException("Invalid DER signature format.");
}

final byte[] joseSignature = new byte[ecNumberSize * 2];

//Skip 0x30
int offset = 1;
if (derSignature[1] == (byte) 0x81) {
//Skip sign
offset++;
}

//Convert to unsigned. Should match DER length - offset
int encodedLength = derSignature[offset++] & 0xff;
if (encodedLength != derSignature.length - offset) {
throw new SignatureException("Invalid DER signature format.");
}

//Skip 0x02
offset++;

//Obtain R number length (Includes padding) and skip it
int rLength = derSignature[offset++];
if (rLength > ecNumberSize + 1) {
throw new SignatureException("Invalid DER signature format.");
}
int rPadding = ecNumberSize - rLength;
//Retrieve R number
System.arraycopy(derSignature, offset + Math.max(-rPadding, 0), joseSignature, Math.max(rPadding, 0), rLength + Math.min(rPadding, 0));

//Skip R number and 0x02
offset += rLength + 1;

//Obtain S number length. (Includes padding)
int sLength = derSignature[offset++];
if (sLength > ecNumberSize + 1) {
throw new SignatureException("Invalid DER signature format.");
}
int sPadding = ecNumberSize - sLength;
//Retrieve R number
System.arraycopy(derSignature, offset + Math.max(-sPadding, 0), joseSignature, ecNumberSize + Math.max(sPadding, 0), sLength + Math.min(sPadding, 0));

return joseSignature;
}

private byte[] JOSEToDER(byte[] joseSignature) throws SignatureException {
//Visible for testing
byte[] JOSEToDER(byte[] joseSignature) throws SignatureException {
if (joseSignature.length != ecNumberSize * 2) {
throw new SignatureException(String.format("The signature length was invalid. Expected %d bytes but received %d", ecNumberSize * 2, joseSignature.length));
throw new SignatureException("Invalid JOSE signature format.");
}

// Retrieve R and S number's length and padding.
Expand All @@ -94,10 +137,10 @@ private byte[] JOSEToDER(byte[] joseSignature) throws SignatureException {

int length = 2 + rLength + 2 + sLength;
if (length > 255) {
throw new SignatureException("Invalid ECDSA signature format");
throw new SignatureException("Invalid JOSE signature format.");
}

byte[] derSignature;
final byte[] derSignature;
int offset;
if (length > 0x7f) {
derSignature = new byte[3 + length];
Expand All @@ -109,22 +152,38 @@ private byte[] JOSEToDER(byte[] joseSignature) throws SignatureException {
}

// DER Structure: http://crypto.stackexchange.com/a/1797
// Header with length info
// Header with signature length info
derSignature[0] = (byte) 0x30;
derSignature[offset++] = (byte) length;
derSignature[offset++] = (byte) (length & 0xff);

// Header with "min R" number length
derSignature[offset++] = (byte) 0x02;
derSignature[offset++] = (byte) rLength;

// R number
System.arraycopy(joseSignature, 0, derSignature, offset + (rLength - ecNumberSize), ecNumberSize);
offset += rLength;
if (rPadding < 0) {
//Sign
derSignature[offset++] = (byte) 0x00;
System.arraycopy(joseSignature, 0, derSignature, offset, ecNumberSize);
offset += ecNumberSize;
} else {
int copyLength = Math.min(ecNumberSize, rLength);
System.arraycopy(joseSignature, rPadding, derSignature, offset, copyLength);
offset += copyLength;
}

// S number length
// Header with "min S" number length
derSignature[offset++] = (byte) 0x02;
derSignature[offset++] = (byte) sLength;

// S number
System.arraycopy(joseSignature, ecNumberSize, derSignature, offset + (sLength - ecNumberSize), ecNumberSize);
if (sPadding < 0) {
//Sign
derSignature[offset++] = (byte) 0x00;
System.arraycopy(joseSignature, ecNumberSize, derSignature, offset, ecNumberSize);
} else {
System.arraycopy(joseSignature, ecNumberSize + sPadding, derSignature, offset, Math.min(ecNumberSize, sLength));
}

return derSignature;
}
Expand All @@ -134,7 +193,7 @@ private int countPadding(byte[] bytes, int fromIndex, int toIndex) {
while (fromIndex + padding < toIndex && bytes[fromIndex + padding] == 0) {
padding++;
}
return bytes[fromIndex + padding] > 0x7f ? padding : padding - 1;
return (bytes[fromIndex + padding] & 0xff) > 0x7f ? padding - 1 : padding;
}

//Visible for testing
Expand Down
30 changes: 0 additions & 30 deletions lib/src/test/java/com/auth0/jwt/ConcurrentVerifyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,6 @@ public void shouldPassECDSA256VerificationWithJOSESignature() throws Exception {
concurrentVerify(verifier, token);
}

@Test
public void shouldPassECDSA256VerificationWithDERSignature() throws Exception {
String token = "eyJhbGciOiJFUzI1NiJ9.eyJpc3MiOiJhdXRoMCJ9.MEYCIQDiJWTf5jS/hFPj/0hpCWn7x1n/h+xPMjKWCs9MMusS9AIhAMcFPJVLe2A9uvb8hl8sRO2IpGoKDRpDmyH14ixNPAHW";
ECKey key = (ECKey) readPublicKeyFromFile(PUBLIC_KEY_FILE_256, "EC");
Algorithm algorithm = Algorithm.ECDSA256(key);
JWTVerifier verifier = JWTVerifier.init(algorithm).withIssuer("auth0").build();

concurrentVerify(verifier, token);
}

@Test
public void shouldPassECDSA384VerificationWithJOSESignature() throws Exception {
String token = "eyJhbGciOiJFUzM4NCJ9.eyJpc3MiOiJhdXRoMCJ9.50UU5VKNdF1wfykY8jQBKpvuHZoe6IZBJm5NvoB8bR-hnRg6ti-CHbmvoRtlLfnHfwITa_8cJMy6TenMC2g63GQHytc8rYoXqbwtS4R0Ko_AXbLFUmfxnGnMC6v4MS_z";
Expand All @@ -160,16 +150,6 @@ public void shouldPassECDSA384VerificationWithJOSESignature() throws Exception {
concurrentVerify(verifier, token);
}

@Test
public void shouldPassECDSA384VerificationWithDERSignature() throws Exception {
String token = "eyJhbGciOiJFUzM4NCJ9.eyJpc3MiOiJhdXRoMCJ9.MGUCMQDnRRTlUo10XXB/KRjyNAEqm+4dmh7ohkEmbk2+gHxtH6GdGDq2L4Idua+hG2Ut+ccCMH8CE2v/HCTMuk3pzAtoOtxkB8rXPK2KF6m8LUuEdCqPwF2yxVJn8ZxpzAur+DEv8w==";
ECKey key = (ECKey) readPublicKeyFromFile(PUBLIC_KEY_FILE_384, "EC");
Algorithm algorithm = Algorithm.ECDSA384(key);
JWTVerifier verifier = JWTVerifier.init(algorithm).withIssuer("auth0").build();

concurrentVerify(verifier, token);
}

@Test
public void shouldPassECDSA512VerificationWithJOSESignature() throws Exception {
String token = "eyJhbGciOiJFUzUxMiJ9.eyJpc3MiOiJhdXRoMCJ9.AeCJPDIsSHhwRSGZCY6rspi8zekOw0K9qYMNridP1Fu9uhrA1QrG-EUxXlE06yvmh2R7Rz0aE7kxBwrnq8L8aOBCAYAsqhzPeUvyp8fXjjgs0Eto5I0mndE2QHlgcMSFASyjHbU8wD2Rq7ZNzGQ5b2MZfpv030WGUajT-aZYWFUJHVg2";
Expand All @@ -179,14 +159,4 @@ public void shouldPassECDSA512VerificationWithJOSESignature() throws Exception {

concurrentVerify(verifier, token);
}

@Test
public void shouldPassECDSA512VerificationWithDERSignature() throws Exception {
String token = "eyJhbGciOiJFUzUxMiJ9.eyJpc3MiOiJhdXRoMCJ9.MIGIAkIB4Ik8MixIeHBFIZkJjquymLzN6Q7DQr2pgw2uJ0/UW726GsDVCsb4RTFeUTTrK+aHZHtHPRoTuTEHCuerwvxo4EICQgGALKocz3lL8qfH1444LNBLaOSNJp3RNkB5YHDEhQEsox21PMA9kau2TcxkOW9jGX6b9N9FhlGo0/mmWFhVCR1YNg==";
ECKey key = (ECKey) readPublicKeyFromFile(PUBLIC_KEY_FILE_512, "EC");
Algorithm algorithm = Algorithm.ECDSA512(key);
JWTVerifier verifier = JWTVerifier.init(algorithm).withIssuer("auth0").build();

concurrentVerify(verifier, token);
}
}
10 changes: 7 additions & 3 deletions lib/src/test/java/com/auth0/jwt/PemUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
import org.bouncycastle.util.io.pem.PemObject;
import org.bouncycastle.util.io.pem.PemReader;

import java.io.*;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;
import java.security.KeyFactory;
import java.security.NoSuchAlgorithmException;
import java.security.PrivateKey;
import java.security.PublicKey;
import java.security.interfaces.ECPublicKey;
import java.security.spec.EncodedKeySpec;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.PKCS8EncodedKeySpec;
Expand All @@ -22,7 +24,9 @@ private static byte[] parsePEMFile(File pemFile) throws IOException {
}
PemReader reader = new PemReader(new FileReader(pemFile));
PemObject pemObject = reader.readPemObject();
return pemObject.getContent();
byte[] content = pemObject.getContent();
reader.close();
return content;
}

private static PublicKey getPublicKey(byte[] keyBytes, String algorithm) {
Expand Down
Loading

0 comments on commit d396396

Please sign in to comment.