From 9dc1bf8937fdf4e9ed9a8bdd7d9421967ee84acf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lum=C3=ADr=20N=C3=A1vrat?= <rimuln@users.noreply.github.com>
Date: Mon, 3 Aug 2026 14:28:41 +0200
Subject: [PATCH] Fix engineGetCertificateAlias returning the alias of an
 unrelated certificate

engineGetCertificateAlias took the certificate from certs.elements() and the
alias from certs.keys(), advancing both in lockstep. That relies on the two
enumerations visiting the table in the same order, which stopped holding when
IgnoresCaseHashtable.keys() began enumerating a copy of the table while
elements() kept enumerating the original: new Hashtable(Map) sizes its bucket
array from the entry count, whereas the original reached its capacity by
incremental rehashing, so for a store with enough entries the layouts differ and
the method returns the wrong alias with no error.

Look the value up by key instead, which does not depend on the two enumerations
agreeing. Applied to the keyCerts loop as well - keyCerts is a plain Hashtable
whose keys() and elements() do agree, so it was not affected, but the pairing was
the same latent fragility.

testGetCertificateAlias covers it with 12 certificate entries plus a negative
case; twelve is needed because with only a few entries the table and its copy can
share a layout and the defect does not reproduce.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .../keystore/pkcs12/PKCS12KeyStoreSpi.java    | 19 ++++-----
 .../jce/provider/test/PKCS12StoreTest.java    | 41 +++++++++++++++++++
 2 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/prov/src/main/java/org/bouncycastle/jcajce/provider/keystore/pkcs12/PKCS12KeyStoreSpi.java b/prov/src/main/java/org/bouncycastle/jcajce/provider/keystore/pkcs12/PKCS12KeyStoreSpi.java
index ddca9f860b..3eaf55059a 100644
--- a/prov/src/main/java/org/bouncycastle/jcajce/provider/keystore/pkcs12/PKCS12KeyStoreSpi.java
+++ b/prov/src/main/java/org/bouncycastle/jcajce/provider/keystore/pkcs12/PKCS12KeyStoreSpi.java
@@ -438,29 +438,24 @@ public Certificate engineGetCertificate(
     public String engineGetCertificateAlias(
         Certificate cert)
     {
-        Enumeration c = certs.elements();
-        Enumeration k = certs.keys();
-
-        while (c.hasMoreElements())
+        // look the certificate up by alias rather than advancing keys() and elements() in
+        // lockstep - IgnoresCaseHashtable.keys() enumerates a copy of the table while
+        // elements() enumerates the original, so the two orders are not required to agree.
+        for (Enumeration k = certs.keys(); k.hasMoreElements();)
         {
-            Certificate tc = (Certificate)c.nextElement();
             String ta = (String)k.nextElement();
 
-            if (tc.equals(cert))
+            if (cert.equals(certs.get(ta)))
             {
                 return ta;
             }
         }
 
-        c = keyCerts.elements();
-        k = keyCerts.keys();
-
-        while (c.hasMoreElements())
+        for (Enumeration k = keyCerts.keys(); k.hasMoreElements();)
         {
-            Certificate tc = (Certificate)c.nextElement();
             String ta = (String)k.nextElement();
 
-            if (tc.equals(cert))
+            if (cert.equals(keyCerts.get(ta)))
             {
                 return ta;
             }
diff --git a/prov/src/test/java/org/bouncycastle/jce/provider/test/PKCS12StoreTest.java b/prov/src/test/java/org/bouncycastle/jce/provider/test/PKCS12StoreTest.java
index f463a2e63f..620f7b7a70 100644
--- a/prov/src/test/java/org/bouncycastle/jce/provider/test/PKCS12StoreTest.java
+++ b/prov/src/test/java/org/bouncycastle/jce/provider/test/PKCS12StoreTest.java
@@ -2417,6 +2417,46 @@ private void testChainCycle()
         keyStore.getEntry("cycle", new KeyStore.PasswordProtection("test".toCharArray()));
     }
 
+    private void testGetCertificateAlias()
+        throws Exception
+    {
+        KeyStore keyStore = KeyStore.getInstance("PKCS12", BC);
+
+        keyStore.load(null, null);
+
+        // enough entries that the backing table has grown past its initial capacity: a copy of
+        // the table is sized from its entry count instead, so the two end up with different
+        // bucket layouts and any order-dependent pairing of keys() with elements() diverges.
+        KeyPair keyPair = TestUtils.generateRSAKeyPair();
+        int certCount = 12;
+
+        X509Certificate[] certs = new X509Certificate[certCount];
+        String[] aliases = new String[certCount];
+
+        for (int i = 0; i != certCount; i++)
+        {
+            aliases[i] = "cert-" + i;
+            certs[i] = TestUtils.createSelfSignedCert("CN=Test Certificate " + i, "SHA256withRSA", keyPair);
+
+            keyStore.setCertificateEntry(aliases[i], certs[i]);
+        }
+
+        for (int i = 0; i != certCount; i++)
+        {
+            String alias = keyStore.getCertificateAlias(certs[i]);
+
+            if (!aliases[i].equals(alias))
+            {
+                fail("getCertificateAlias returned wrong alias: expected " + aliases[i] + ", got " + alias);
+            }
+        }
+
+        if (keyStore.getCertificateAlias(TestUtils.createSelfSignedCert("CN=Absent", "SHA256withRSA", keyPair)) != null)
+        {
+            fail("getCertificateAlias returned an alias for a certificate that is not in the store");
+        }
+    }
+
     private void testOrphanedCertCleanup()
         throws Exception
     {
@@ -2798,6 +2838,7 @@ public void performTest()
         testPKCS12Store();
         testGOSTStore();
         testChainCycle();
+        testGetCertificateAlias();
         testBCFKSLoad();
         testCertsOnly();
         testJKS();