From 083da95a263675f2f5bfb27333128cf77a6c5609 Mon Sep 17 00:00:00 2001 From: Les Hazlewood <121180+lhazlewood@users.noreply.github.com> Date: Tue, 8 Aug 2023 15:46:40 -0700 Subject: [PATCH] - removed DynamicJwkBuilder chain methods with array arguments - added generic DynamicJwkBuilder#keyPair(KeyPair) method - added generic DynamicJwkBuilder#chain method --- .../security/DynamicJwkBuilder.java | 143 ++++++++++++------ .../security/DefaultDynamicJwkBuilder.java | 30 ++-- .../impl/security/JwksTest.groovy | 82 ++++++++-- .../io/jsonwebtoken/impl/security/README.md | 2 +- 4 files changed, 181 insertions(+), 76 deletions(-) diff --git a/api/src/main/java/io/jsonwebtoken/security/DynamicJwkBuilder.java b/api/src/main/java/io/jsonwebtoken/security/DynamicJwkBuilder.java index d2e6b4151..5ac13fecf 100644 --- a/api/src/main/java/io/jsonwebtoken/security/DynamicJwkBuilder.java +++ b/api/src/main/java/io/jsonwebtoken/security/DynamicJwkBuilder.java @@ -37,6 +37,48 @@ */ public interface DynamicJwkBuilder> extends JwkBuilder> { + /** + * Ensures the builder will create a {@link PublicJwk} for the specified Java {@link X509Certificate} chain. + * The first {@code X509Certificate} in the chain (at array index 0) MUST contain a {@link PublicKey} + * instance when calling the certificate's {@link X509Certificate#getPublicKey() getPublicKey()} method. + * + *

This method is provided for congruence with the other {@code chain} methods and is expected to be used when + * the calling code has a variable {@code PublicKey} reference. Based on the argument type, it will + * delegate to one of the following methods if possible: + *

+ * + *

If the specified {@code chain} argument is not capable of being supported by one of those methods, an + * {@link UnsupportedKeyException} will be thrown.

+ * + *

Type Parameters

+ * + *

In addition to the public key type A, the public key's associated private key type + * B is parameterized as well. This ensures that any subsequent call to the builder's + * {@link PublicJwkBuilder#privateKey(PrivateKey) privateKey} method will be type-safe. For example:

+ * + * 
Jwks.builder().<EdECPublicKey, EdECPrivateKey>chain(edECPublicKeyX509CertificateChain)
+     *     .privateKey(aPrivateKey) // <-- must be an EdECPrivateKey instance
+     *     ... etc ...
+     *     .build();
+ * + * @param the type of {@link PublicKey} provided by the created public JWK. + * @param the type of {@link PrivateKey} that may be paired with the {@link PublicKey} to produce a + * {@link PrivateJwk} if desired. + * @param chain the {@link X509Certificate} chain to inspect to find the {@link PublicKey} to represent as a + * {@link PublicJwk}. + * @return the builder coerced as a {@link PublicJwkBuilder} for continued method chaining. + * @throws UnsupportedKeyException if the specified key is not a supported type and cannot be used to delegate to + * other {@code key} methods. + * @see PublicJwk + * @see PrivateJwk + */ + PublicJwkBuilder chain(List chain) + throws UnsupportedKeyException; + /** * Ensures the builder will create a {@link SecretJwk} for the specified Java {@link SecretKey}. * @@ -87,7 +129,6 @@ public interface DynamicJwkBuilder> extends JwkB */ EcPrivateJwkBuilder key(ECPrivateKey key); - /** * Ensures the builder will create a {@link PublicJwk} for the specified Java {@link PublicKey} argument. This * method is provided for congruence with the other {@code key} methods and is expected to be used when @@ -161,6 +202,45 @@ public interface DynamicJwkBuilder> extends JwkB */ PrivateJwkBuilder key(B key) throws UnsupportedKeyException; + /** + * Ensures the builder will create a {@link PrivateJwk} for the specified Java {@link KeyPair} argument. This + * method is provided for congruence with the other {@code keyPair} methods and is expected to be used when + * the calling code has a variable {@code PrivateKey} reference. Based on the argument's {@code PrivateKey} type, + * it will delegate to one of the following methods if possible: + *
    + *
  • {@link #key(RSAPrivateKey)}
    • + *
  • {@link #key(ECPrivateKey)}
    • + *
  • {@link #octetKey(PrivateKey)}
    • + *
+ *

and automatically set the resulting builder's {@link PrivateJwkBuilder#publicKey(PublicKey) publicKey} with + * the pair's {@code PublicKey}.

+ * + *

If the specified {@code key} argument is not capable of being supported by one of those methods, an + * {@link UnsupportedKeyException} will be thrown.

+ * + *

Type Parameters

+ * + *

In addition to the private key type B, the private key's associated public key type + * A is parameterized as well. This ensures that any subsequent call to the builder's + * {@link PrivateJwkBuilder#publicKey(PublicKey) publicKey} method will be type-safe. For example:

+ * + * 
Jwks.builder().<EdECPublicKey, EdECPrivateKey>keyPair(anEdECKeyPair)
+     *     .publicKey(aPublicKey) // <-- must be an EdECPublicKey instance
+     *     ... etc ...
+     *     .build();
+ * + * @param the {@code keyPair} argument's {@link PublicKey} type + * @param the {@code keyPair} argument's {@link PrivateKey} type + * @param keyPair the {@code KeyPair} containing the public and private key + * @return the builder coerced as a {@link PrivateJwkBuilder} for continued method chaining. + * @throws UnsupportedKeyException if the specified {@code KeyPair}'s keys are not supported and cannot be used to + * delegate to other {@code key} methods. + * @see PublicJwk + * @see PrivateJwk + */ + PrivateJwkBuilder keyPair(KeyPair keyPair) + throws UnsupportedKeyException; + /** * Ensures the builder will create an {@link OctetPublicJwk} for the specified Edwards-curve {@code PublicKey} * argument. The {@code PublicKey} must be an instance of one of the following: @@ -227,22 +307,6 @@ public interface DynamicJwkBuilder> extends JwkB */ OctetPrivateJwkBuilder octetKey(A key); - /** - * Ensures the builder will create an {@link OctetPrivateJwk} for the specified Java Edwards-curve - * {@link KeyPair}. The pair's {@link KeyPair#getPublic() public key} MUST be an - * Edwards-curve public key as defined by {@link #octetKey(PublicKey)}. The pair's - * {@link KeyPair#getPrivate() private key} MUST be an Edwards-curve private key as defined by - * {@link #octetKey(PrivateKey)}. - * - * @param the type of Edwards-curve {@link PublicKey} contained in the key pair. - * @param the type of the Edwards-curve {@link PrivateKey} contained in the key pair. - * @param keyPair the Edwards-curve {@link KeyPair} to represent as an {@link OctetPrivateJwk}. - * @return the builder coerced as an {@link OctetPrivateJwkBuilder} for continued method chaining. - * @throws IllegalArgumentException if the {@code keyPair} does not contain Edwards-curve public and private key - * instances. - */ - OctetPrivateJwkBuilder octetKeyPair(KeyPair keyPair); - /** * Ensures the builder will create an {@link OctetPublicJwk} for the specified Java {@link X509Certificate} chain. * The first {@code X509Certificate} in the chain (at list index 0) MUST @@ -259,30 +323,20 @@ public interface DynamicJwkBuilder> extends JwkB OctetPublicJwkBuilder octetChain(List chain); /** - * Ensures the builder will create an {@link OctetPublicJwk} for the specified Java {@link X509Certificate} chain. - * The first {@code X509Certificate} in the chain (at array index 0) MUST - * {@link X509Certificate#getPublicKey() contain} an Edwards-curve public key as defined by - * {@link #octetKey(PublicKey)}. - * - * @param the type of Edwards-curve {@link PublicKey} contained in the first {@code X509Certificate}. - * @param the type of Edwards-curve {@link PrivateKey} that may be paired with the {@link PublicKey} to produce - * an {@link OctetPrivateJwk} if desired. - * @param chain the {@link X509Certificate} chain to inspect to find the Edwards-curve {@code PublicKey} to - * represent as an {@link OctetPublicJwk}. - * @return the builder coerced as an {@link OctetPublicJwkBuilder} for continued method chaining. - */ - OctetPublicJwkBuilder octetChain(X509Certificate... chain); - - /** - * Ensures the builder will create an {@link EcPublicJwk} for the specified Java {@link X509Certificate} chain. - * The first {@code X509Certificate} in the chain (at array index 0) MUST contain an {@link ECPublicKey} - * instance when calling the certificate's {@link X509Certificate#getPublicKey() getPublicKey()} method. + * Ensures the builder will create an {@link OctetPrivateJwk} for the specified Java Edwards-curve + * {@link KeyPair}. The pair's {@link KeyPair#getPublic() public key} MUST be an + * Edwards-curve public key as defined by {@link #octetKey(PublicKey)}. The pair's + * {@link KeyPair#getPrivate() private key} MUST be an Edwards-curve private key as defined by + * {@link #octetKey(PrivateKey)}. * - * @param chain the {@link X509Certificate} chain to inspect to find the {@link ECPublicKey} to represent as a - * {@link EcPublicJwk}. - * @return the builder coerced as an {@link EcPublicJwkBuilder}. + * @param the type of Edwards-curve {@link PublicKey} contained in the key pair. + * @param the type of the Edwards-curve {@link PrivateKey} contained in the key pair. + * @param keyPair the Edwards-curve {@link KeyPair} to represent as an {@link OctetPrivateJwk}. + * @return the builder coerced as an {@link OctetPrivateJwkBuilder} for continued method chaining. + * @throws IllegalArgumentException if the {@code keyPair} does not contain Edwards-curve public and private key + * instances. */ - EcPublicJwkBuilder ecChain(X509Certificate... chain); + OctetPrivateJwkBuilder octetKeyPair(KeyPair keyPair); /** * Ensures the builder will create an {@link EcPublicJwk} for the specified Java {@link X509Certificate} chain. @@ -308,17 +362,6 @@ public interface DynamicJwkBuilder> extends JwkB */ EcPrivateJwkBuilder ecKeyPair(KeyPair keyPair) throws IllegalArgumentException; - /** - * Ensures the builder will create an {@link RsaPublicJwk} for the specified Java {@link X509Certificate} chain. - * The first {@code X509Certificate} in the chain (at array index 0) MUST contain an {@link RSAPublicKey} - * instance when calling the certificate's {@link X509Certificate#getPublicKey() getPublicKey()} method. - * - * @param chain the {@link X509Certificate} chain to inspect to find the {@link RSAPublicKey} to represent as a - * {@link RsaPublicJwk}. - * @return the builder coerced as an {@link RsaPublicJwkBuilder}. - */ - RsaPublicJwkBuilder rsaChain(X509Certificate... chain); - /** * Ensures the builder will create an {@link RsaPublicJwk} for the specified Java {@link X509Certificate} chain. * The first {@code X509Certificate} in the chain (at list index 0) MUST contain an {@link RSAPublicKey} diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultDynamicJwkBuilder.java b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultDynamicJwkBuilder.java index b9f5c7c11..3cba163f8 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultDynamicJwkBuilder.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultDynamicJwkBuilder.java @@ -15,7 +15,6 @@ */ package io.jsonwebtoken.impl.security; -import io.jsonwebtoken.lang.Arrays; import io.jsonwebtoken.lang.Assert; import io.jsonwebtoken.lang.Strings; import io.jsonwebtoken.security.DynamicJwkBuilder; @@ -125,10 +124,14 @@ public OctetPrivateJwkBuilder return new AbstractAsymmetricJwkBuilder.DefaultOctetPrivateJwkBuilder<>(newContext(key)); } + @SuppressWarnings("unchecked") @Override - public RsaPublicJwkBuilder rsaChain(X509Certificate... chain) { + public PublicJwkBuilder chain(List chain) + throws UnsupportedKeyException { Assert.notEmpty(chain, "chain cannot be null or empty."); - return rsaChain(Arrays.asList(chain)); + X509Certificate cert = Assert.notNull(chain.get(0), "The first X509Certificate cannot be null."); + PublicKey key = Assert.notNull(cert.getPublicKey(), "The first X509Certificate's PublicKey cannot be null."); + return this.key((A)key).x509CertificateChain(chain); } @Override @@ -140,12 +143,6 @@ public RsaPublicJwkBuilder rsaChain(List chain) { return key(pubKey).x509CertificateChain(chain); } - @Override - public EcPublicJwkBuilder ecChain(X509Certificate... chain) { - Assert.notEmpty(chain, "chain cannot be null or empty."); - return ecChain(Arrays.asList(chain)); - } - @Override public EcPublicJwkBuilder ecChain(List chain) { Assert.notEmpty(chain, "X509Certificate chain cannot be empty."); @@ -165,12 +162,6 @@ public OctetPrivateJwkBuilder return (OctetPrivateJwkBuilder) octetKey(priv).publicKey(pub); } - @Override - public OctetPublicJwkBuilder octetChain(X509Certificate... chain) { - Assert.notEmpty(chain, "X509Certificate chain cannot be null or empty."); - return octetChain(Arrays.asList(chain)); - } - @SuppressWarnings("unchecked") // ok because of the EdwardsCurve.assertEdwards calls @Override public OctetPublicJwkBuilder octetChain(List chain) { @@ -196,6 +187,15 @@ public EcPrivateJwkBuilder ecKeyPair(KeyPair pair) { return key(priv).publicKey(pub); } + @SuppressWarnings("unchecked") + @Override + public PrivateJwkBuilder keyPair(KeyPair keyPair) + throws UnsupportedKeyException { + A pub = (A)KeyPairs.getKey(keyPair, PublicKey.class); + B priv = (B)KeyPairs.getKey(keyPair, PrivateKey.class); + return this.key(priv).publicKey(pub); + } + @Override public J build() { if (Strings.hasText(this.DELEGATE.get(AbstractJwk.KTY))) { diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/JwksTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/JwksTest.groovy index 25c9befa4..bc398f6a7 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/JwksTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/JwksTest.groovy @@ -31,6 +31,7 @@ import java.security.cert.X509Certificate import java.security.interfaces.ECKey import java.security.interfaces.ECPublicKey import java.security.interfaces.RSAKey +import java.security.interfaces.RSAPublicKey import java.security.spec.ECParameterSpec import java.security.spec.ECPoint @@ -186,16 +187,7 @@ class JwksTest { for (def alg : algs) { //get test cert: X509Certificate cert = TestCertificates.readTestCertificate(alg) - def pubKey = cert.getPublicKey() - - def builder = Jwks.builder() - if (pubKey instanceof ECKey) { - builder = builder.ecChain(cert) - } else if (pubKey instanceof RSAKey) { - builder = builder.rsaChain(cert) - } else { - builder = builder.octetChain(cert) - } + def builder = Jwks.builder().chain(Arrays.asList(cert)) if (number == 1) { builder.withX509Sha1Thumbprint(true) @@ -415,6 +407,76 @@ class JwksTest { } } + @Test + void testEcChain() { + TestKeys.EC.each { + ECPublicKey key = it.pair.public as ECPublicKey + def jwk = Jwks.builder().ecChain(it.chain).build() + assertEquals key, jwk.toKey() + assertEquals it.chain, jwk.getX509CertificateChain() + } + } + + @Test + void testRsaChain() { + TestKeys.RSA.each { + RSAPublicKey key = it.pair.public as RSAPublicKey + def jwk = Jwks.builder().rsaChain(it.chain).build() + assertEquals key, jwk.toKey() + assertEquals it.chain, jwk.getX509CertificateChain() + } + } + + @Test + void testOctetChain() { + TestKeys.EdEC.findAll({ it -> it.cert != null }).each { // no chains for XEC keys + PublicKey key = it.pair.public + def jwk = Jwks.builder().octetChain(it.chain).build() + assertEquals key, jwk.toKey() + assertEquals it.chain, jwk.getX509CertificateChain() + } + } + + @Test + void testRsaKeyPair() { + TestKeys.RSA.each { + java.security.KeyPair pair = it.pair + PrivateJwk jwk = Jwks.builder().rsaKeyPair(pair).build() + assertEquals it.pair.public, jwk.toPublicJwk().toKey() + assertEquals it.pair.private, jwk.toKey() + } + } + + @Test + void testEcKeyPair() { + TestKeys.EC.each { + java.security.KeyPair pair = it.pair + PrivateJwk jwk = Jwks.builder().ecKeyPair(pair).build() + assertEquals it.pair.public, jwk.toPublicJwk().toKey() + assertEquals it.pair.private, jwk.toKey() + } + } + + @Test + void testOctetKeyPair() { + TestKeys.EdEC.findAll(it -> it.cert != null).each { + java.security.KeyPair pair = it.pair + PrivateJwk jwk = Jwks.builder().octetKeyPair(pair).build() + assertEquals it.pair.public, jwk.toPublicJwk().toKey() + assertEquals it.pair.private, jwk.toKey() + } + } + + @Test + void testKeyPair() { + TestKeys.ASYM.each { + java.security.KeyPair pair = it.pair + PrivateJwk jwk = Jwks.builder().keyPair(pair).build() + assertEquals it.pair.public, jwk.toPublicJwk().toKey() + assertEquals it.pair.private, jwk.toKey() + } + } + private static class InvalidECPublicKey implements ECPublicKey { private final ECPublicKey good diff --git a/impl/src/test/resources/io/jsonwebtoken/impl/security/README.md b/impl/src/test/resources/io/jsonwebtoken/impl/security/README.md index 81428ba83..6677b7e57 100644 --- a/impl/src/test/resources/io/jsonwebtoken/impl/security/README.md +++ b/impl/src/test/resources/io/jsonwebtoken/impl/security/README.md @@ -33,7 +33,7 @@ The Elliptic Curve `*.key.pem`, `*.crt.pem` and `*.pub.pem` files in this direct openssl x509 -pubkey -noout -in ES384.crt.pem > ES384.pub.pem openssl x509 -pubkey -noout -in ES512.crt.pem > ES512.pub.pem -The Edwards Curve `*.key.pem`, `*.crt.pem` and `*.pub.pem` files in this directory were created for testing as follows +The Edwards Curve `*.key.pem`, `*.crt.pem` and `*.pub.pem` files in this directory were created for testing as follows. Note that we don't/can't create self-signed certificates (`*.crt.pem` files) for X25519 and X448 because these algorithms cannot be used for signing (perhaps we could have signed them with another key, but it wasn't necessary for our testing):