From 256a703d421ff34bfa821ecc28ce38ec74481948 Mon Sep 17 00:00:00 2001 From: Les Hazlewood <121180+lhazlewood@users.noreply.github.com> Date: Wed, 19 Mar 2025 11:00:09 -0400 Subject: [PATCH 1/2] Ensured JwkSet `keys` parameter is not `secret` by default. Each JWK element should determine which members are secret or not. Resolves #976. --- .../jsonwebtoken/impl/security/DefaultJwkSet.java | 1 - .../impl/security/DefaultJwkSetBuilderTest.groovy | 4 +++- .../impl/security/DefaultJwkSetTest.groovy | 13 ++++++++----- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultJwkSet.java b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultJwkSet.java index a4ae214db..7104991ed 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultJwkSet.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultJwkSet.java @@ -35,7 +35,6 @@ static Parameter>> param(Converter, ?> converter) { return Parameters.builder(JwkConverter.JWK_CLASS) .setConverter(converter).set() .setId("keys").setName("JSON Web Keys") - .setSecret(true) .build(); } diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetBuilderTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetBuilderTest.groovy index b94e4a07f..eeb338b2b 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetBuilderTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetBuilderTest.groovy @@ -278,7 +278,9 @@ class DefaultJwkSetBuilderTest { key_ops: ['sign', 'encrypt'] // unrelated operations ] - String msg = "Invalid Map ${DefaultJwkSet.KEYS} value: . Unable to create JWK: Unrelated key " + + String msg = "Invalid Map ${DefaultJwkSet.KEYS} value: " + + "[{kty=${badMap.kty}, k=${badMap.k}, key_ops=${badMap.key_ops}}]. " + + "Unable to create JWK: Unrelated key " + "operations are not allowed. KeyOperation [${Jwks.OP.ENCRYPT}] is unrelated to " + "[${Jwks.OP.SIGN}]." diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetTest.groovy index 03b970f33..a39fa37ef 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultJwkSetTest.groovy @@ -67,14 +67,17 @@ class DefaultJwkSetTest { } /** - * Asserts that the raw keys value is a RedactedSupplier and not a raw value due to potential sensitivity if - * the JwkSet contains secret or private JWKs. + * Asserts that the raw 'keys' value is not a RedactedSupplier per https://github.com/jwtk/jjwt/issues/976, + * but an internal secret key parameter does have a RedactedSupplier */ @Test - void testKeysFromGetIsRedactedSupplier() { + void testGetKeysNotRedactedSupplier() { def jwk = Jwks.builder().key(TestKeys.HS256).build() def set = new DefaultJwkSet(DefaultJwkSet.KEYS, [keys: [jwk]]) - def result = set.get('keys') - assertTrue result instanceof RedactedSupplier + def keys = set.get('keys') + assertFalse keys instanceof RedactedSupplier + keys = keys as List + def element = keys[0] as Map// result is an array/list, so get first JWK in the list + assertTrue element.k instanceof RedactedSupplier // 'k' is a secret property, should be redacted } } From 52335084db6aca8b5c5e20121bad4152f8dcb459 Mon Sep 17 00:00:00 2001 From: Les Hazlewood <121180+lhazlewood@users.noreply.github.com> Date: Wed, 19 Mar 2025 11:30:07 -0400 Subject: [PATCH 2/2] Ensured JwkSet `keys` parameter is not `secret` by default. Each JWK element should determine which members are secret or not. Resolves #976. --- CHANGELOG.md | 1 + .../java/io/jsonwebtoken/impl/security/JwkSetConverter.java | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a8fb137a..5add327aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ This patch release: [Issue 947](https://github.com/jwtk/jjwt/issues/947). * Fixes a decompression memory leak in concurrent/multi-threaded environments introduced in 0.12.0 when decompressing JWTs with a `zip` header of `GZIP`. See [Issue 949](https://github.com/jwtk/jjwt/issues/949). * Upgrades BouncyCastle to 1.78 via [PR 941](https://github.com/jwtk/jjwt/pull/941). +* Ensures that a `JwkSet`'s `keys` list member is no longer considered secret and is not redacted by default. However, each individual JWK element within the `keys` list may still have [redacted private or secret members](https://github.com/jwtk/jjwt?tab=readme-ov-file#jwk-tostring-safety) as expected. See [Issue 976](https://github.com/jwtk/jjwt/issues/976). ### 0.12.5 diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/JwkSetConverter.java b/impl/src/main/java/io/jsonwebtoken/impl/security/JwkSetConverter.java index b5a49f279..c13e9933d 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/security/JwkSetConverter.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/JwkSetConverter.java @@ -91,9 +91,6 @@ public JwkSet applyFrom(Object o) { String msg = "JWK Set " + PARAM + " value cannot be null."; throw new MalformedKeySetException(msg); } - if (val instanceof Supplier) { - val = ((Supplier) val).get(); - } if (!(val instanceof Collection)) { String msg = "JWK Set " + PARAM + " value must be a Collection (JSON Array). Type found: " + val.getClass().getName();