Skip to content

Commit 628bd6f

Browse files
lhazlewoodlhazlewood
committed
Secret JWK k values larger than HMAC-SHA minimums (#909)
- Ensured Secret JWK 'k' byte arrays for HMAC-SHA algorithms can be larger than the identified HS* algorithm. This is allowed per https://datatracker.ietf.org/doc/html/rfc7518#section-3.2: "A key of the same size as the hash output ... _or larger_ MUST be used with this algorithm" - Ensured that, when using the JwkBuilder, Secret JWK 'alg' values would automatically be set to 'HS256', 'HS384', or 'HS512' if the specified Java SecretKey algorithm name equals a JCA standard name (HmacSHA256, HmacSHA384, etc) or JCA standard HMAC-SHA OID. - Updated CHANGELOG.md accordingly. Fixes #905
1 parent b12dabf commit 628bd6f

File tree

8 files changed

+191
-53
lines changed

8 files changed

+191
-53
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ This release also:
6565
[6.2.1.3](https://datatracker.ietf.org/doc/html/rfc7518#section-6.2.1.3), and
6666
[6.2.2.1](https://datatracker.ietf.org/doc/html/rfc7518#section-6.2.2.1), respectively.
6767
[Issue 901](https://github.com/jwtk/jjwt/issues/901).
68+
* Ensures that Secret JWKs for HMAC-SHA algorithms with `k` sizes larger than the algorithm minimum can
69+
be parsed/used as expected. See [Issue #905](https://github.com/jwtk/jjwt/issues/905)
6870
* Fixes various typos in documentation and JavaDoc. Thanks to those contributing pull requests for these!
6971

7072
### 0.12.3

‎impl/src/main/java/io/jsonwebtoken/impl/security/AesAlgorithm.java‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import javax.crypto.SecretKey;
3131
import javax.crypto.spec.GCMParameterSpec;
3232
import javax.crypto.spec.IvParameterSpec;
33+
import javax.crypto.spec.SecretKeySpec;
3334
import java.io.InputStream;
3435
import java.io.OutputStream;
3536
import java.security.SecureRandom;
@@ -54,9 +55,22 @@ abstract class AesAlgorithm extends CryptoAlgorithm implements KeyBuilderSupplie
5455
protected final int tagBitLength;
5556
protected final boolean gcm;
5657

58+
static void assertKeyBitLength(int keyBitLength) {
59+
if (keyBitLength == 128 || keyBitLength == 192 || keyBitLength == 256) return; // valid
60+
String msg = "Invalid AES key length: " + Bytes.bitsMsg(keyBitLength) + ". AES only supports " +
61+
"128, 192, or 256 bit keys.";
62+
throw new IllegalArgumentException(msg);
63+
}
64+
65+
static SecretKey keyFor(byte[] bytes) {
66+
int bitlen = (int) Bytes.bitLength(bytes);
67+
assertKeyBitLength(bitlen);
68+
return new SecretKeySpec(bytes, KEY_ALG_NAME);
69+
}
70+
5771
AesAlgorithm(String id, final String jcaTransformation, int keyBitLength) {
5872
super(id, jcaTransformation);
59-
Assert.isTrue(keyBitLength == 128 || keyBitLength == 192 || keyBitLength == 256, "Invalid AES key length: it must equal 128, 192, or 256.");
73+
assertKeyBitLength(keyBitLength);
6074
this.keyBitLength = keyBitLength;
6175
this.gcm = jcaTransformation.startsWith("AES/GCM");
6276
this.ivBitLength = jcaTransformation.equals("AESWrap") ? 0 : (this.gcm ? GCM_IV_SIZE : BLOCK_SIZE);

‎impl/src/main/java/io/jsonwebtoken/impl/security/SecretJwkFactory.java‎

Lines changed: 69 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,22 @@
1515
*/
1616
package io.jsonwebtoken.impl.security;
1717

18+
import io.jsonwebtoken.Identifiable;
1819
import io.jsonwebtoken.Jwts;
1920
import io.jsonwebtoken.impl.lang.Bytes;
2021
import io.jsonwebtoken.impl.lang.ParameterReadable;
2122
import io.jsonwebtoken.impl.lang.RequiredParameterReader;
2223
import io.jsonwebtoken.io.Encoders;
2324
import io.jsonwebtoken.lang.Assert;
2425
import io.jsonwebtoken.lang.Strings;
26+
import io.jsonwebtoken.security.AeadAlgorithm;
2527
import io.jsonwebtoken.security.InvalidKeyException;
28+
import io.jsonwebtoken.security.Keys;
2629
import io.jsonwebtoken.security.MacAlgorithm;
2730
import io.jsonwebtoken.security.MalformedKeyException;
2831
import io.jsonwebtoken.security.SecretJwk;
29-
import io.jsonwebtoken.security.SecureDigestAlgorithm;
32+
import io.jsonwebtoken.security.SecretKeyAlgorithm;
33+
import io.jsonwebtoken.security.WeakKeyException;
3034

3135
import javax.crypto.SecretKey;
3236
import javax.crypto.spec.SecretKeySpec;
@@ -44,61 +48,97 @@ class SecretJwkFactory extends AbstractFamilyJwkFactory<SecretKey, SecretJwk> {
4448
protected SecretJwk createJwkFromKey(JwkContext<SecretKey> ctx) {
4549
SecretKey key = Assert.notNull(ctx.getKey(), "JwkContext key cannot be null.");
4650
String k;
51+
byte[] encoded = null;
4752
try {
48-
byte[] encoded = KeysBridge.getEncoded(key);
53+
encoded = KeysBridge.getEncoded(key);
4954
k = Encoders.BASE64URL.encode(encoded);
5055
Assert.hasText(k, "k value cannot be null or empty.");
5156
} catch (Throwable t) {
5257
String msg = "Unable to encode SecretKey to JWK: " + t.getMessage();
5358
throw new InvalidKeyException(msg, t);
59+
} finally {
60+
Bytes.clear(encoded);
61+
}
62+
63+
MacAlgorithm mac = DefaultMacAlgorithm.findByKey(key);
64+
if (mac != null) {
65+
ctx.put(AbstractJwk.ALG.getId(), mac.getId());
5466
}
5567

5668
ctx.put(DefaultSecretJwk.K.getId(), k);
5769

58-
return new DefaultSecretJwk(ctx);
70+
return createJwkFromValues(ctx);
5971
}
6072

6173
private static void assertKeyBitLength(byte[] bytes, MacAlgorithm alg) {
6274
long bitLen = Bytes.bitLength(bytes);
6375
long requiredBitLen = alg.getKeyBitLength();
64-
if (bitLen != requiredBitLen) {
76+
if (bitLen < requiredBitLen) {
6577
// Implementors note: Don't print out any information about the `bytes` value itself - size,
6678
// content, etc., as it is considered secret material:
6779
String msg = "Secret JWK " + AbstractJwk.ALG + " value is '" + alg.getId() +
68-
"', but the " + DefaultSecretJwk.K + " length does not equal the '" + alg.getId() +
69-
"' length requirement of " + Bytes.bitsMsg(requiredBitLen) +
70-
". This discrepancy could be the result of an algorithm " +
71-
"substitution attack or simply an erroneously constructed JWK. In either case, it is likely " +
72-
"to result in unexpected or undesired security consequences.";
73-
throw new MalformedKeyException(msg);
80+
"', but the " + DefaultSecretJwk.K + " length is smaller than the " + alg.getId() +
81+
" minimum length of " + Bytes.bitsMsg(requiredBitLen) +
82+
" required by " +
83+
"[JWA RFC 7518, Section 3.2](https://www.rfc-editor.org/rfc/rfc7518.html#section-3.2), " +
84+
"2nd paragraph: 'A key of the same size as the hash output or larger MUST be used with this " +
85+
"algorithm.'";
86+
throw new WeakKeyException(msg);
7487
}
7588
}
7689

90+
private static void assertSymmetric(Identifiable alg) {
91+
if (alg instanceof MacAlgorithm || alg instanceof SecretKeyAlgorithm || alg instanceof AeadAlgorithm)
92+
return; // valid
93+
String msg = "Invalid Secret JWK " + AbstractJwk.ALG + " value '" + alg.getId() + "'. Secret JWKs " +
94+
"may only be used with symmetric (secret) key algorithms.";
95+
throw new MalformedKeyException(msg);
96+
}
97+
7798
@Override
7899
protected SecretJwk createJwkFromValues(JwkContext<SecretKey> ctx) {
79100
ParameterReadable reader = new RequiredParameterReader(ctx);
80-
byte[] bytes = reader.get(DefaultSecretJwk.K);
81-
String jcaName = null;
82-
83-
String id = ctx.getAlgorithm();
84-
if (Strings.hasText(id)) {
85-
SecureDigestAlgorithm<?, ?> alg = Jwts.SIG.get().get(id);
86-
if (alg instanceof MacAlgorithm) {
87-
jcaName = ((CryptoAlgorithm) alg).getJcaName(); // valid for all JJWT alg implementations
88-
Assert.hasText(jcaName, "Algorithm jcaName cannot be null or empty.");
89-
assertKeyBitLength(bytes, (MacAlgorithm) alg);
90-
}
91-
}
92-
if (!Strings.hasText(jcaName)) {
93-
if (ctx.isSigUse()) {
101+
final byte[] bytes = reader.get(DefaultSecretJwk.K);
102+
SecretKey key;
103+
104+
String algId = ctx.getAlgorithm();
105+
if (!Strings.hasText(algId)) { // optional per https://www.rfc-editor.org/rfc/rfc7517.html#section-4.4
106+
107+
// Here we try to infer the best type of key to create based on siguse and/or key length.
108+
//
109+
// AES requires 128, 192, or 256 bits, so anything larger than 256 cannot be AES, so we'll need to assume
110+
// HMAC.
111+
//
112+
// Also, 256 bits works for either HMAC or AES, so we just have to choose one as there is no other
113+
// RFC-based criteria for determining. Historically, we've chosen AES due to the larger number of
114+
// KeyAlgorithm and AeadAlgorithm use cases, so that's our default.
115+
int kBitLen = (int) Bytes.bitLength(bytes);
116+
117+
if (ctx.isSigUse() || kBitLen > Jwts.SIG.HS256.getKeyBitLength()) {
94118
// The only JWA SecretKey signature algorithms are HS256, HS384, HS512, so choose based on bit length:
95-
jcaName = "HmacSHA" + Bytes.bitLength(bytes);
96-
} else { // not an HS* algorithm, and all standard AeadAlgorithms use AES keys:
97-
jcaName = AesAlgorithm.KEY_ALG_NAME;
119+
key = Keys.hmacShaKeyFor(bytes);
120+
} else {
121+
key = AesAlgorithm.keyFor(bytes);
98122
}
123+
ctx.setKey(key);
124+
return new DefaultSecretJwk(ctx);
125+
}
126+
127+
//otherwise 'alg' was specified, ensure it's valid for secret key use:
128+
Identifiable alg = Jwts.SIG.get().get(algId);
129+
if (alg == null) alg = Jwts.KEY.get().get(algId);
130+
if (alg == null) alg = Jwts.ENC.get().get(algId);
131+
if (alg != null) assertSymmetric(alg); // if we found a standard alg, it must be a symmetric key algorithm
132+
133+
if (alg instanceof MacAlgorithm) {
134+
assertKeyBitLength(bytes, ((MacAlgorithm) alg));
135+
String jcaName = ((CryptoAlgorithm) alg).getJcaName();
136+
Assert.hasText(jcaName, "Algorithm jcaName cannot be null or empty.");
137+
key = new SecretKeySpec(bytes, jcaName);
138+
} else {
139+
// all other remaining JWA-standard symmetric algs use AES:
140+
key = AesAlgorithm.keyFor(bytes);
99141
}
100-
Assert.stateNotNull(jcaName, "jcaName cannot be null (invariant)");
101-
SecretKey key = new SecretKeySpec(bytes, jcaName);
102142
ctx.setKey(key);
103143
return new DefaultSecretJwk(ctx);
104144
}

‎impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractJwkBuilderTest.groovy‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,8 @@ import static org.junit.Assert.*
2929

3030
class AbstractJwkBuilderTest {
3131

32-
private static final SecretKey SKEY = TestKeys.A256GCM
33-
3432
private static AbstractJwkBuilder<SecretKey, SecretJwk, AbstractJwkBuilder> builder() {
35-
return (AbstractJwkBuilder) Jwks.builder().key(SKEY)
33+
return (AbstractJwkBuilder) Jwks.builder().key(TestKeys.NA256)
3634
}
3735

3836
@Test

‎impl/src/test/groovy/io/jsonwebtoken/impl/security/JwkSerializationTest.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ class JwkSerializationTest {
9797

9898
static void testSecretJwk(Serializer ser, Deserializer des) {
9999

100-
def key = TestKeys.A128GCM
100+
def key = TestKeys.NA256
101101
def jwk = Jwks.builder().key(key).id('id').build()
102102
assertWrapped(jwk, ['k'])
103103

‎impl/src/test/groovy/io/jsonwebtoken/impl/security/JwksTest.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import static org.junit.Assert.*
4040

4141
class JwksTest {
4242

43-
private static final SecretKey SKEY = Jwts.SIG.HS256.key().build()
43+
private static final SecretKey SKEY = TestKeys.NA256
4444
private static final java.security.KeyPair EC_PAIR = Jwts.SIG.ES256.keyPair().build()
4545

4646
private static String srandom() {
@@ -172,7 +172,7 @@ class JwksTest {
172172
@Test
173173
void testOperations() {
174174
def val = [Jwks.OP.SIGN, Jwks.OP.VERIFY] as Set<KeyOperation>
175-
def jwk = Jwks.builder().key(TestKeys.A128GCM).operations().add(val).and().build()
175+
def jwk = Jwks.builder().key(TestKeys.NA256).operations().add(val).and().build()
176176
assertEquals val, jwk.getOperations()
177177
}
178178

0 commit comments

Comments
 (0)