8147502: Digest is incorrectly truncated for ECDSA signatures when the bit length of n is less than the field size
authorapetcher
Tue, 19 Mar 2019 15:18:35 -0400
changeset 54197 ddfb658c8ce3
parent 54196 6146ab937899
child 54198 6ba98ff89499
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: jnimeh
src/jdk.crypto.ec/share/classes/sun/security/ec/ECDSASignature.java
src/jdk.crypto.ec/share/native/libsunec/impl/ec.c
test/jdk/sun/security/ec/SignatureDigestTruncate.java
--- a/src/jdk.crypto.ec/share/classes/sun/security/ec/ECDSASignature.java	Tue Mar 19 14:22:47 2019 -0400
+++ b/src/jdk.crypto.ec/share/classes/sun/security/ec/ECDSASignature.java	Tue Mar 19 15:18:35 2019 -0400
@@ -410,10 +410,10 @@
 
         // 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);
 
--- a/src/jdk.crypto.ec/share/native/libsunec/impl/ec.c	Tue Mar 19 14:22:47 2019 -0400
+++ b/src/jdk.crypto.ec/share/native/libsunec/impl/ec.c	Tue Mar 19 15:18:35 2019 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007, 2018, 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
@@ -660,6 +660,7 @@
     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];
@@ -762,10 +763,11 @@
     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
@@ -898,6 +900,7 @@
     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];
@@ -977,11 +980,12 @@
     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
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/sun/security/ec/SignatureDigestTruncate.java	Tue Mar 19 15:18:35 2019 -0400
@@ -0,0 +1,125 @@
+/*
+ * 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.test.lib.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 /test/lib
+ * @build jdk.test.lib.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("SHA384withECDSAinP1363Format", "sect283r1",
+            "abcdef10234567", "010203040506070809",
+            "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d" +
+            "1e1f20212223",
+            "01d7544b5d3935216bd45e2f8042537e1e0296a11e0eb96666199281b409" +
+            "42abccd5358a035de8a314d3e6c2a97614daebf5fb1313540eec3f9a3272" +
+            "068aa10922ccae87d255c84c");
+    }
+}