8147502: Digest is incorrectly truncated for ECDSA signatures when the bit...

8147502: Digest is incorrectly truncated for ECDSA signatures when the bit length of n is less than the field size
Summary: Truncate the digest according to the group order, not the field size
Reviewed-by: shade, andrew

src/share/classes/sun/security/ec/ECDSASignature.java

@@ -329,10 +329,10 @@ abstract class ECDSASignature extends SignatureSpi {
 
         // DER OID
         byte[] encodedParams = ECUtil.encodeECParameterSpec(null, params);
-        int keySize = params.getCurve().getField().getFieldSize();
+        int orderLength = params.getOrder().bitLength();
 
-        // seed is twice the key size (in bytes) plus 1
-        byte[] seed = new byte[(((keySize + 7) >> 3) + 1) * 2];
+        // seed is twice the order length (in bytes) plus 1
+        byte[] seed = new byte[(((orderLength + 7) >> 3) + 1) * 2];
 
         random.nextBytes(seed);
 

src/share/native/sun/security/ec/impl/ec.c

 /*
- * Copyright (c) 2007, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2007, 2019, Oracle and/or its affiliates. All rights reserved.
  * Use is subject to license terms.
  *
  * This library is free software; you can redistribute it and/or
@@ -659,6 +659,7 @@ ECDSA_SignDigestWithSeed(ECPrivateKey *key, SECItem *signature,
     SECItem kGpoint = { siBuffer, NULL, 0};
     int flen = 0;    /* length in bytes of the field size */
     unsigned olen;   /* length in bytes of the base point order */
+    unsigned int orderBitSize;
 
 #if EC_DEBUG
     char mpstr[256];
@@ -761,10 +762,11 @@ ECDSA_SignDigestWithSeed(ECPrivateKey *key, SECItem *signature,
     SECITEM_TO_MPINT(*digest, &s);        /* s = HASH(M)     */
 
     /* In the definition of EC signing, digests are truncated
-     * to the length of n in bits.
+     * to the order length
      * (see SEC 1 "Elliptic Curve Digit Signature Algorithm" section 4.1.*/
-    if (digest->len*8 > (unsigned int)ecParams->fieldID.size) {
-        mpl_rsh(&s,&s,digest->len*8 - ecParams->fieldID.size);
+    orderBitSize = mpl_significant_bits(&n);
+    if (digest->len*8 > orderBitSize) {
+        mpl_rsh(&s,&s,digest->len*8 - orderBitSize);
     }
 
 #if EC_DEBUG
@@ -897,6 +899,7 @@ ECDSA_VerifyDigest(ECPublicKey *key, const SECItem *signature,
     int slen;       /* length in bytes of a half signature (r or s) */
     int flen;       /* length in bytes of the field size */
     unsigned olen;  /* length in bytes of the base point order */
+    unsigned int orderBitSize;
 
 #if EC_DEBUG
     char mpstr[256];
@@ -976,11 +979,12 @@ ECDSA_VerifyDigest(ECPublicKey *key, const SECItem *signature,
     SECITEM_TO_MPINT(*digest, &u1);                  /* u1 = HASH(M)     */
 
     /* In the definition of EC signing, digests are truncated
-     * to the length of n in bits.
+     * to the order length, in bits.
      * (see SEC 1 "Elliptic Curve Digit Signature Algorithm" section 4.1.*/
     /* u1 = HASH(M')     */
-    if (digest->len*8 > (unsigned int)ecParams->fieldID.size) {
-        mpl_rsh(&u1,&u1,digest->len*8- ecParams->fieldID.size);
+    orderBitSize = mpl_significant_bits(&n);
+    if (digest->len*8 > orderBitSize) {
+        mpl_rsh(&u1,&u1,digest->len*8- orderBitSize);
     }
 
 #if EC_DEBUG

test/lib/testlibrary/jdk/testlibrary/Convert.java

+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package jdk.testlibrary;
+
+import java.math.BigInteger;
+
+/**
+ * Utility class containing conversions between strings, arrays, and numeric
+ * values.
+ */
+
+public class Convert {
+
+    // Convert from a byte array to a hexadecimal representation as a string.
+    public static String byteArrayToHexString(byte[] arr) {
+        StringBuilder result = new StringBuilder();
+        for (int i = 0; i < arr.length; ++i) {
+            byte curVal = arr[i];
+            result.append(Character.forDigit(curVal >> 4 & 0xF, 16));
+            result.append(Character.forDigit(curVal & 0xF, 16));
+        }
+        return result.toString();
+    }
+
+    // Expand a single byte to a byte array
+    public static byte[] byteToByteArray(byte v, int length) {
+        byte[] result = new byte[length];
+        result[0] = v;
+        return result;
+    }
+
+    // Convert a hexadecimal string to a byte array
+    public static byte[] hexStringToByteArray(String str) {
+        byte[] result = new byte[str.length() / 2];
+        for (int i = 0; i < result.length; i++) {
+            result[i] = (byte) Character.digit(str.charAt(2 * i), 16);
+            result[i] <<= 4;
+            result[i] += Character.digit(str.charAt(2 * i + 1), 16);
+        }
+        return result;
+    }
+
+    /*
+     * Convert a hexadecimal string to the corresponding little-ending number
+     * as a BigInteger. The clearHighBit argument determines whether the most
+     * significant bit of the highest byte should be set to 0 in the result.
+     */
+    public static
+    BigInteger hexStringToBigInteger(boolean clearHighBit, String str) {
+        BigInteger result = BigInteger.ZERO;
+        for (int i = 0; i < str.length() / 2; i++) {
+            int curVal = Character.digit(str.charAt(2 * i), 16);
+            curVal <<= 4;
+            curVal += Character.digit(str.charAt(2 * i + 1), 16);
+            if (clearHighBit && i == str.length() / 2 - 1) {
+                curVal &= 0x7F;
+            }
+            result = result.add(BigInteger.valueOf(curVal).shiftLeft(8 * i));
+        }
+        return result;
+    }
+}
+

test/sun/security/ec/SignatureDigestTruncate.java

+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import jdk.testlibrary.Convert;
+
+import java.security.*;
+import java.security.spec.*;
+import java.math.*;
+import java.util.*;
+
+/*
+ * @test
+ * @bug 8147502
+ * @summary Test that digests are properly truncated before the signature
+ *     is applied. The digest should be truncated to the bit length of the
+ *     group order.
+ * @library /lib/testlibrary
+ * @build jdk.testlibrary.Convert
+ * @run main SignatureDigestTruncate
+ */
+public class SignatureDigestTruncate {
+
+    /*
+     * A SecureRandom that produces nextBytes in a way that causes the nonce
+     * to be set to the value supplied to the constructor. This class
+     * is specific to the way that the native ECDSA implementation in
+     * SunEC produces nonces from random input. It may not work for all
+     * test cases, and it will need to be updated when the behavior of
+     * SunEC changes.
+     */
+    private static class FixedRandom extends SecureRandom {
+
+        private final byte[] val;
+
+        public FixedRandom(byte[] val) {
+            // SunEC adds one to the value returned, so subtract one here in
+            // order to get back to the correct value.
+            BigInteger biVal = new BigInteger(1, val);
+            biVal = biVal.subtract(BigInteger.ONE);
+            byte[] temp = biVal.toByteArray();
+            this.val = new byte[val.length];
+            int inStartPos = Math.max(0, temp.length - val.length);
+            int outStartPos = Math.max(0, val.length - temp.length);
+            System.arraycopy(temp, inStartPos, this.val, outStartPos,
+                temp.length - inStartPos);
+        }
+
+        @Override
+        public void nextBytes(byte[] bytes) {
+            // SunEC samples (n + 1) * 2 bytes, but only n*2 bytes are used by
+            // the native implementation. So the value must be offset slightly.
+            Arrays.fill(bytes, (byte) 0);
+            int copyLength = Math.min(val.length, bytes.length - 2);
+            System.arraycopy(val, 0, bytes, bytes.length - copyLength - 2,
+                copyLength);
+        }
+    }
+
+    private static void assertEquals(byte[] expected, byte[] actual,
+            String name) {
+        if (!Arrays.equals(actual, expected)) {
+            System.out.println("expect: "
+                    + Convert.byteArrayToHexString(expected));
+            System.out.println("actual: "
+                    + Convert.byteArrayToHexString(actual));
+            throw new RuntimeException("Incorrect " + name + " value");
+        }
+    }
+
+    private static void runTest(String alg, String curveName,
+        String privateKeyStr, String msgStr, String kStr, String sigStr)
+        throws Exception {
+
+        byte[] privateKey = Convert.hexStringToByteArray(privateKeyStr);
+        byte[] msg = Convert.hexStringToByteArray(msgStr);
+        byte[] k = Convert.hexStringToByteArray(kStr);
+        byte[] expectedSig = Convert.hexStringToByteArray(sigStr);
+
+        AlgorithmParameters params = AlgorithmParameters.getInstance("EC");
+        params.init(new ECGenParameterSpec(curveName));
+        ECParameterSpec ecParams =
+            params.getParameterSpec(ECParameterSpec.class);
+
+        KeyFactory kf = KeyFactory.getInstance("EC");
+        BigInteger s = new BigInteger(1, privateKey);
+        ECPrivateKeySpec privKeySpec = new ECPrivateKeySpec(s, ecParams);
+        PrivateKey privKey = kf.generatePrivate(privKeySpec);
+
+        Signature sig = Signature.getInstance(alg);
+        sig.initSign(privKey, new FixedRandom(k));
+        sig.update(msg);
+        byte[] computedSig = sig.sign();
+        assertEquals(expectedSig, computedSig, "signature");
+    }
+
+    public static void main(String[] args) throws Exception {
+        runTest("SHA384withECDSA", "sect283r1",
+            "abcdef10234567", "010203040506070809",
+            "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d" +
+            "1e1f20212223",
+            "304c022401d7544b5d3935216bd45e2f8042537e1e0296a11e0eb9666619" +
+            "9281b40942abccd5358a0224035de8a314d3e6c2a97614daebf5fb131354" +
+            "0eec3f9a3272068aa10922ccae87d255c84c");
+    }
+}