diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f1b9f7d7..d637784f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ This patch release: `java.io.Reader` instance. [Issue 882](https://github.com/jwtk/jjwt/issues/882). * Ensures a single string `aud` (Audience) claim is retained (without converting it to a `Set`) when copying/applying a source Claims instance to a destination Claims builder. [Issue 890](https://github.com/jwtk/jjwt/issues/890). +* Ensures P-256, P-384 and P-521 Elliptic Curve JWKs zero-pad their field element (`x`, `y`, and `d`) byte array values + if necessary before Base64Url-encoding per [RFC 7518](https://datatracker.ietf.org/doc/html/rfc7518), Sections + [6.2.1.2](https://datatracker.ietf.org/doc/html/rfc7518#section-6.2.1.2), + [6.2.1.3](https://datatracker.ietf.org/doc/html/rfc7518#section-6.2.1.3), and + [6.2.2.1](https://datatracker.ietf.org/doc/html/rfc7518#section-6.2.2.1), respectively. + [Issue 901](https://github.com/jwtk/jjwt/issues/901). ### 0.12.3 diff --git a/impl/src/main/java/io/jsonwebtoken/impl/lang/Bytes.java b/impl/src/main/java/io/jsonwebtoken/impl/lang/Bytes.java index 3cb49c6c7..7f1c84ee0 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/lang/Bytes.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/lang/Bytes.java @@ -240,4 +240,25 @@ public static void increment(byte[] a) { } } } + + /** + * Pads the front of the specified byte array with zeros if necessary, returning a new padded result, or the + * original array unmodified if padding isn't necessary. Padding is only performed if {@code length} is greater + * than {@code bytes.length}. + * + * @param bytes the byte array to pre-pad with zeros if necessary + * @param length the length of the required output array + * @return the potentially pre-padded byte array, or the existing {@code bytes} array if padding wasn't necessary. + * @since 0.12.4 + */ + public static byte[] prepad(byte[] bytes, int length) { + Assert.notNull(bytes, "byte array cannot be null."); + Assert.gt(length, 0, "length must be positive (> 0)."); + if (bytes.length < length) { // need to pad with leading zero(es): + byte[] padded = new byte[length]; + System.arraycopy(bytes, 0, padded, length - bytes.length, bytes.length); + bytes = padded; + } + return bytes; + } } diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractEcJwkFactory.java b/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractEcJwkFactory.java index dc2c9ca03..46b844dd4 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractEcJwkFactory.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractEcJwkFactory.java @@ -15,6 +15,7 @@ */ package io.jsonwebtoken.impl.security; +import io.jsonwebtoken.impl.lang.Bytes; import io.jsonwebtoken.impl.lang.Converters; import io.jsonwebtoken.impl.lang.Parameter; import io.jsonwebtoken.io.Encoders; @@ -24,6 +25,7 @@ import java.math.BigInteger; import java.security.Key; import java.security.interfaces.ECKey; +import java.security.spec.EllipticCurve; import java.util.Set; abstract class AbstractEcJwkFactory> extends AbstractFamilyJwkFactory { @@ -41,19 +43,16 @@ protected static ECCurve getCurveByJwaId(String jwaCurveId) { * https://tools.ietf.org/html/rfc7518#section-6.2.1.2 indicates that this algorithm logic is defined in * http://www.secg.org/sec1-v2.pdf Section 2.3.5. * - * @param fieldSize EC field size - * @param coordinate EC point coordinate (e.g. x or y) + * @param curve EllipticCurve + * @param coordinate EC point coordinate (e.g. x or y) on the {@code curve} * @return A base64Url-encoded String representing the EC field element per the RFC format */ // Algorithm defined in http://www.secg.org/sec1-v2.pdf Section 2.3.5 - static String toOctetString(int fieldSize, BigInteger coordinate) { + static String toOctetString(EllipticCurve curve, BigInteger coordinate) { byte[] bytes = Converters.BIGINT_UBYTES.applyTo(coordinate); - int mlen = (int) Math.ceil(fieldSize / 8d); - if (mlen > bytes.length) { - byte[] m = new byte[mlen]; - System.arraycopy(bytes, 0, m, mlen - bytes.length, bytes.length); - bytes = m; - } + int fieldSizeInBits = curve.getField().getFieldSize(); + int mlen = Bytes.length(fieldSizeInBits); + bytes = Bytes.prepad(bytes, mlen); return Encoders.BASE64URL.encode(bytes); } diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultEcPrivateJwk.java b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultEcPrivateJwk.java index c8f6c6e77..61f56c137 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultEcPrivateJwk.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultEcPrivateJwk.java @@ -31,7 +31,10 @@ class DefaultEcPrivateJwk extends AbstractPrivateJwk implements EcPrivateJwk { - static final Parameter D = Parameters.secretBigInt("d", "ECC Private Key"); + static final Parameter D = Parameters.bigInt("d", "ECC Private Key") + .setConverter(FieldElementConverter.B64URL_CONVERTER) + .setSecret(true) // important! + .build(); static final Set> PARAMS = Collections.concat(DefaultEcPublicJwk.PARAMS, D); DefaultEcPrivateJwk(JwkContext ctx, EcPublicJwk pubJwk) { diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultEcPublicJwk.java b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultEcPublicJwk.java index b60864f39..add89f69c 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultEcPublicJwk.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultEcPublicJwk.java @@ -30,9 +30,12 @@ class DefaultEcPublicJwk extends AbstractPublicJwk implements EcPublicJwk { static final String TYPE_VALUE = "EC"; + static final Parameter CRV = Parameters.string("crv", "Curve"); - static final Parameter X = Parameters.bigInt("x", "X Coordinate").build(); - static final Parameter Y = Parameters.bigInt("y", "Y Coordinate").build(); + static final Parameter X = Parameters.bigInt("x", "X Coordinate") + .setConverter(FieldElementConverter.B64URL_CONVERTER).build(); + static final Parameter Y = Parameters.bigInt("y", "Y Coordinate") + .setConverter(FieldElementConverter.B64URL_CONVERTER).build(); static final Set> PARAMS = Collections.concat(AbstractAsymmetricJwk.PARAMS, CRV, X, Y); // https://www.rfc-editor.org/rfc/rfc7638#section-3.2 @@ -52,4 +55,5 @@ static boolean equalsPublic(ParameterReadable self, Object candidate) { protected boolean equals(PublicJwk jwk) { return jwk instanceof EcPublicJwk && equalsPublic(this, jwk); } + } diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/EcPrivateJwkFactory.java b/impl/src/main/java/io/jsonwebtoken/impl/security/EcPrivateJwkFactory.java index 941cdc20b..b37c22429 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/security/EcPrivateJwkFactory.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/EcPrivateJwkFactory.java @@ -35,7 +35,8 @@ class EcPrivateJwkFactory extends AbstractEcJwkFactory { - private static final String ECPUBKEY_ERR_MSG = "JwkContext publicKey must be an " + ECPublicKey.class.getName() + " instance."; + private static final String ECPUBKEY_ERR_MSG = "JwkContext publicKey must be an " + ECPublicKey.class.getName() + + " instance."; private static final EcPublicJwkFactory PUB_FACTORY = EcPublicJwkFactory.INSTANCE; @@ -96,8 +97,7 @@ protected EcPrivateJwk createJwkFromKey(JwkContext ctx) { ctx.setId(pubJwk.getId()); } - int fieldSize = key.getParams().getCurve().getField().getFieldSize(); - String d = toOctetString(fieldSize, key.getS()); + String d = toOctetString(key.getParams().getCurve(), key.getS()); ctx.put(DefaultEcPrivateJwk.D.getId(), d); return new DefaultEcPrivateJwk(ctx, pubJwk); diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/EcPublicJwkFactory.java b/impl/src/main/java/io/jsonwebtoken/impl/security/EcPublicJwkFactory.java index be86446b6..b0421c7dc 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/security/EcPublicJwkFactory.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/EcPublicJwkFactory.java @@ -81,11 +81,10 @@ protected EcPublicJwk createJwkFromKey(JwkContext ctx) { ctx.put(DefaultEcPublicJwk.CRV.getId(), curveId); - int fieldSize = curve.getField().getFieldSize(); - String x = toOctetString(fieldSize, point.getAffineX()); + String x = toOctetString(curve, point.getAffineX()); ctx.put(DefaultEcPublicJwk.X.getId(), x); - String y = toOctetString(fieldSize, point.getAffineY()); + String y = toOctetString(curve, point.getAffineY()); ctx.put(DefaultEcPublicJwk.Y.getId(), y); return new DefaultEcPublicJwk(ctx); diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/FieldElementConverter.java b/impl/src/main/java/io/jsonwebtoken/impl/security/FieldElementConverter.java new file mode 100644 index 000000000..7c07ca438 --- /dev/null +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/FieldElementConverter.java @@ -0,0 +1,69 @@ +/* + * Copyright © 2024 jsonwebtoken.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.jsonwebtoken.impl.security; + +import io.jsonwebtoken.impl.io.Codec; +import io.jsonwebtoken.impl.lang.Bytes; +import io.jsonwebtoken.impl.lang.Converter; +import io.jsonwebtoken.impl.lang.Converters; + +import java.math.BigInteger; + +/** + * Hotfix for JJWT Issue 901. This is currently hard-coded + * expecting field elements for NIST P-256, P-384, or P-521 curves. Ideally this should be refactored to work for + * any curve based on its field size, not just for these NIST curves. However, the + * {@link EcPublicJwkFactory} and {@link EcPrivateJwkFactory} implementations only work with JWA NIST curves, + * so this implementation is acceptable until (and if) different Weierstrass elliptic curves (ever) need to be + * supported. + * + * @since 0.12.4 + */ +final class FieldElementConverter implements Converter { + + static final FieldElementConverter INSTANCE = new FieldElementConverter(); + + static final Converter B64URL_CONVERTER = Converters.forEncoded(BigInteger.class, + Converters.compound(INSTANCE, Codec.BASE64URL)); + + private static int bytelen(ECCurve curve) { + return Bytes.length(curve.toParameterSpec().getCurve().getField().getFieldSize()); + } + + private static final int P256_BYTE_LEN = bytelen(ECCurve.P256); + private static final int P384_BYTE_LEN = bytelen(ECCurve.P384); + private static final int P521_BYTE_LEN = bytelen(ECCurve.P521); + + @Override + public byte[] applyTo(BigInteger bigInteger) { + byte[] bytes = Converters.BIGINT_UBYTES.applyTo(bigInteger); + int len = bytes.length; + if (len == P256_BYTE_LEN || len == P384_BYTE_LEN || len == P521_BYTE_LEN) return bytes; + if (len < P256_BYTE_LEN) { + bytes = Bytes.prepad(bytes, P256_BYTE_LEN); + } else if (len < P384_BYTE_LEN) { + bytes = Bytes.prepad(bytes, P384_BYTE_LEN); + } else { // > P-384, so must be P-521: + bytes = Bytes.prepad(bytes, P521_BYTE_LEN); + } + return bytes; + } + + @Override + public BigInteger applyFrom(byte[] bytes) { + return Converters.BIGINT_UBYTES.applyFrom(bytes); + } +} diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractEcJwkFactoryTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractEcJwkFactoryTest.groovy index 78fb0a18b..42fb889ee 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractEcJwkFactoryTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractEcJwkFactoryTest.groovy @@ -15,10 +15,16 @@ */ package io.jsonwebtoken.impl.security - +import io.jsonwebtoken.impl.lang.Bytes +import io.jsonwebtoken.impl.lang.Services +import io.jsonwebtoken.io.Decoders +import io.jsonwebtoken.io.Deserializer +import io.jsonwebtoken.security.Jwks import io.jsonwebtoken.security.UnsupportedKeyException import org.junit.Test +import java.security.interfaces.ECPrivateKey + import static org.junit.Assert.assertEquals import static org.junit.Assert.fail @@ -35,4 +41,36 @@ class AbstractEcJwkFactoryTest { assertEquals msg, e.getMessage() } } + + /** + * Asserts correct behavior per https://github.com/jwtk/jjwt/issues/901 + */ + @Test + void fieldElementByteArrayLength() { + + EcSignatureAlgorithmTest.algs().each { alg -> + + def key = alg.keyPair().build().getPrivate() as ECPrivateKey + def jwk = Jwks.builder().key(key).build() + + def json = Jwks.UNSAFE_JSON(jwk) + def map = Services.get(Deserializer).deserialize(new StringReader(json)) as Map + def xs = map.get("x") as String + def ys = map.get("y") as String + def ds = map.get("d") as String + + def x = Decoders.BASE64URL.decode(xs) + def y = Decoders.BASE64URL.decode(ys) + def d = Decoders.BASE64URL.decode(ds) + + // most important part of the test: the decoded byte arrays must have a length equal to the curve + // field size (in bytes): + int fieldSizeInBits = key.getParams().getCurve().getField().getFieldSize() + int fieldSizeInBytes = Bytes.length(fieldSizeInBits) + + assertEquals fieldSizeInBytes, x.length + assertEquals fieldSizeInBytes, y.length + assertEquals fieldSizeInBytes, d.length + } + } } diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/DispatchingJwkFactoryTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/DispatchingJwkFactoryTest.groovy index 612b75fae..126ce1863 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/DispatchingJwkFactoryTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/DispatchingJwkFactoryTest.groovy @@ -109,9 +109,9 @@ class DispatchingJwkFactoryTest { assertTrue jwk instanceof EcPrivateJwk def key = jwk.toKey() assertTrue key instanceof ECPrivateKey - String x = AbstractEcJwkFactory.toOctetString(key.params.curve.field.fieldSize, jwk.toPublicJwk().toKey().w.affineX) - String y = AbstractEcJwkFactory.toOctetString(key.params.curve.field.fieldSize, jwk.toPublicJwk().toKey().w.affineY) - String d = AbstractEcJwkFactory.toOctetString(key.params.curve.field.fieldSize, key.s) + String x = AbstractEcJwkFactory.toOctetString(key.params.curve, jwk.toPublicJwk().toKey().w.affineX) + String y = AbstractEcJwkFactory.toOctetString(key.params.curve, jwk.toPublicJwk().toKey().w.affineY) + String d = AbstractEcJwkFactory.toOctetString(key.params.curve, key.s) assertEquals jwk.d.get(), d //remove the 'd' mapping to represent only a public key: diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/FieldElementConverterTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/FieldElementConverterTest.groovy new file mode 100644 index 000000000..e8a1ea16c --- /dev/null +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/FieldElementConverterTest.groovy @@ -0,0 +1,41 @@ +/* + * Copyright © 2024 jsonwebtoken.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.jsonwebtoken.impl.security + +import io.jsonwebtoken.impl.lang.Bytes +import org.junit.Test + +import static org.junit.Assert.assertEquals + +/** + * @since 0.12.4 + */ +class FieldElementConverterTest { + + static FieldElementConverter converter = FieldElementConverter.INSTANCE + + @Test + void p384CoordinateNeedsPadding() { + def requiredByteLen = 48 + def coordBytes = Bytes.random(requiredByteLen - 1) // one less to see if padding is applied + def coord = new BigInteger(1, coordBytes) + byte[] result = converter.applyTo(coord) + assertEquals requiredByteLen, result.length + assertEquals 0x00 as byte, result[0] + //ensure roundtrip works: + assertEquals coord, converter.applyFrom(result) + } +} diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/RFC7517AppendixA2Test.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/RFC7517AppendixA2Test.groovy index b3321944b..d7e9fa0c4 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/RFC7517AppendixA2Test.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/RFC7517AppendixA2Test.groovy @@ -23,6 +23,7 @@ import org.junit.Test import java.security.interfaces.ECPrivateKey import java.security.interfaces.RSAPrivateCrtKey +import java.security.spec.EllipticCurve import static org.junit.Assert.* @@ -31,8 +32,8 @@ import static org.junit.Assert.* */ class RFC7517AppendixA2Test { - private static final String ecEncode(int fieldSize, BigInteger coord) { - return AbstractEcJwkFactory.toOctetString(fieldSize, coord) + private static final String ecEncode(EllipticCurve curve, BigInteger coord) { + return AbstractEcJwkFactory.toOctetString(curve, coord) } private static final String rsaEncode(BigInteger i) { @@ -90,17 +91,17 @@ class RFC7517AppendixA2Test { def m = keys[0] def jwk = Jwks.builder().add(m).build() as EcPrivateJwk def key = jwk.toKey() - int fieldSize = key.params.curve.field.fieldSize + def curve = key.params.curve assertTrue key instanceof ECPrivateKey assertEquals m.size(), jwk.size() assertEquals m.kty, jwk.getType() assertEquals m.crv, jwk.get('crv') assertEquals m.x, jwk.get('x') - assertEquals m.x, ecEncode(fieldSize, jwk.toPublicJwk().toKey().w.affineX) + assertEquals m.x, ecEncode(curve, jwk.toPublicJwk().toKey().w.affineX) assertEquals m.y, jwk.get('y') - assertEquals m.y, ecEncode(fieldSize, jwk.toPublicJwk().toKey().w.affineY) + assertEquals m.y, ecEncode(curve, jwk.toPublicJwk().toKey().w.affineY) assertEquals m.d, jwk.get('d').get() - assertEquals m.d, ecEncode(fieldSize, key.s) + assertEquals m.d, ecEncode(curve, key.s) assertEquals m.use, jwk.getPublicKeyUse() assertEquals m.kid, jwk.getId()