[11u] RFR: 8208698: Improved ECC Implementation

[Langer, Christoph]

Hi,

please review this backport of JDK-8208698: Improved ECC Implementation.

Bug: https://bugs.openjdk.java.net/browse/JDK-8208698
Original Change: http://hg.openjdk.java.net/jdk/jdk/rev/752e57845ad2
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8208698.11u/

The patch did not apply cleanly because there were conflicts in src/jdk.crypto.ec/share/classes/sun/security/ec/ECDHKeyAgreement.java due to JDK-8205476 which is part of jdk/jdk but not of jdk11u-dev. Unfortunately, JDK-8205476 can not be downported as prerequisite because it brings a behavioral change and is associated with a CSR. So I resolved the rejects manually. I add the rejects below.

Thanks
Christoph


--- ECDHKeyAgreement.java
+++ ECDHKeyAgreement.java
@@ -99,42 +104,74 @@
                 ("Key must be a PublicKey with algorithm EC");
         }
-        ECPublicKey ecKey = (ECPublicKey)key;
-        ECParameterSpec params = ecKey.getParams();
+        this.publicKey = (ECPublicKey) key;
-        if (ecKey instanceof ECPublicKeyImpl) {
-            publicValue = ((ECPublicKeyImpl)ecKey).getEncodedPublicValue();
-        } else { // instanceof ECPublicKey
-            publicValue =
-                ECUtil.encodePoint(ecKey.getW(), params.getCurve());
-        }
+        ECParameterSpec params = publicKey.getParams();
         int keyLenBits = params.getCurve().getField().getFieldSize();
         secretLen = (keyLenBits + 7) >> 3;
         return null;
     }
+    private static void validateCoordinate(BigInteger c, BigInteger mod) {
+        if (c.compareTo(BigInteger.ZERO) < 0) {
+            throw new ProviderException("invalid coordinate");
+        }
+
+        if (c.compareTo(mod) >= 0) {
+            throw new ProviderException("invalid coordinate");
+        }
+    }
+
+    /*
+     * Check whether a public key is valid. Throw ProviderException
+     * if it is not valid or could not be validated.
+     */
+    private static void validate(ECOperations ops, ECPublicKey key) {
+
+        // ensure that integers are in proper range
+        BigInteger x = key.getW().getAffineX();
+        BigInteger y = key.getW().getAffineY();
+
+        BigInteger p = ops.getField().getSize();
+        validateCoordinate(x, p);
+        validateCoordinate(y, p);
+
+        // ensure the point is on the curve
+        EllipticCurve curve = key.getParams().getCurve();
+        BigInteger rhs = x.modPow(BigInteger.valueOf(3), p).add(curve.getA()
+            .multiply(x)).add(curve.getB()).mod(p);
+        BigInteger lhs = y.modPow(BigInteger.valueOf(2), p).mod(p);
+        if (!rhs.equals(lhs)) {
+            throw new ProviderException("point is not on curve");
+        }
+
+        // check the order of the point
+        ImmutableIntegerModuloP xElem = ops.getField().getElement(x);
+        ImmutableIntegerModuloP yElem = ops.getField().getElement(y);
+        AffinePoint affP = new AffinePoint(xElem, yElem);
+        byte[] order = key.getParams().getOrder().toByteArray();
+        ArrayUtil.reverse(order);
+        Point product = ops.multiply(affP, order);
+        if (!ops.isNeutral(product)) {
+            throw new ProviderException("point has incorrect order");
+        }
+
+    }
+
     // see JCE spec
     @Override
     protected byte[] engineGenerateSecret() throws IllegalStateException {
-        if ((privateKey == null) || (publicValue == null)) {
+        if ((privateKey == null) || (publicKey == null)) {
             throw new IllegalStateException("Not initialized correctly");
         }
-        byte[] s = privateKey.getS().toByteArray();
-        byte[] encodedParams =                   // DER OID
-            ECUtil.encodeECParameterSpec(null, privateKey.getParams());
-
-        try {
-
-            byte[] result = deriveKey(s, publicValue, encodedParams);
-            publicValue = null;
-            return result;
-
-        } catch (GeneralSecurityException e) {
-            throw new ProviderException("Could not derive key", e);
-        }
-
+        Optional<byte[]> resultOpt = deriveKeyImpl(privateKey, publicKey);
+        byte[] result = resultOpt.orElseGet(
+            () -> deriveKeyNative(privateKey, publicKey)
+        );
+        publicKey = null;
+        return result;
     }
     // see JCE spec
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20190528/a0980809/attachment.htm>