Skip to content

Commit 064a01e

Browse files
lbalmacedalbalmaceda
committed
fix ECDSA signature encoding
Use a copy of the ECDSA tests with BouncyCastle provider
1 parent 1d060b9 commit 064a01e

File tree

7 files changed

+1573
-67
lines changed

7 files changed

+1573
-67
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ Temporary Items
4242

4343
# IntelliJ
4444
/out/
45+
/lib/out/
4546

4647
# mpeltonen/sbt-idea plugin
4748
.idea_modules/

‎lib/build.gradle‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ compileJava {
3636
dependencies {
3737
compile 'com.fasterxml.jackson.core:jackson-databind:2.8.4'
3838
compile 'commons-codec:commons-codec:1.10'
39-
compile 'org.bouncycastle:bcprov-jdk15on:1.55'
39+
testCompile 'org.bouncycastle:bcprov-jdk15on:1.58'
4040
testCompile 'junit:junit:4.12'
4141
testCompile 'net.jodah:concurrentunit:0.4.2'
42-
testCompile 'org.hamcrest:hamcrest-library:1.3'
42+
testCompile 'org.hamcrest:java-hamcrest:2.0.0.0'
4343
testCompile 'org.mockito:mockito-core:2.2.8'
4444
}
4545

‎lib/src/main/java/com/auth0/jwt/algorithms/ECDSAAlgorithm.java‎

Lines changed: 78 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,7 @@ public void verify(DecodedJWT jwt) throws SignatureVerificationException {
4444
if (publicKey == null) {
4545
throw new IllegalStateException("The given Public Key is null.");
4646
}
47-
if (!isDERSignature(signatureBytes)) {
48-
signatureBytes = JOSEToDER(signatureBytes);
49-
}
50-
boolean valid = crypto.verifySignatureFor(getDescription(), publicKey, contentBytes, signatureBytes);
47+
boolean valid = crypto.verifySignatureFor(getDescription(), publicKey, contentBytes, JOSEToDER(signatureBytes));
5148

5249
if (!valid) {
5350
throw new SignatureVerificationException(this);
@@ -64,7 +61,8 @@ public byte[] sign(byte[] contentBytes) throws SignatureGenerationException {
6461
if (privateKey == null) {
6562
throw new IllegalStateException("The given Private Key is null.");
6663
}
67-
return crypto.createSignatureFor(getDescription(), privateKey, contentBytes);
64+
byte[] signature = crypto.createSignatureFor(getDescription(), privateKey, contentBytes);
65+
return DERToJOSE(signature);
6866
} catch (NoSuchAlgorithmException | SignatureException | InvalidKeyException | IllegalStateException e) {
6967
throw new SignatureGenerationException(this, e);
7068
}
@@ -75,15 +73,60 @@ public String getSigningKeyId() {
7573
return keyProvider.getPrivateKeyId();
7674
}
7775

78-
private boolean isDERSignature(byte[] signature) {
76+
//Visible for testing
77+
byte[] DERToJOSE(byte[] derSignature) throws SignatureException {
7978
// DER Structure: http://crypto.stackexchange.com/a/1797
80-
// Should begin with 0x30 and have exactly the expected length
81-
return signature[0] == 0x30 && signature.length != ecNumberSize * 2;
79+
boolean derEncoded = derSignature[0] == 0x30 && derSignature.length != ecNumberSize * 2;
80+
if (!derEncoded) {
81+
throw new SignatureException("Invalid DER signature format.");
82+
}
83+
84+
final byte[] joseSignature = new byte[ecNumberSize * 2];
85+
86+
//Skip 0x30
87+
int offset = 1;
88+
if (derSignature[1] == (byte) 0x81) {
89+
//Skip sign
90+
offset++;
91+
}
92+
93+
//Convert to unsigned. Should match DER length - offset
94+
int encodedLength = derSignature[offset++] & 0xff;
95+
if (encodedLength != derSignature.length - offset) {
96+
throw new SignatureException("Invalid DER signature format.");
97+
}
98+
99+
//Skip 0x02
100+
offset++;
101+
102+
//Obtain R number length (Includes padding) and skip it
103+
int rLength = derSignature[offset++];
104+
if (rLength > ecNumberSize + 1) {
105+
throw new SignatureException("Invalid DER signature format.");
106+
}
107+
int rPadding = ecNumberSize - rLength;
108+
//Retrieve R number
109+
System.arraycopy(derSignature, offset + Math.max(-rPadding, 0), joseSignature, Math.max(rPadding, 0), rLength + Math.min(rPadding, 0));
110+
111+
//Skip R number and 0x02
112+
offset += rLength + 1;
113+
114+
//Obtain S number length. (Includes padding)
115+
int sLength = derSignature[offset++];
116+
if (sLength > ecNumberSize + 1) {
117+
throw new SignatureException("Invalid DER signature format.");
118+
}
119+
int sPadding = ecNumberSize - sLength;
120+
//Retrieve R number
121+
System.arraycopy(derSignature, offset + Math.max(-sPadding, 0), joseSignature, ecNumberSize + Math.max(sPadding, 0), sLength + Math.min(sPadding, 0));
122+
123+
return joseSignature;
82124
}
83125

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

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

95138
int length = 2 + rLength + 2 + sLength;
96139
if (length > 255) {
97-
throw new SignatureException("Invalid ECDSA signature format");
140+
throw new SignatureException("Invalid JOSE signature format.");
98141
}
99142

100-
byte[] derSignature;
143+
final byte[] derSignature;
101144
int offset;
102145
if (length > 0x7f) {
103146
derSignature = new byte[3 + length];
@@ -109,22 +152,38 @@ private byte[] JOSEToDER(byte[] joseSignature) throws SignatureException {
109152
}
110153

111154
// DER Structure: http://crypto.stackexchange.com/a/1797
112-
// Header with length info
155+
// Header with signature length info
113156
derSignature[0] = (byte) 0x30;
114-
derSignature[offset++] = (byte) length;
157+
derSignature[offset++] = (byte) (length & 0xff);
158+
159+
// Header with "min R" number length
115160
derSignature[offset++] = (byte) 0x02;
116161
derSignature[offset++] = (byte) rLength;
117162

118163
// R number
119-
System.arraycopy(joseSignature, 0, derSignature, offset + (rLength - ecNumberSize), ecNumberSize);
120-
offset += rLength;
164+
if (rPadding < 0) {
165+
//Sign
166+
derSignature[offset++] = (byte) 0x00;
167+
System.arraycopy(joseSignature, 0, derSignature, offset, ecNumberSize);
168+
offset += ecNumberSize;
169+
} else {
170+
int copyLength = Math.min(ecNumberSize, rLength);
171+
System.arraycopy(joseSignature, rPadding, derSignature, offset, copyLength);
172+
offset += copyLength;
173+
}
121174

122-
// S number length
175+
// Header with "min S" number length
123176
derSignature[offset++] = (byte) 0x02;
124177
derSignature[offset++] = (byte) sLength;
125178

126179
// S number
127-
System.arraycopy(joseSignature, ecNumberSize, derSignature, offset + (sLength - ecNumberSize), ecNumberSize);
180+
if (sPadding < 0) {
181+
//Sign
182+
derSignature[offset++] = (byte) 0x00;
183+
System.arraycopy(joseSignature, ecNumberSize, derSignature, offset, ecNumberSize);
184+
} else {
185+
System.arraycopy(joseSignature, ecNumberSize + sPadding, derSignature, offset, Math.min(ecNumberSize, sLength));
186+
}
128187

129188
return derSignature;
130189
}
@@ -134,7 +193,7 @@ private int countPadding(byte[] bytes, int fromIndex, int toIndex) {
134193
while (fromIndex + padding < toIndex && bytes[fromIndex + padding] == 0) {
135194
padding++;
136195
}
137-
return bytes[fromIndex + padding] > 0x7f ? padding : padding - 1;
196+
return (bytes[fromIndex + padding] & 0xff) > 0x7f ? padding - 1 : padding;
138197
}
139198

140199
//Visible for testing

‎lib/src/test/java/com/auth0/jwt/ConcurrentVerifyTest.java‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -140,16 +140,6 @@ public void shouldPassECDSA256VerificationWithJOSESignature() throws Exception {
140140
concurrentVerify(verifier, token);
141141
}
142142

143-
@Test
144-
public void shouldPassECDSA256VerificationWithDERSignature() throws Exception {
145-
String token = "eyJhbGciOiJFUzI1NiJ9.eyJpc3MiOiJhdXRoMCJ9.MEYCIQDiJWTf5jS/hFPj/0hpCWn7x1n/h+xPMjKWCs9MMusS9AIhAMcFPJVLe2A9uvb8hl8sRO2IpGoKDRpDmyH14ixNPAHW";
146-
ECKey key = (ECKey) readPublicKeyFromFile(PUBLIC_KEY_FILE_256, "EC");
147-
Algorithm algorithm = Algorithm.ECDSA256(key);
148-
JWTVerifier verifier = JWTVerifier.init(algorithm).withIssuer("auth0").build();
149-
150-
concurrentVerify(verifier, token);
151-
}
152-
153143
@Test
154144
public void shouldPassECDSA384VerificationWithJOSESignature() throws Exception {
155145
String token = "eyJhbGciOiJFUzM4NCJ9.eyJpc3MiOiJhdXRoMCJ9.50UU5VKNdF1wfykY8jQBKpvuHZoe6IZBJm5NvoB8bR-hnRg6ti-CHbmvoRtlLfnHfwITa_8cJMy6TenMC2g63GQHytc8rYoXqbwtS4R0Ko_AXbLFUmfxnGnMC6v4MS_z";
@@ -160,16 +150,6 @@ public void shouldPassECDSA384VerificationWithJOSESignature() throws Exception {
160150
concurrentVerify(verifier, token);
161151
}
162152

163-
@Test
164-
public void shouldPassECDSA384VerificationWithDERSignature() throws Exception {
165-
String token = "eyJhbGciOiJFUzM4NCJ9.eyJpc3MiOiJhdXRoMCJ9.MGUCMQDnRRTlUo10XXB/KRjyNAEqm+4dmh7ohkEmbk2+gHxtH6GdGDq2L4Idua+hG2Ut+ccCMH8CE2v/HCTMuk3pzAtoOtxkB8rXPK2KF6m8LUuEdCqPwF2yxVJn8ZxpzAur+DEv8w==";
166-
ECKey key = (ECKey) readPublicKeyFromFile(PUBLIC_KEY_FILE_384, "EC");
167-
Algorithm algorithm = Algorithm.ECDSA384(key);
168-
JWTVerifier verifier = JWTVerifier.init(algorithm).withIssuer("auth0").build();
169-
170-
concurrentVerify(verifier, token);
171-
}
172-
173153
@Test
174154
public void shouldPassECDSA512VerificationWithJOSESignature() throws Exception {
175155
String token = "eyJhbGciOiJFUzUxMiJ9.eyJpc3MiOiJhdXRoMCJ9.AeCJPDIsSHhwRSGZCY6rspi8zekOw0K9qYMNridP1Fu9uhrA1QrG-EUxXlE06yvmh2R7Rz0aE7kxBwrnq8L8aOBCAYAsqhzPeUvyp8fXjjgs0Eto5I0mndE2QHlgcMSFASyjHbU8wD2Rq7ZNzGQ5b2MZfpv030WGUajT-aZYWFUJHVg2";
@@ -179,14 +159,4 @@ public void shouldPassECDSA512VerificationWithJOSESignature() throws Exception {
179159

180160
concurrentVerify(verifier, token);
181161
}
182-
183-
@Test
184-
public void shouldPassECDSA512VerificationWithDERSignature() throws Exception {
185-
String token = "eyJhbGciOiJFUzUxMiJ9.eyJpc3MiOiJhdXRoMCJ9.MIGIAkIB4Ik8MixIeHBFIZkJjquymLzN6Q7DQr2pgw2uJ0/UW726GsDVCsb4RTFeUTTrK+aHZHtHPRoTuTEHCuerwvxo4EICQgGALKocz3lL8qfH1444LNBLaOSNJp3RNkB5YHDEhQEsox21PMA9kau2TcxkOW9jGX6b9N9FhlGo0/mmWFhVCR1YNg==";
186-
ECKey key = (ECKey) readPublicKeyFromFile(PUBLIC_KEY_FILE_512, "EC");
187-
Algorithm algorithm = Algorithm.ECDSA512(key);
188-
JWTVerifier verifier = JWTVerifier.init(algorithm).withIssuer("auth0").build();
189-
190-
concurrentVerify(verifier, token);
191-
}
192162
}

‎lib/src/test/java/com/auth0/jwt/PemUtils.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33
import org.bouncycastle.util.io.pem.PemObject;
44
import org.bouncycastle.util.io.pem.PemReader;
55

6-
import java.io.*;
6+
import java.io.File;
7+
import java.io.FileNotFoundException;
8+
import java.io.FileReader;
9+
import java.io.IOException;
710
import java.security.KeyFactory;
811
import java.security.NoSuchAlgorithmException;
912
import java.security.PrivateKey;
1013
import java.security.PublicKey;
11-
import java.security.interfaces.ECPublicKey;
1214
import java.security.spec.EncodedKeySpec;
1315
import java.security.spec.InvalidKeySpecException;
1416
import java.security.spec.PKCS8EncodedKeySpec;
@@ -22,7 +24,9 @@ private static byte[] parsePEMFile(File pemFile) throws IOException {
2224
}
2325
PemReader reader = new PemReader(new FileReader(pemFile));
2426
PemObject pemObject = reader.readPemObject();
25-
return pemObject.getContent();
27+
byte[] content = pemObject.getContent();
28+
reader.close();
29+
return content;
2630
}
2731

2832
private static PublicKey getPublicKey(byte[] keyBytes, String algorithm) {

0 commit comments

Comments
 (0)