PKCS12KeyStoreSpi.engineGetCertificateAlias returns the alias of an unrelated certificate (since 1.80)

[rimuln]

Summary

KeyStore.getCertificateAlias(cert) on a BC PKCS12 keystore returns the alias of a different
certificate. The returned alias exists in the store, so callers get a plausible but wrong answer with
no exception.

Present in 1.80, 1.80.2, 1.81.1, 1.83, 1.84 and 1.85 (all tested). 1.79 is correct.

Cause

PKCS12KeyStoreSpi.engineGetCertificateAlias pairs two enumerations positionally:

Enumeration c = certs.elements();
Enumeration k = certs.keys();

while (c.hasMoreElements())
{
    Certificate tc = (Certificate)c.nextElement();
    String ta = (String)k.nextElement();

    if (tc.equals(cert))
    {
        return ta;
    }
}

That holds only while keys() and elements() walk the same table in the same order. Commit
a5be993 ("Changed IgnoresCaseHashtable.keys() to copy the table content to ensure consistency",
first released in 1.80) changed keys() to enumerate a copy, while elements() still enumerates the
original:

public Enumeration keys()
{
    return new Hashtable(orig).keys();   // copy
}

public Enumeration elements()
{
    return orig.elements();              // original
}

new Hashtable(Map) sizes its bucket array as max(2 * size, 11), whereas orig reached its
capacity by incremental rehashing. Once the store holds enough entries for those to differ, the two
enumerations no longer agree and ta is the key at position i of the copy while tc is the value
at position i of the original.

engineGetCertificateAlias was not touched by that commit and is unchanged from r1rv79 through
r1rv85, so this looks like an unintended side effect rather than a deliberate change.

Reproduction

Security.addProvider(new BouncyCastleProvider());

KeyStore store = KeyStore.getInstance("PKCS12", "BC");
store.load(null, null);

// 12 entries is enough for the original table and a copy of it to differ in layout
for (int i = 0; i != 12; i++)
{
    store.setCertificateEntry("cert-" + i, certs[i]);
}

for (int i = 0; i != 12; i++)
{
    System.out.println("cert-" + i + " -> " + store.getCertificateAlias(certs[i]));
}

Expected cert-i -> cert-i for every entry. On 1.80+ the mapping is permuted.

Originally hit on a real store holding one PrivateKeyEntry with a three-certificate chain plus four
trusted certificate entries, where every chain certificate resolved to an unrelated alias:

chain certificate	1.79	1.80 .. 1.85
CN=level2 (leaf)	key-entry	trusted-c
CN=level1	CN=level1	trusted-b
CN=root	CN=root	key-entry

Every certificate's fingerprint and the full alias list are identical across versions — only the
lookup result changes. A store small enough that the table has not yet grown will not reproduce it,
which is likely why it went unnoticed.

Impact

Silent misidentification in any code that maps a certificate back to its keystore alias — certificate
chain classification, trust-store bookkeeping, deciding whether a received certificate is already
stored. Nothing throws, so it fails quietly. In the application where I found it, an incoming
certificate was reported as already present (and so not stored) and then associated with a different
peer's certificate.

Suggested fix

The copy in keys() was added deliberately, so the caller is the better place to fix: look the value
up by key instead of relying on the two enumerations agreeing.

for (Enumeration k = certs.keys(); k.hasMoreElements();)
{
    String ta = (String)k.nextElement();

    if (cert.equals(certs.get(ta)))
    {
        return ta;
    }
}

The same applies to the keyCerts loop below it. PR to follow.

Note the prov/src/main/jdk1.3 and prov/src/main/jdk1.4 copies of this class still have
return orig.keys();, so they behave correctly today, but they carry the same order-dependent
pairing in engineGetCertificateAlias.

Environment

• bcprov-jdk18on 1.80 – 1.85 (1.79 unaffected)
• JDK 21 and JDK 25
• KeyStore.getInstance("PKCS12", "BC")

[dghgit]

Thanks for the patch, now up on https://www.bouncycastle.org/betas

[dghgit]

I've published a patch release 1.85.2 on https://www.bouncycastle.org/betas which includes the fix.

Let me know if it works for you and I will get it published to Maven Central.