Update parsing/checking private key encoding and equals() (#1091)

The method that parses the private key encoding is updated
to check the encoding's validity.

An equals() method is added that calls the super.equals()
and, if that fails, directly compares the keys and params.

Several tests are, also, added to test key import, direct
and interop, through ECPrivateKeySpec.

Signed-off-by: Kostas Tsiounis <kostas.tsiounis@ibm.com>

Author: KostasTsiounis

src/main/java/com/ibm/crypto/plus/provider/ECKeyFactory.java

@@ -82,10 +82,10 @@ protected PrivateKey engineGeneratePrivate(KeySpec keySpec) throws InvalidKeySpe
             }
 
         } catch (InvalidKeyException e) {
-            throw new InvalidKeySpecException("Inappropriate key specification: " + e.getMessage());
+            throw new InvalidKeySpecException("Inappropriate key specification: ", e);
         } catch (InvalidParameterSpecException e) {
             throw new InvalidKeySpecException(
-                    "Inappropriate Parameter specification: " + e.getMessage());
+                    "Inappropriate Parameter specification: ", e);
         }
     }
 

src/main/java/com/ibm/crypto/plus/provider/ECPrivateKey.java

@@ -103,17 +103,10 @@ final class ECPrivateKey extends PKCS8Key implements java.security.interfaces.EC
         this.provider = provider;
 
         try {
-            // Set parameters.
-            AlgorithmParameters algParams = this.algid.getParameters();
-            if (algParams == null) {
-                throw new IOException(
-                        "EC domain parameters must be encoded in the algorithm identifier");
-            }
-            this.params = algParams.getParameterSpec(ECParameterSpec.class);
-
             // Get from the encoding:
             //    * the private key as a BigInteger (this.s)
             //    * the public key, if available (this.pubKeyEncoded)
+            // and set parameters.
             parsePrivateKeyEncoding();
 
             // Create appropriate encoding and create ecKey.
@@ -140,20 +133,13 @@ final class ECPrivateKey extends PKCS8Key implements java.security.interfaces.EC
             algidOut.putDerValue(new DerValue(ecKey.getParameters()));
             this.algid = AlgorithmId
                     .parse(new DerValue(DerValue.tag_Sequence, algidOut.toByteArray()));
-
-            AlgorithmParameters algParams = this.algid.getParameters();
-            if (algParams == null) {
-                throw new IOException(
-                        "EC domain parameters must be encoded in the algorithm identifier");
-            }
-            this.params = algParams.getParameterSpec(ECParameterSpec.class);
-
             // Get private key encoding from ECKey.
             this.privKeyMaterial = ecKey.getPrivateKeyBytes();
 
             // Get from the encoding:
             //    * the private key as a BigInteger (this.s)
             //    * the public key, if available (this.pubKeyEncoded)
+            // and set parameters.
             parsePrivateKeyEncoding();
         } catch (Exception exception) {
             throw new InvalidKeyException("Failed to create EC private key", exception);
@@ -181,33 +167,13 @@ private byte[] createEncodedPrivateKeyWithParams() throws IOException {
         DerValue[] inputDerValue = privKeyBytesEncodedStream.getSequence(4);
         DerOutputStream outEncodedStream = new DerOutputStream();
 
-        if (inputDerValue.length < 2) {
-            throw new IOException("Incorrect EC private key encoding");
-        }
         BigInteger tempVersion1 = inputDerValue[0].getBigInteger();
-        if (tempVersion1.compareTo(BigInteger.ONE) != 0) {
-            throw new IOException("Decoding EC private key failed. The version must be 1");
-        }
         outEncodedStream.putInteger(tempVersion1);
 
         byte[] privateKeyBytes = inputDerValue[1].getOctetString();
         outEncodedStream.putOctetString(privateKeyBytes);
 
         byte[] encodedParams = this.getAlgorithmId().getEncodedParams();
-        if (inputDerValue.length > 2) { 
-            if (inputDerValue[2].isContextSpecific(TAG_PARAMETERS_ATTRS)) {
-                DerInputStream paramDerInputStream = inputDerValue[2].getData();
-                byte[] privateKeyParams = paramDerInputStream.toByteArray();
-                // Check against the existing parameters created by PKCS8Key.
-                if (!Arrays.equals(privateKeyParams, encodedParams)) {
-                    throw new IOException("Decoding EC private key failed. The params are not the same as PKCS8Key's");
-                }
-            } else if (!inputDerValue[2].isContextSpecific(TAG_PUBLIC_KEY_ATTRS)) {
-                // Unknown third+ element: we can throw, or ignore.
-                // Keeping old behavior would be to throw; but RFC allows only [0]/[1] here.
-                throw new IOException("Decoding EC private key failed. Unexpected tagged field in ECPrivateKey");
-            }
-        }
         // The native library needs the ASN.1 DER decoding of the private key to contain the parameters (i.e., the OID).
         outEncodedStream.write(
                     DerValue.createTag(DerValue.TAG_CONTEXT, true, TAG_PARAMETERS_ATTRS),
@@ -219,25 +185,69 @@ private byte[] createEncodedPrivateKeyWithParams() throws IOException {
     }
 
     /**
-     * Parse the private key encoding to:
+     * Check that the encoding is correct and at the same time
+     * parse the private key encoding to:
      * - get the key and set it as a BigInteger (i.e., this.s)
+     * - validate the parameters, if available
      * - get the public key, if available, and save its X.509 encoding
      *
-     * @throws IOException
+     * @throws InvalidKeyException
      */
-    private void parsePrivateKeyEncoding() throws IOException {
-        DerInputStream privKeyBytesEncodedStream = new DerInputStream(this.privKeyMaterial);
-        DerValue[] inputDerValue = privKeyBytesEncodedStream.getSequence(4);
+    private void parsePrivateKeyEncoding() throws InvalidKeyException {
+        // Parse private key material from PKCS8Key.decode()
+        try {
+            DerInputStream in = new DerInputStream(this.privKeyMaterial);
+            DerValue derValue = in.getDerValue();
+            if (derValue.tag != DerValue.tag_Sequence) {
+                throw new IOException("Not a SEQUENCE");
+            }
+            DerInputStream data = derValue.data;
+            int version = data.getInteger();
+            if (version != 1) {
+                throw new IOException("Version must be 1");
+            }
+            byte[] privData = data.getOctetString();
+            this.s = new BigInteger(1, privData);
 
-        byte[] privateKeyBytes = inputDerValue[1].getOctetString();
-        this.s = new BigInteger(1, privateKeyBytes);
+            // Validate parameters stored from PKCS8Key.decode()
+            AlgorithmParameters algParams = this.algid.getParameters();
+            if (algParams == null) {
+                throw new InvalidKeyException("EC domain parameters must be "
+                    + "encoded in the algorithm identifier");
+            }
+            this.params = algParams.getParameterSpec(ECParameterSpec.class);
+
+            if (data.available() == 0) {
+                return;
+            }
+
+            DerValue value = data.getDerValue();
+            if (value.isContextSpecific(TAG_PARAMETERS_ATTRS)) {
+                byte[] privateKeyParams = value.getDataBytes();
+                byte[] encodedParams = this.getAlgorithmId().getEncodedParams();
+                // Check against the existing parameters created by PKCS8Key.
+                if (!Arrays.equals(privateKeyParams, encodedParams)) {
+                    throw new InvalidKeyException("Decoding EC private key failed. The params are not the same as PKCS8Key's");
+                }
+                if (data.available() == 0) {
+                    return;
+                }
+                value = data.getDerValue();
+            }
 
-        for (int i = 2; i < inputDerValue.length; i++) {
-            DerValue v = inputDerValue[i];
-            if (v.isContextSpecific(TAG_PUBLIC_KEY_ATTRS)) {
-                DerValue bits = v.withTag(DerValue.tag_BitString);
+            if (value.isContextSpecific(TAG_PUBLIC_KEY_ATTRS)) {
+                DerValue bits = value.withTag(DerValue.tag_BitString);
                 this.pubKeyEncoded = new X509Key(this.algid, bits.data.getUnalignedBitString()).getEncoded();
+            } else {
+                throw new InvalidKeyException("Unexpected value: " + value);
+            }
+
+            if (data.available() != 0) {
+                throw new InvalidKeyException("Encoding has more than 4 values.");
             }
+
+        } catch (IOException | InvalidParameterSpecException e) {
+            throw new InvalidKeyException("Invalid EC private key", e);
         }
     }
 
@@ -324,4 +334,39 @@ private void checkDestroyed() {
             throw new IllegalStateException("This key is no longer valid");
         }
     }
+
+    /**
+     * Compares two private keys.
+     *
+     * The PKCS8Key.equals() method that compares encodings is used first.
+     *
+     * If that fails, we compare the private part of the key and the params to validate equivalence,
+     * since the keys might be equal but have different encodings if one or more of the optional
+     * parts are missing.
+     *
+     * @param object the object with which to compare
+     * @return {@code true} if this key is equal to the object argument; {@code false} otherwise.
+     */
+    @Override
+    public boolean equals(Object object) {
+        boolean sameEncoding = super.equals(object);
+        if (!sameEncoding) {
+            if (!(object instanceof java.security.interfaces.ECPrivateKey ecObj)) {
+                return false;
+            }
+
+            // 1. Compare the secret scalar (S)
+            if (!this.getS().equals(ecObj.getS())) {
+                return false;
+            }
+
+            // 2. Compare the Curve Parameters
+            ECParameterSpec s1 = this.getParams();
+            ECParameterSpec s2 = ecObj.getParams();
+
+            return ECUtils.equals(s1, s2);
+        }
+
+        return true;
+    }
 }

src/test/java/ibm/jceplus/junit/base/BaseTestECKeyImport.java

@@ -21,8 +21,11 @@
 import java.security.spec.ECGenParameterSpec;
 import java.security.spec.ECParameterSpec;
 import java.security.spec.ECPoint;
+import java.security.spec.ECPrivateKeySpec;
 import java.security.spec.EllipticCurve;
 import java.security.spec.EncodedKeySpec;
+import java.security.spec.InvalidKeySpecException;
+import java.security.spec.KeySpec;
 import java.security.spec.PKCS8EncodedKeySpec;
 import java.security.spec.X509EncodedKeySpec;
 import java.util.Arrays;
@@ -34,6 +37,7 @@
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 public class BaseTestECKeyImport extends BaseTestJunit5 {
 
@@ -92,6 +96,19 @@ public class BaseTestECKeyImport extends BaseTestJunit5 {
                                                 "Jlk6HKBiJlBvMWVSxEWYNipV2SOgCgYIKoZIzj0DAQehRANCAARFcF00hBK8Es2M" +
                                                 "H29DmA3fsYf4qWFSloVWoFct4CxffJ7hG0O4TXkMaPrAjgXc42SPdKRb7FcO0Lhz" +
                                                 "EVpYquVY";
+
+    private static final String private_secp256r1_parameters_twice_publickey =
+                                                "MIGhAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBIGGMIGDAgEBBCDXlTQ8RGqy6mfo" +
+                                                "fLkmWTocoGImUG8xZVLERZg2KlXZI6AKBggqhkjOPQMBB6AKBggqhkjOPQMBB6FE" +
+                                                "A0IABEVwXTSEErwSzYwfb0OYDd+xh/ipYVKWhVagVy3gLF98nuEbQ7hNeQxo+sCO" +
+                                                "BdzjZI90pFvsVw7QuHMRWliq5Vg=";
+
+    private static final String private_secp256r1_publickey_parameters =
+                                                "MIGTAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBHkwdwIBAQQg15U0PERqsupn6Hy5" +
+                                                "Jlk6HKBiJlBvMWVSxEWYNipV2SOhRANCAARFcF00hBK8Es2MH29DmA3fsYf4qWFS" +
+                                                "loVWoFct4CxffJ7hG0O4TXkMaPrAjgXc42SPdKRb7FcO0LhzEVpYquVYoAoGCCqG" +
+                                                "SM49AwEH";
+
     private static final String public_secp256r1_parameters_publickey =
                                                 "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAERXBdNIQSvBLNjB9vQ5gN37GH+Klh" +
                                                 "UpaFVqBXLeAsX3ye4RtDuE15DGj6wI4F3ONkj3SkW+xXDtC4cxFaWKrlWA==";
@@ -152,12 +169,17 @@ public void testCreateKeyPairECGenParamImport() throws Exception {
         EncodedKeySpec privateKeySpec = new PKCS8EncodedKeySpec(privKeyBytes);
         PrivateKey privateKey2 = keyFactory.generatePrivate(privateKeySpec);
 
+        KeySpec privateKeySpec2 = keyFactory.getKeySpec(privateKey, ECPrivateKeySpec.class);
+        PrivateKey privateKey3 = keyFactory.generatePrivate(privateKeySpec2);
+
         EncodedKeySpec publicKeySpec = new X509EncodedKeySpec(pubKeyBytes);
         PublicKey publicKey2 = keyFactory.generatePublic(publicKeySpec);
 
         // The original and new keys are the same
         boolean same = privateKey.equals(privateKey2);
         assertTrue(same);
+        same = privateKey.equals(privateKey3);
+        assertTrue(same);
         same = publicKey.equals(publicKey2);
         assertTrue(same);
     }
@@ -214,14 +236,20 @@ public void testCreateKeyPairECParamImport() throws Exception {
         EncodedKeySpec privateKeySpec = new PKCS8EncodedKeySpec(privKeyBytes);
         PrivateKey privateKey2 = keyFactory.generatePrivate(privateKeySpec);
 
+        KeySpec privateKeySpec2 = keyFactory.getKeySpec(privateKey, ECPrivateKeySpec.class);
+        PrivateKey privateKey3 = keyFactory.generatePrivate(privateKeySpec2);
+
         EncodedKeySpec publicKeySpec = new X509EncodedKeySpec(publicKeyBytes);
         PublicKey publicKey2 = keyFactory.generatePublic(publicKeySpec);
 
         // The original and new keys are the same
         boolean same = privateKey.equals(privateKey2);
         assertTrue(same);
+        same = privateKey.equals(privateKey3);
+        assertTrue(same);
         same = publicKey.equals(publicKey2);
         assertTrue(same);
+
         byte[] publicKey2Bytes = publicKey2.getEncoded();
         byte[] privateKey2Bytes = privateKey2.getEncoded();
 
@@ -383,6 +411,28 @@ public void testECPrivateKeyWithVariousOptionalFields() throws Exception {
         System.out.println("ALL tests completed.");
     }
 
+    @Test
+    public void testInvalidEncodings() throws Exception {
+        KeyFactory kf = KeyFactory.getInstance("EC", getProviderName());
+        try {
+            // Test encoding with two parameters.
+            kf.generatePrivate(new PKCS8EncodedKeySpec(Base64.getMimeDecoder()
+                    .decode(private_secp256r1_parameters_twice_publickey)));
+            fail("Expected InvalidKeySpecException not thrown.");
+        } catch (InvalidKeySpecException ikse) {
+            assertTrue("Inappropriate key specification: ".equals(ikse.getMessage()));
+        }
+
+        try {
+            // Test encoding where the public key is before the parameters.
+            kf.generatePrivate(new PKCS8EncodedKeySpec(Base64.getMimeDecoder()
+                    .decode(private_secp256r1_publickey_parameters)));
+            fail("Expected InvalidKeySpecException not thrown.");
+        } catch (InvalidKeySpecException ikse) {
+            assertTrue("Inappropriate key specification: ".equals(ikse.getMessage()));
+        }
+    }
+
     private static void testSignAndVerify(
             String label,
             KeyFactory kf,

src/test/java/ibm/jceplus/junit/base/BaseTestECKeyImportInterop.java

@@ -1,5 +1,5 @@
 /*
- * Copyright IBM Corp. 2023, 2024
+ * Copyright IBM Corp. 2023, 2026
  *
  * This code is free software; you can redistribute it and/or modify it
  * under the terms provided by IBM in the LICENSE file that accompanied
@@ -19,8 +19,10 @@
 import java.security.spec.ECGenParameterSpec;
 import java.security.spec.ECParameterSpec;
 import java.security.spec.ECPoint;
+import java.security.spec.ECPrivateKeySpec;
 import java.security.spec.EllipticCurve;
 import java.security.spec.EncodedKeySpec;
+import java.security.spec.KeySpec;
 import java.security.spec.PKCS8EncodedKeySpec;
 import java.security.spec.X509EncodedKeySpec;
 import java.util.Arrays;
@@ -114,6 +116,52 @@ public void doCreateKeyPairECGenParamImport(String generateProviderName,
         assertTrue(Arrays.equals(privateKey2.getEncoded(), privKeyBytes));
     }
 
+    @Test
+    public void testCreateKeyPairECImportCompareKeys() throws Exception {
+        doCreateKeyPairECImportCompareKeys(getProviderName(), getInteropProviderName());
+        doCreateKeyPairECImportCompareKeys(getInteropProviderName(), getProviderName());
+    }
+
+    private void doCreateKeyPairECImportCompareKeys(String createProviderName,
+            String importProviderName) throws Exception {
+
+        //final String methodName = "testCreateKeyPairECImportCompareKeys";
+
+        KeyPairGenerator keyPairGen = KeyPairGenerator.getInstance("EC", createProviderName);
+
+        keyPairGen.initialize(256);
+        KeyPair keyPair = keyPairGen.generateKeyPair();
+        PrivateKey privateKey = keyPair.getPrivate();
+        PublicKey publicKey = keyPair.getPublic();
+
+        byte[] publicKeyBytes = publicKey.getEncoded();
+        byte[] privKeyBytes = privateKey.getEncoded();
+
+        KeyFactory keyFactory = KeyFactory.getInstance("EC", importProviderName);
+        EncodedKeySpec privateKeySpec = new PKCS8EncodedKeySpec(privKeyBytes);
+        PrivateKey privateKey2 = keyFactory.generatePrivate(privateKeySpec);
+
+        KeySpec privateKeySpec2 = keyFactory.getKeySpec(privateKey, ECPrivateKeySpec.class);
+        PrivateKey privateKey3 = keyFactory.generatePrivate(privateKeySpec2);
+
+        EncodedKeySpec publicKeySpec = new X509EncodedKeySpec(publicKeyBytes);
+        PublicKey publicKey2 = keyFactory.generatePublic(publicKeySpec);
+
+        // The original and new keys are the same
+        boolean same = privateKey.equals(privateKey2);
+        assertTrue(same);
+        same = privateKey.equals(privateKey3);
+        assertTrue(same);
+        same = publicKey.equals(publicKey2);
+        assertTrue(same);
+
+        byte[] publicKey2Bytes = publicKey2.getEncoded();
+        byte[] privateKey2Bytes = privateKey2.getEncoded();
+
+        assertArrayEquals(publicKeyBytes, publicKey2Bytes);
+        assertArrayEquals(privKeyBytes, privateKey2Bytes);
+    }
+
     @Test
     public void testCreateKeyPairECParamCustomCurveImport() throws Exception {
         doCreateKeyPairECParamCustomCurveImport(getProviderName(), getInteropProviderName());