[11u] RFR: 8208698: Improved ECC Implementation

[Doerr, Martin]

Hi Christoph,

looks like quite some manual resolution just because of a small conflicting change in one file.
Backport looks good, but please backport it together with JDK-8217344.
After that, ECDHKeyAgreement.java should be identical to the jdk13 version.

Best regards,
Martin


> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-bounces at openjdk.java.net> On
> Behalf Of Langer, Christoph
> Sent: Dienstag, 28. Mai 2019 09:21
> To: jdk-updates-dev at openjdk.java.net
> Cc: security-dev <security-dev at openjdk.java.net>
> Subject: [11u] RFR: 8208698: Improved ECC Implementation
> 
> 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