JDK-8261502 : ECDHKeyAgreement: Allows alternate ECPrivateKey impl and revised exception handling
  • Type: Bug
  • Component: security-libs
  • Sub-Component: javax.crypto
  • Affected Version: 15,17
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2021-02-09
  • Updated: 2024-03-26
  • Resolved: 2021-06-04
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 17
17 b16Fixed
Related Reports
Duplicate :  
Relates :  
Relates :  
Description
A DESCRIPTION OF THE PROBLEM :
After updating from OpenJDK 13.0.2 to OpenJDK 15.0.2 our code that generates an ECDH key agreement fails with error "Legacy SunEC curve disabled".

However we are not using a legacy curve, we are using secp256r1 which is supported. This message appears to be caused by this piece of code which is failing any PrivateKey that doesn't extend ECPrivateKeyImpl:
https://github.com/openjdk/jdk/blob/b0245c2b54f709857ad73d5530f76a33b1979c6a/src/jdk.crypto.ec/share/classes/sun/security/ec/ECDHKeyAgreement.java#L231-L233 - and then prints an inaccurate error message.

REGRESSION : Last worked in version 13

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Run test code supplied in report. Verify it passes with earlier version of the JDK, and fails with JDK 15.

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
No output and exit code of 0.
ACTUAL -
Stack trace and exit code of non-zero:

xception in thread "main" java.lang.IllegalStateException: java.security.InvalidAlgorithmParameterException: Legacy SunEC curve disabled, one or both keys:  Private: secp256r1 [NIST P-256,X9.62 prime256v1] (1.2.840.10045.3.1.7), PublicKey:secp256r1 [NIST P-256,X9.62 prime256v1] (1.2.840.10045.3.1.7)
        at jdk.crypto.ec/sun.security.ec.ECDHKeyAgreement.engineGenerateSecret(ECDHKeyAgreement.java:184)
        at java.base/javax.crypto.KeyAgreement.generateSecret(KeyAgreement.java:604)
        at JDK15Bug.generateECDHSecret(JDK15Bug.java:31)
        at JDK15Bug.main(JDK15Bug.java:20)
Caused by: java.security.InvalidAlgorithmParameterException: Legacy SunEC curve disabled, one or both keys:  Private: secp256r1 [NIST P-256,X9.62 prime256v1] (1.2.840.10045.3.1.7), PublicKey:secp256r1 [NIST P-256,X9.62 prime256v1] (1.2.840.10045.3.1.7)
        ... 4 more

---------- BEGIN SOURCE ----------
public class JDK15Bug {

    public static void main(String []args) throws Exception {
        // make secp256r1 key pair generator
        java.security.KeyPairGenerator g = java.security.KeyPairGenerator.getInstance("EC");
        java.security.spec.ECGenParameterSpec spec = new java.security.spec.ECGenParameterSpec("secp256r1");
        g.initialize(spec);

        // generate a private and a public EC key
        java.security.interfaces.ECPrivateKey privKey = (java.security.interfaces.ECPrivateKey) g.generateKeyPair().getPrivate();
        java.security.interfaces.ECPublicKey pubKey = (java.security.interfaces.ECPublicKey) g.generateKeyPair().getPublic();

        // this works fine in JDK 15
        generateECDHSecret(privKey, pubKey);
        
        // this breaks in JDK 15 with:
        // Caused by: java.security.InvalidAlgorithmParameterException: Legacy SunEC curve disabled, one or both keys:  Private: secp256r1 [NIST P-256,X9.62 prime256v1] (1.2.840.10045.3.1.7), PublicKey:secp256r1 [NIST P-256,X9.62 prime256v1] (1.2.840.10045.3.1.7)
        // even though this is not a legacy curve
        // This has worked in prior version (e.g. 11, 13)
        generateECDHSecret(new MyECPrivateKey(privKey), pubKey);
    }

    // given an ECPrivateKey and ECPublicKey, return the ECDH exchange generated secret
    private static byte[] generateECDHSecret(
        java.security.interfaces.ECPrivateKey privKey,
        java.security.interfaces.ECPublicKey pubKey
    ) throws Exception {
        javax.crypto.KeyAgreement ka = javax.crypto.KeyAgreement.getInstance("ECDH");
        ka.init(privKey);
        ka.doPhase(pubKey, true);
        return ka.generateSecret();
    }

    // this class implements ECPrivateKey, wrapping a supplied ECPrivateKey
    private static class MyECPrivateKey implements java.security.interfaces.ECPrivateKey {
        private java.security.interfaces.ECPrivateKey p;

        MyECPrivateKey(java.security.interfaces.ECPrivateKey p) {
            this.p = p;
        }

        public java.math.BigInteger getS() { return this.p.getS(); }
        public byte[] getEncoded() { return this.p.getEncoded(); }
        public String getFormat() { return this.p.getFormat(); }
        public String getAlgorithm() { return this.p.getAlgorithm(); }
        public java.security.spec.ECParameterSpec getParams() { return this.p.getParams(); }
    }
}
---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
We have rolled back to previous version (13) of the JDK while we evaluate options. Much of our code base passes around our own class which implements ECPrivateKey (and includes additional metadata such as who the key belongs to etc), so if not fixed we will need to invest in a substantial refactor to accommodate what feels like an unnecessarily new tight coupling to a private implementation class.

If the JDK code (https://github.com/openjdk/jdk/blob/b0245c2b54f709857ad73d5530f76a33b1979c6a/src/jdk.crypto.ec/share/classes/sun/security/ec/ECDHKeyAgreement.java#L231-L233) instead checked for "instanceof ECPrivateKey" and called "getS()" instead of the private implementation version "getArrayS()", then things would work again.

FREQUENCY : always



Comments
Verifications from the submitter: OpenJDK 17 EA 24 – works. OpenJDK 16.0.1 - fails. OpenJDK 15.0.2 – fails. OpenJDK 11.0.11 – works. So yes, looks like this is fixed in JDK 17.
04-06-2021

No response from the submitter.
20-04-2021

Request the submitter to verify the fix with the latest version of JDK at https://jdk.java.net/17/
07-04-2021

Changeset: 374272fd Author: Anthony Scarpino <ascarpino@openjdk.org> Date: 2021-03-25 19:18:44 +0000 URL: https://git.openjdk.java.net/jdk/commit/374272fd
25-03-2021

Thanks for the suggestion, that is much better.
23-02-2021

Additional Information from submitter: =========================== I'd have expected that the following small patch (removing all code related to "privImpl" and instead passing "priv.getS()" to IntegerFieldModuleP.getElement() which already has a method which accepts a BigInteger) should work. I'm unable to figure out how to run the full test-cycle locally, so haven't been able to verify. diff --git a/src/jdk.crypto.ec/share/classes/sun/security/ec/ECDHKeyAgreement.java b/src/jdk.crypto.ec/share/classes/sun/security/ec/ECDHKeyAgreement.java index 6c79228ef22..56359710271 100644 --- a/src/jdk.crypto.ec/share/classes/sun/security/ec/ECDHKeyAgreement.java +++ b/src/jdk.crypto.ec/share/classes/sun/security/ec/ECDHKeyAgreement.java @@ -219,11 +219,6 @@ public final class ECDHKeyAgreement extends KeyAgreementSpi { return Optional.empty(); } ECOperations ops = opsOpt.get(); - if (! (priv instanceof ECPrivateKeyImpl)) { - return Optional.empty(); - } - ECPrivateKeyImpl privImpl = (ECPrivateKeyImpl) priv; - byte[] sArr = privImpl.getArrayS(); // to match the native implementation, validate the public key here // and throw ProviderException if it is invalid @@ -231,7 +226,7 @@ public final class ECDHKeyAgreement extends KeyAgreementSpi { IntegerFieldModuloP field = ops.getField(); // convert s array into field element and multiply by the cofactor - MutableIntegerModuloP scalar = field.getElement(sArr).mutable(); + MutableIntegerModuloP scalar = field.getElement(priv.getS()).mutable(); SmallValue cofactor = field.getSmallValue(priv.getParams().getCofactor()); scalar.setProduct(cofactor); The proposed patch in the bug has an unnecessary check for "priv instanceof ECPrivateKey", which is unnecessary since priv is declared of that type.
23-02-2021

NOTE: This can be backported directly to jdk16. jdk15 has the native library disabled, not removed, so the exception handling would need to remain. It would looks something like this: protected byte[] engineGenerateSecret() throws IllegalStateException { if ((privateKey == null) || (publicKey == null)) { throw new IllegalStateException("Not initialized correctly"); } - Optional<byte[]> resultOpt = deriveKeyImpl(privateKey, publicKey); + Optional<byte[]> resultOpt; + try { + resultOpt = deriveKeyImpl(privateKey, publicKey); + } catch (Exception e) { + throw new ProviderException(e); + } ... and ... Optional<byte[]> deriveKeyImpl(ECPrivateKey priv, ECPublicKey pubKey) + throws InvalidKeyException { ECParameterSpec ecSpec = priv.getParams(); EllipticCurve curve = ecSpec.getCurve(); Optional<ECOperations> opsOpt = ECOperations.forParameters(ecSpec); if (opsOpt.isEmpty()) { return Optional.empty(); } ECOperations ops = opsOpt.get(); - if (! (priv instanceof ECPrivateKeyImpl)) { + ECPrivateKeyImpl privImpl; + if (priv instanceof ECPrivateKeyImpl) { + privImpl = (ECPrivateKeyImpl) priv; + } else if (priv instanceof ECPrivateKey) { + privImpl = new ECPrivateKeyImpl(priv.getEncoded()); + } else { return Optional.empty(); } - ECPrivateKeyImpl privImpl = (ECPrivateKeyImpl) priv;
20-02-2021

Merged with JDK-8257932, see details for that part of the change in that bug id.
20-02-2021

If -Djdk.sunec.disableNative=false is used, the tests passes. Clearly a code path using the native sunec library allowed ECPrivateKey to work before. The SunEC code that is throwing the exception has been there long before 15. A fix is pretty simple.
11-02-2021

The observations on Windows 10: JDK 8: Passed. JDK 11: Passed. JDK 15 ea+16: Passed. JDK 15 ea+17: Failed, threw InvalidAlgorithmParameterException: Legacy SunEC curve disabled JDK 17 ea+7: Failed
10-02-2021