8185844: MSCAPI doesn't list aliases correctly

Reviewed-by: valeriep

src/jdk.crypto.mscapi/windows/classes/sun/security/mscapi/CKeyStore.java

@@ -165,6 +165,25 @@ abstract class CKeyStore extends KeyStoreSpi {
             }
             certChain = chain;
         }
+
+        public void delete() throws KeyStoreException {
+            // Get end-entity certificate and remove from system cert store
+            X509Certificate[] certChain = getCertificateChain();
+            if (certChain != null && certChain.length > 0) {
+                try {
+                    byte[] encoding = certChain[0].getEncoded();
+                    removeCertificate(getName(), getAlias(), encoding,
+                            encoding.length);
+                } catch (CertificateException e) {
+                    throw new KeyStoreException("Cannot remove entry: ", e);
+                }
+            }
+            CKey privateKey = getPrivateKey();
+            if (privateKey != null) {
+                destroyKeyContainer(
+                        CKey.getContainerName(privateKey.getHCryptProvider()));
+            }
+        }
     }
 
     /*
@@ -368,8 +387,6 @@ abstract class CKeyStore extends KeyStoreSpi {
 
         if (key instanceof RSAPrivateCrtKey) {
 
-            KeyEntry entry = entries.get(alias);
-
             X509Certificate[] xchain;
             if (chain != null) {
                 if (chain instanceof X509Certificate[]) {
@@ -382,26 +399,20 @@ abstract class CKeyStore extends KeyStoreSpi {
                 xchain = null;
             }
 
-            if (entry == null) {
+            KeyEntry oldEntry = entries.get(alias);
-                entry =
-                    //TODO new KeyEntry(alias, key, (X509Certificate[]) chain);
-                    new KeyEntry(alias, null, xchain);
-                storeWithUniqueAlias(alias, entry);
-            }
-
-            entry.setAlias(alias);
 
             try {
-                entry.setRSAPrivateKey(key);
+                KeyEntry newEntry = new KeyEntry(alias, null, xchain);
-                entry.setCertificateChain(xchain);
+                newEntry.setRSAPrivateKey(key);
-
+                newEntry.setCertificateChain(xchain);
-            } catch (CertificateException ce) {
+                entries.put(alias, newEntry);
-                throw new KeyStoreException(ce);
+            } catch (CertificateException | InvalidKeyException e) {
-
+                throw new KeyStoreException(e);
-            } catch (InvalidKeyException ike) {
-                throw new KeyStoreException(ike);
             }
 
+            if (oldEntry != null) {
+                oldEntry.delete();
+            }
         } else {
             throw new UnsupportedOperationException(
                 "Cannot assign the key to the given alias.");
@@ -463,25 +474,23 @@ abstract class CKeyStore extends KeyStoreSpi {
             // TODO - build CryptoAPI chain?
             X509Certificate[] chain =
                 new X509Certificate[]{ (X509Certificate) cert };
-            KeyEntry entry = entries.get(alias);
 
-            if (entry == null) {
+            KeyEntry oldEntry = entries.get(alias);
-                entry =
+            if (oldEntry != null && oldEntry.privateKey != null) {
-                    new KeyEntry(alias, null, chain);
+                throw new KeyStoreException("Cannot overwrite key entry");
-                storeWithUniqueAlias(alias, entry);
             }
 
-            if (entry.getPrivateKey() == null) { // trusted-cert entry
+            try {
-                entry.setAlias(alias);
+                KeyEntry newEntry = new KeyEntry(alias, null, chain);
-
+                newEntry.setCertificateChain(chain);
-                try {
+                entries.put(alias, newEntry);
-                    entry.setCertificateChain(chain);
+            } catch (CertificateException ce) {
-
+                throw new KeyStoreException(ce);
-                } catch (CertificateException ce) {
-                    throw new KeyStoreException(ce);
-                }
             }
 
+            if (oldEntry != null) {
+                oldEntry.delete();
+            }
         } else {
             throw new UnsupportedOperationException(
                 "Cannot assign the certificate to the given alias.");
@@ -502,25 +511,7 @@ abstract class CKeyStore extends KeyStoreSpi {
 
         KeyEntry entry = entries.remove(alias);
         if (entry != null) {
-            // Get end-entity certificate and remove from system cert store
+            entry.delete();
-            X509Certificate[] certChain = entry.getCertificateChain();
-            if (certChain != null && certChain.length > 0) {
-
-                try {
-
-                    byte[] encoding = certChain[0].getEncoded();
-                    removeCertificate(getName(), entry.getAlias(), encoding,
-                            encoding.length);
-
-                } catch (CertificateException e) {
-                    throw new KeyStoreException("Cannot remove entry: ", e);
-                }
-            }
-            CKey privateKey = entry.getPrivateKey();
-            if (privateKey != null) {
-                destroyKeyContainer(
-                    CKey.getContainerName(privateKey.getHCryptProvider()));
-            }
         }
     }
 

test/jdk/sun/security/mscapi/SetDupNameEntry.java

@@ -0,0 +1,98 @@
+/*
+ * Copyright (c) 2021, 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.
+ */
+
+import java.security.KeyStore;
+import java.security.cert.Certificate;
+import java.security.cert.X509Certificate;
+import java.util.Collections;
+
+import jdk.test.lib.Asserts;
+import sun.security.tools.keytool.CertAndKeyGen;
+import sun.security.x509.X500Name;
+
+/*
+ * @test
+ * @bug 8185844
+ * @summary ensure setEntry overwrite old entry
+ * @library /test/lib
+ * @requires os.family == "windows"
+ * @modules java.base/sun.security.tools.keytool
+ *          java.base/sun.security.x509
+ */
+public class SetDupNameEntry {
+
+    final KeyStore keyStore;
+    final CertAndKeyGen ckg;
+
+    static final String PREFIX = "8185844";
+
+    public static void main(String[] args) throws Exception {
+        SetDupNameEntry test = new SetDupNameEntry();
+        test.cleanup();
+        try {
+            test.test(true);    // test key entry
+            test.test(false);   // test cert entry
+        } finally {
+            test.cleanup();
+        }
+    }
+
+    SetDupNameEntry() throws Exception {
+        keyStore = KeyStore.getInstance("Windows-MY");
+        ckg = new CertAndKeyGen("RSA", "SHA1withRSA");
+    }
+
+    void test(boolean testKey) throws Exception {
+        keyStore.load(null, null);
+        int size = keyStore.size();
+
+        String alias = PREFIX + (testKey ? "k" : "c");
+        for (int i = 0; i < 2; i++) {
+            ckg.generate(1024);
+            X509Certificate cert = ckg
+                    .getSelfCertificate(new X500Name("CN=TEST"), 1000);
+            if (testKey) {
+                keyStore.setKeyEntry(
+                        alias,
+                        ckg.getPrivateKey(),
+                        null,
+                        new Certificate[] { cert });
+            } else {
+                keyStore.setCertificateEntry(alias, cert);
+            }
+        }
+        Asserts.assertEQ(keyStore.size(), size + 1);
+
+        keyStore.load(null, null);
+        Asserts.assertEQ(keyStore.size(), size + 1);
+    }
+
+    void cleanup() throws Exception {
+        keyStore.load(null, null);
+        for (String alias : Collections.list(keyStore.aliases())) {
+            if (alias.startsWith(PREFIX)) {
+                keyStore.deleteEntry(alias);
+            }
+        }
+    }
+}