From 7409dee953e67aa3e8ee40362e0a7f2497baf8a1 Mon Sep 17 00:00:00 2001 From: mbalao Date: Wed, 4 Sep 2019 19:08:57 +0100 Subject: [PATCH] 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 --- .../sun/security/ec/ECDSASignature.java | 6 +- src/share/native/sun/security/ec/impl/ec.c | 18 ++- .../testlibrary/jdk/testlibrary/Convert.java | 84 ++++++++++++ .../security/ec/SignatureDigestTruncate.java | 125 ++++++++++++++++++ 4 files changed, 223 insertions(+), 10 deletions(-) create mode 100644 test/lib/testlibrary/jdk/testlibrary/Convert.java create mode 100644 test/sun/security/ec/SignatureDigestTruncate.java diff --git a/src/share/classes/sun/security/ec/ECDSASignature.java b/src/share/classes/sun/security/ec/ECDSASignature.java index 95c53f4f7..03aceae37 100644 --- a/src/share/classes/sun/security/ec/ECDSASignature.java +++ b/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); diff --git a/src/share/native/sun/security/ec/impl/ec.c b/src/share/native/sun/security/ec/impl/ec.c index 4e94d2779..264e1a86e 100644 --- a/src/share/native/sun/security/ec/impl/ec.c +++ b/src/share/native/sun/security/ec/impl/ec.c @@ -1,5 +1,5 @@ /* - * 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 diff --git a/test/lib/testlibrary/jdk/testlibrary/Convert.java b/test/lib/testlibrary/jdk/testlibrary/Convert.java new file mode 100644 index 000000000..33afbf0d1 --- /dev/null +++ b/test/lib/testlibrary/jdk/testlibrary/Convert.java @@ -0,0 +1,84 @@ +/* + * 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; + } +} + diff --git a/test/sun/security/ec/SignatureDigestTruncate.java b/test/sun/security/ec/SignatureDigestTruncate.java new file mode 100644 index 000000000..a0ebbb2cf --- /dev/null +++ b/test/sun/security/ec/SignatureDigestTruncate.java @@ -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.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"); + } +} -- GitLab