[11u] RFR: 8208698: Improved ECC Implementation

[Langer, Christoph]

Dang, you're right!

I'll open an issue and put it out for review. I guess you'll want to push it in your closed repository?

BTW: I just reported  another regression which affects 11.0.4 as well: https://bugs.openjdk.java.net/browse/JDK-8226876 It's "just" a Java level assert but maybe we'll need to take some (backout?) action here for 11.0.4, too...

Thanks
Christoph



> -----Original Message-----
> From: Andrew John Hughes <gnu.andrew at redhat.com>
> Sent: Donnerstag, 27. Juni 2019 06:09
> To: Langer, Christoph <christoph.langer at sap.com>; jdk-updates-
> dev at openjdk.java.net
> Cc: security-dev <security-dev at openjdk.java.net>
> Subject: Re: [11u] RFR: 8208698: Improved ECC Implementation
> 
> On 28/05/2019 08:21, Langer, Christoph wrote:
> > 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
> >
> >
> 
> I'm just looking over this in backporting to 8u. You mention not being
> able to include the JDK-8205476 change, but then it is included in the
> patch.
> 
> JDK-8205476 is essentially:
> 
> @@ -127,7 +127,9 @@
> 
>          try {
> 
> -            return deriveKey(s, publicValue, encodedParams);
> +            byte[] result = deriveKey(s, publicValue, encodedParams);
> +            publicValue = null;
> +            return result;
> 
>          } catch (GeneralSecurityException e) {
>              throw new ProviderException("Could not derive key", e);
> 
> The same change is made in 11u by adopting the line in the 8208698 patch
> verbatim:
> 
> -        try {
> -
> -            return deriveKey(s, publicValue, encodedParams);
> -
> -        } 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;
> 
> I think this should actually be:
> 
> return resultOpt.orElseGet(() -> deriveKeyNative(privateKey, publicKey));
> 
> if we want to avoid the JDK-8205476 change of nulling publicKey.
> 
> Thanks,
> --
> Andrew :)
> 
> Senior Free Java Software Engineer
> Red Hat, Inc. (http://www.redhat.com)
> 
> PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
> Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
> https://keybase.io/gnu_andrew