8286908: ECDSA signature should not return parameters

Reviewed-by: ascarpino, hchao, valeriep

src/java.base/share/classes/sun/security/util/SignatureUtil.java

@@ -151,17 +151,10 @@ public class SignatureUtil {
                     createAlgorithmParameters(sigName, paramBytes);
                 paramSpec = RSAUtil.getParamSpec(params);
             } else if (sigName.contains("ECDSA")) {
-                try {
+                // Some certificates have params in an ECDSA algorithmID.
-                    Provider p = Signature.getInstance(sigName).getProvider();
+                // According to RFC 3279 2.2.3 and RFC 5758 3.2,
-                    paramSpec = ECUtil.getECParameterSpec(p, paramBytes);
+                // they are useless and should be ignored.
-                } catch (Exception e) {
+                return null;
-                    throw new ProviderException("Error handling EC parameters", e);
-                }
-                // ECUtil discards exception and returns null, so we need to check
-                // the returned value
-                if (paramSpec == null) {
-                    throw new ProviderException("Error handling EC parameters");
-                }
             } else {
                 throw new ProviderException
                      ("Unrecognized algorithm for signature parameters " +

src/java.base/share/classes/sun/security/x509/AlgorithmId.java

@@ -213,10 +213,14 @@ public class AlgorithmId implements Serializable, DerEncoder {
                     || algid.equals(ed25519_oid)
                     || algid.equals(x448_oid)
                     || algid.equals(x25519_oid)
+                    || algid.equals(SHA1withECDSA_oid)
                     || algid.equals(SHA224withECDSA_oid)
                     || algid.equals(SHA256withECDSA_oid)
                     || algid.equals(SHA384withECDSA_oid)
                     || algid.equals(SHA512withECDSA_oid)) {
+                // RFC 3279 2.2.3: When the ecdsa-with-SHA1 algorithm identifier
+                // appears as the algorithm field in an AlgorithmIdentifier,
+                // the encoding MUST omit the parameters field.
                 // RFC 4055 3.3: when an RSASSA-PSS key does not require
                 // parameter validation, field is absent.
                 // RFC 8410 3: for id-X25519, id-X448, id-Ed25519, and
@@ -692,6 +696,8 @@ public class AlgorithmId implements Serializable, DerEncoder {
     public static final ObjectIdentifier x448_oid =
             ObjectIdentifier.of(KnownOIDs.X448);
 
+    public static final ObjectIdentifier SHA1withECDSA_oid =
+            ObjectIdentifier.of(KnownOIDs.SHA1withECDSA);
     public static final ObjectIdentifier SHA224withECDSA_oid =
             ObjectIdentifier.of(KnownOIDs.SHA224withECDSA);
     public static final ObjectIdentifier SHA256withECDSA_oid =

src/jdk.crypto.ec/share/classes/sun/security/ec/ECDSASignature.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2022, 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
@@ -80,9 +80,6 @@ abstract class ECDSASignature extends SignatureSpi {
     // public key, if initialized for verifying
     private ECPublicKey publicKey;
 
-    // signature parameters
-    private ECParameterSpec sigParams = null;
-
     // The format. true for the IEEE P1363 format. false (default) for ASN.1
     private final boolean p1363Format;
 
@@ -347,10 +344,6 @@ abstract class ECDSASignature extends SignatureSpi {
     protected void engineInitVerify(PublicKey publicKey)
     throws InvalidKeyException {
         ECPublicKey key = (ECPublicKey) ECKeyFactory.toECKey(publicKey);
-        if (!isCompatible(this.sigParams, key.getParams())) {
-            throw new InvalidKeyException("Key params does not match signature params");
-        }
-
         // Should check that the supplied key is appropriate for signature
         // algorithm (e.g. P-256 for SHA256withECDSA)
         this.publicKey = key;
@@ -370,10 +363,6 @@ abstract class ECDSASignature extends SignatureSpi {
     protected void engineInitSign(PrivateKey privateKey, SecureRandom random)
     throws InvalidKeyException {
         ECPrivateKey key = (ECPrivateKey) ECKeyFactory.toECKey(privateKey);
-        if (!isCompatible(this.sigParams, key.getParams())) {
-            throw new InvalidKeyException("Key params does not match signature params");
-        }
-
         ECUtil.checkPrivateKey(key);
         // Should check that the supplied key is appropriate for signature
         // algorithm (e.g. P-256 for SHA256withECDSA)
@@ -430,15 +419,6 @@ abstract class ECDSASignature extends SignatureSpi {
         needsReset = true;
     }
 
-    private static boolean isCompatible(ECParameterSpec sigParams,
-            ECParameterSpec keyParams) {
-        if (sigParams == null) {
-            // no restriction on key param
-            return true;
-        }
-        return ECUtil.equals(sigParams, keyParams);
-    }
-
     private byte[] signDigestImpl(ECDSAOperations ops, int seedBits,
         byte[] digest, ECPrivateKey priv, SecureRandom random)
         throws SignatureException {
@@ -528,17 +508,21 @@ abstract class ECDSASignature extends SignatureSpi {
 
     @Override
     protected void engineSetParameter(AlgorithmParameterSpec params)
-    throws InvalidAlgorithmParameterException {
+            throws InvalidAlgorithmParameterException {
-        if (params != null && !(params instanceof ECParameterSpec)) {
+        // Interop: some certificates include parameters in an ECDSA
-            throw new InvalidAlgorithmParameterException("No parameter accepted");
+        // algorithm identifier. We only accept one matching the key.
+        if (params == null) {
+            return;
+        }
+        if (!(params instanceof ECParameterSpec ecparams)) {
+            throw new InvalidAlgorithmParameterException(
+                    "Parameters must be of type ECParameterSpec");
         }
         ECKey key = (this.privateKey == null? this.publicKey : this.privateKey);
-        if ((key != null) && !isCompatible((ECParameterSpec)params, key.getParams())) {
+        if ((key != null) && !ECUtil.equals(ecparams, key.getParams())) {
             throw new InvalidAlgorithmParameterException
                 ("Signature params does not match key params");
         }
-
-        sigParams = (ECParameterSpec) params;
     }
 
     // get parameter, not supported. See JCA doc
@@ -551,16 +535,9 @@ abstract class ECDSASignature extends SignatureSpi {
 
     @Override
     protected AlgorithmParameters engineGetParameters() {
-        if (sigParams == null) {
+        // Always return null even if setParameter is called before.
-            return null;
+        // According to RFC 3279 2.2.3 and RFC 5758 3.2, no parameters is
-        }
+        // defined for ECDSA AlgorithmIdentifiers.
-        try {
+        return null;
-            AlgorithmParameters ap = AlgorithmParameters.getInstance("EC");
-            ap.init(sigParams);
-            return ap;
-        } catch (Exception e) {
-            // should never happen
-            throw new ProviderException("Error retrieving EC parameters", e);
-        }
     }
 }

test/jdk/sun/security/ec/SignatureParameters.java

@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2022, 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.
+ */
+
+/*
+ * @test
+ * @bug 8286908
+ * @summary ECDSA signature should not return parameters
+ * @library /test/lib
+ * @modules jdk.crypto.ec
+ */
+
+import jdk.test.lib.Asserts;
+
+import java.security.*;
+import java.security.interfaces.ECPrivateKey;
+import java.security.spec.ECGenParameterSpec;
+
+public class SignatureParameters {
+    public static void main(String[] args) throws Exception {
+        KeyPairGenerator kpg = KeyPairGenerator.getInstance("EC");
+        kpg.initialize(new ECGenParameterSpec("secp384r1"));
+        ECPrivateKey key = (ECPrivateKey) kpg.generateKeyPair().getPrivate();
+        Signature s = Signature.getInstance("SHA384withECDSA");
+        s.initSign(key);
+        s.setParameter(key.getParams());
+        Asserts.assertEQ(s.getParameters(), null);
+    }
+}