NIST Elliptic Curve JWKs: field element byte array padding (#903)

* Ensured NIST Elliptic Curve JWKs pre-pad their X, Y and D byte arrays with zero bytes before Base64URL-encoding if necessary per length requirements defined in: - https://datatracker.ietf.org/doc/html/rfc7518#section-6.2.1.2 - https://datatracker.ietf.org/doc/html/rfc7518#section-6.2.1.3 - https://datatracker.ietf.org/doc/html/rfc7518#section-6.2.2.1 Fixes #901.
jwtk · Jan 26, 2024 · 0763191 · 0763191
1 parent 3e8f8a8
commit 0763191
Show file tree

Hide file tree

Showing 12 changed files with 209 additions and 28 deletions.
diff --git 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
@@ -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
@@ -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<K extends Key & ECKey, J extends Jwk<K>> extends AbstractFamilyJwkFactory<K, J> {
@@ -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
@@ -31,7 +31,10 @@
 
 class DefaultEcPrivateJwk extends AbstractPrivateJwk<ECPrivateKey, ECPublicKey, EcPublicJwk> implements EcPrivateJwk {
 
-    static final Parameter<BigInteger> D = Parameters.secretBigInt("d", "ECC Private Key");
+    static final Parameter<BigInteger> D = Parameters.bigInt("d", "ECC Private Key")
+            .setConverter(FieldElementConverter.B64URL_CONVERTER)
+            .setSecret(true) // important!
+            .build();
     static final Set<Parameter<?>> PARAMS = Collections.concat(DefaultEcPublicJwk.PARAMS, D);
 
     DefaultEcPrivateJwk(JwkContext<ECPrivateKey> 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
@@ -30,9 +30,12 @@
 class DefaultEcPublicJwk extends AbstractPublicJwk<ECPublicKey> implements EcPublicJwk {
 
     static final String TYPE_VALUE = "EC";
+
     static final Parameter<String> CRV = Parameters.string("crv", "Curve");
-    static final Parameter<BigInteger> X = Parameters.bigInt("x", "X Coordinate").build();
-    static final Parameter<BigInteger> Y = Parameters.bigInt("y", "Y Coordinate").build();
+    static final Parameter<BigInteger> X = Parameters.bigInt("x", "X Coordinate")
+            .setConverter(FieldElementConverter.B64URL_CONVERTER).build();
+    static final Parameter<BigInteger> Y = Parameters.bigInt("y", "Y Coordinate")
+            .setConverter(FieldElementConverter.B64URL_CONVERTER).build();
     static final Set<Parameter<?>> 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
@@ -35,7 +35,8 @@
 
 class EcPrivateJwkFactory extends AbstractEcJwkFactory<ECPrivateKey, EcPrivateJwk> {
 
-    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<ECPrivateKey> 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
@@ -81,11 +81,10 @@ protected EcPublicJwk createJwkFromKey(JwkContext<ECPublicKey> 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
@@ -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 <a href="https://github.com/jwtk/jjwt/issues/901">JJWT Issue 901</a>. 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
+ * <em>any</em> 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<BigInteger, byte[]> {
+
+    static final FieldElementConverter INSTANCE = new FieldElementConverter();
+
+    static final Converter<BigInteger, Object> 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
@@ -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<String, ?>
+            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
@@ -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
@@ -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
@@ -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()