PKCS#12 keystores inherit a loaded file's MAC KDF parameters and write them out again

[Arpan0995]

PKCS12PBMAC1KeyStoreSpi.engineLoad copies the MAC AlgorithmIdentifier out of the file it is reading into the field macAlgorithm (PKCS12PBMAC1KeyStoreSpi.java:1034), and takes the MAC iteration count and salt length from the same MacData at :1036 and :1037. calculatePbeMac, which doStore reaches on every write that carries a MAC, mints fresh PBMAC1 parameters only when macAlgorithm.getParameters() == null (:2282), the 32-byte salt being generated at :2284 and :2285 and the default count, key length and PRF set at :2290. So after a load that branch no longer runs, and each file the object writes carries the loaded file's PBKDF2 salt, iteration count, key length and PRF, under whatever password the caller stores with. The three assignments sit ahead of the constant-time comparison at :1044, and engineLoad(null, ...) returns at :979 without clearing them. The classic PKCS12KeyStoreSpi latches the same fields at :1032, :1034, :1035 ahead of its comparison at :1042, sizes the MAC salt it writes from the inherited length at :2113, and hands the latched macAlgorithm back to the MAC it writes at :2129 and :2131. Line numbers are origin/main ab16374d37, release-note lines included; the two keystore SPI files are byte identical at tag r1rv86 and in the 1.87-SNAPSHOT beta sources, and the behaviour below was the same on the 1.86 jars and on the beta jars.

Reproduction

Build a PFX declaring pbkdf2Salt=00*32, iterationCount=1, keyLength=64, with a MAC valid under its own password; load it; store under a different password. The same run covers a load that fails the MAC check (rows F1 to F3), the classic keystore (rows C0 to C6) and a PKCS12-PBMAC1 object that never loads anything (rows Q1 to Q3).

Full harness

import java.io.*; import java.math.BigInteger; import java.security.*;
import java.security.cert.Certificate; import java.security.cert.X509Certificate; import java.util.*;
import javax.crypto.Mac; import javax.crypto.spec.PBEParameterSpec;
import org.bouncycastle.asn1.*; import org.bouncycastle.asn1.pkcs.*;
import org.bouncycastle.asn1.x500.X500Name; import org.bouncycastle.asn1.x509.*;
import org.bouncycastle.cert.jcajce.*; import org.bouncycastle.crypto.*;
import org.bouncycastle.crypto.digests.*; import org.bouncycastle.crypto.generators.PKCS5S2ParametersGenerator;
import org.bouncycastle.crypto.macs.HMac; import org.bouncycastle.jcajce.PKCS12Key;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder;
import org.bouncycastle.util.Strings; import org.bouncycastle.util.encoders.Hex;

public class A1
{
    static KeyPair kp; static X509Certificate cert;

    static void dump(String tag, byte[] pfx)
    {
        MacData md = Pfx.getInstance(pfx).getMacData();
        AlgorithmIdentifier ai = md.getMac().getAlgorithmId();
        String s;
        if (PKCSObjectIdentifiers.id_PBMAC1.equals(ai.getAlgorithm()))
        {
            PBMAC1Params pp = PBMAC1Params.getInstance(ai.getParameters());
            PBKDF2Params k2 = PBKDF2Params.getInstance(pp.getKeyDerivationFunc().getParameters());
            s = "PBMAC1 pbkdf2Salt=" + Hex.toHexString(k2.getSalt()) + " it=" + k2.getIterationCount()
                + " keyLen=" + k2.getKeyLength() + " prf=" + k2.getPrf().getAlgorithm()
                + " authScheme=" + pp.getMessageAuthScheme().getAlgorithm()
                + "  [MacData salt=" + Hex.toHexString(md.getSalt()) + " it=" + md.getIterationCount() + "]";
        }
        else
        {
            s = "classic macAlg=" + ai.getAlgorithm() + " saltLen=" + md.getSalt().length
                + " salt=" + Hex.toHexString(md.getSalt()) + " itCount=" + md.getIterationCount();
        }
        System.out.println(tag + ": " + s);
    }

    static byte[] octets(ContentInfo ci) { return ASN1OctetString.getInstance(ci.getContent()).getOctets(); }

    static Digest prf(ASN1ObjectIdentifier o)
    { return PKCSObjectIdentifiers.id_hmacWithSHA512.equals(o) ? new SHA512Digest() : new SHA256Digest(); }

    static byte[] pbmac1(PBMAC1Params pm, char[] pw, byte[] data)
    {
        PBKDF2Params k2 = PBKDF2Params.getInstance(pm.getKeyDerivationFunc().getParameters());
        PKCS5S2ParametersGenerator g = new PKCS5S2ParametersGenerator(prf(k2.getPrf().getAlgorithm()));
        g.init(Strings.toUTF8ByteArray(pw), k2.getSalt(), k2.getIterationCount().intValue());
        HMac h = new HMac(prf(pm.getMessageAuthScheme().getAlgorithm()));
        h.init(g.generateDerivedParameters(k2.getKeyLength().intValue() * 8));
        h.update(data, 0, data.length);
        byte[] r = new byte[h.getMacSize()]; h.doFinal(r, 0); return r;
    }

    // a PBMAC1 PFX declaring the given PBKDF2 salt / iterations / keyLength; validMac recomputes it under pw
    static byte[] craftPbmac1(byte[] good, char[] pw, byte[] salt, int it, int keyLen, boolean validMac) throws Exception
    {
        ContentInfo ci = Pfx.getInstance(good).getAuthSafe();
        PBMAC1Params pm = new PBMAC1Params(new AlgorithmIdentifier(PKCSObjectIdentifiers.id_PBKDF2,
            new PBKDF2Params(salt, it, keyLen, new AlgorithmIdentifier(PKCSObjectIdentifiers.id_hmacWithSHA256))),
            new AlgorithmIdentifier(PKCSObjectIdentifiers.id_hmacWithSHA512));
        byte[] m = validMac ? pbmac1(pm, pw, octets(ci)) : new byte[64];
        DigestInfo di = new DigestInfo(new AlgorithmIdentifier(PKCSObjectIdentifiers.id_PBMAC1, pm), m);
        return new Pfx(ci, new MacData(di, new byte[8], 1)).getEncoded(ASN1Encoding.DER);
    }

    static byte[] craftClassic(byte[] good, char[] pw, byte[] salt, int it) throws Exception
    {
        Pfx p = Pfx.getInstance(good); ContentInfo ci = p.getAuthSafe();
        AlgorithmIdentifier macAlg = p.getMacData().getMac().getAlgorithmId();
        Mac m = Mac.getInstance(macAlg.getAlgorithm().getId(), "BC");
        m.init(new PKCS12Key(pw, false), new PBEParameterSpec(salt, it));
        m.update(octets(ci));
        return new Pfx(ci, new MacData(new DigestInfo(macAlg, m.doFinal()), salt, it)).getEncoded(ASN1Encoding.DER);
    }

    static byte[] write(KeyStore ks, char[] pw) throws Exception
    { ByteArrayOutputStream o = new ByteArrayOutputStream(); ks.store(o, pw); return o.toByteArray(); }

    static byte[] freshFile(String type, char[] pw) throws Exception
    {
        KeyStore k = KeyStore.getInstance(type, "BC"); k.load(null, null);
        k.setKeyEntry("k", kp.getPrivate(), "kp".toCharArray(), new Certificate[]{cert}); return write(k, pw);
    }

    public static void main(String[] args) throws Exception
    {
        Security.addProvider(new BouncyCastleProvider());
        KeyPairGenerator g = KeyPairGenerator.getInstance("EC", "BC"); g.initialize(256);
        kp = g.generateKeyPair(); X500Name n = new X500Name("CN=test");
        cert = new JcaX509CertificateConverter().setProvider("BC").getCertificate(
            new JcaX509v3CertificateBuilder(n, BigInteger.ONE, new Date(System.currentTimeMillis() - 86400000L),
                new Date(System.currentTimeMillis() + 86400000L), n, kp.getPublic()).build(
                new JcaContentSignerBuilder("SHA256withECDSA").setProvider("BC").build(kp.getPrivate())));

        char[] sourcePw = "SourceFilePassword".toCharArray();
        char[] victimPw = "VictimStorePassword!".toCharArray();

        // ---- P. PKCS12-PBMAC1, load that SUCCEEDS under the correct password ----
        byte[] base = freshFile("PKCS12-PBMAC1", sourcePw);
        dump("P0 fresh instance default write ", base);
        byte[] src = craftPbmac1(base, sourcePw, new byte[32], 1, 64, true);
        dump("P1 source file (valid MAC)      ", src);

        KeyStore v = KeyStore.getInstance("PKCS12-PBMAC1", "BC");
        v.load(new ByteArrayInputStream(src), sourcePw);
        System.out.println("P2 successful load, aliases=" + Collections.list(v.aliases()));
        byte[] out = write(v, victimPw);                   // store under a DIFFERENT password
        dump("P3 store under different pw     ", out);

        Pfx op = Pfx.getInstance(out);
        PBMAC1Params opm = PBMAC1Params.getInstance(op.getMacData().getMac().getAlgorithmId().getParameters());
        System.out.println("P3b output MAC recomputes with inherited params + victim pw: "
            + org.bouncycastle.util.Arrays.areEqual(
                pbmac1(opm, victimPw, octets(op.getAuthSafe())), op.getMacData().getMac().getDigest()));
        KeyStore rv = KeyStore.getInstance("PKCS12-PBMAC1", "BC");
        rv.load(new ByteArrayInputStream(out), victimPw);
        System.out.println("P3c output re-loads OK, aliases=" + Collections.list(rv.aliases()));

        v.load(null, null);                                // re-initialise the same object
        v.setKeyEntry("after", kp.getPrivate(), "kp".toCharArray(), new Certificate[]{cert});
        dump("P4 after load(null,null) + store", write(v, victimPw));
        dump("P5 second store, same object    ", write(v, victimPw));
        dump("P6 control fresh instance       ", freshFile("PKCS12-PBMAC1", victimPw));

        // ---- F. PKCS12-PBMAC1, load that FAILS (MAC does not verify) ----
        byte[] bad = craftPbmac1(base, sourcePw, new byte[32], 1, 64, false);
        KeyStore f = KeyStore.getInstance("PKCS12-PBMAC1", "BC");
        try { f.load(new ByteArrayInputStream(bad), "wrong-password".toCharArray()); System.out.println("F1 load threw   : NONE"); }
        catch (Exception e) { System.out.println("F1 load threw   : " + e.getClass().getName() + ": " + e.getMessage()); }
        f.load(null, null);
        System.out.println("F2 aliases after load(null,null)=" + Collections.list(f.aliases()));
        f.setKeyEntry("victim", kp.getPrivate(), "kp".toCharArray(), new Certificate[]{cert});
        dump("F3 victim's OWN file            ", write(f, victimPw));

        // ---- C. classic PKCS12, source file declaring a zero length MAC salt ----
        byte[] cbase = freshFile("PKCS12", sourcePw);
        dump("C0 fresh instance default write ", cbase);
        byte[] csrc = craftClassic(cbase, sourcePw, new byte[0], 1);
        dump("C1 source file (valid MAC)      ", csrc);
        KeyStore cv = KeyStore.getInstance("PKCS12", "BC");
        cv.load(new ByteArrayInputStream(csrc), sourcePw);
        System.out.println("C2 successful load, aliases=" + Collections.list(cv.aliases()));
        byte[] cout = write(cv, victimPw);
        dump("C3 store under different pw     ", cout);
        cv.load(null, null);
        cv.setKeyEntry("after", kp.getPrivate(), "kp".toCharArray(), new Certificate[]{cert});
        dump("C4 after load(null,null) + store", write(cv, victimPw));
        dump("C5 control fresh instance       ", freshFile("PKCS12", victimPw));

        // ---- Q. no loaded file at all: one object, three store() calls, three passwords ----
        KeyStore q = KeyStore.getInstance("PKCS12-PBMAC1", "BC"); q.load(null, null);
        q.setKeyEntry("k", kp.getPrivate(), "kp".toCharArray(), new Certificate[]{cert});
        dump("Q1 store#1 (pw=store-password-1)", write(q, "store-password-1".toCharArray()));
        dump("Q2 store#2 (pw=store-password-2)", write(q, "store-password-2".toCharArray()));
        dump("Q3 store#3 (pw=store-password-3)", write(q, "store-password-3".toCharArray()));

        // the key bags in the C3 output, for the contrast with the MAC
        AuthenticatedSafe as = AuthenticatedSafe.getInstance(
            ASN1Primitive.fromByteArray(octets(Pfx.getInstance(cout).getAuthSafe())));
        StringBuilder sb = new StringBuilder();
        ContentInfo[] cis = as.getContentInfo();
        for (int i = 0; i != cis.length; i++)
        {
            if (cis[i].getContentType().equals(PKCSObjectIdentifiers.encryptedData))
            {
                EncryptedData ed = EncryptedData.getInstance(cis[i].getContent());
                sb.append(" encData[" + ed.getEncryptionAlgorithm().getAlgorithm()
                    + " params=" + ed.getEncryptionAlgorithm().getParameters() + "]");
            }
        }
        System.out.println("C6 classic victim output key-bag PBE:" + sb);
    }
}

P0 fresh instance default write : PBMAC1 pbkdf2Salt=10bce0879b0c4d95fbb2851715f2c7c2481249b8af0bc8d19a21834bbe330304 it=65536 keyLen=64 prf=1.2.840.113549.2.9 authScheme=1.2.840.113549.2.11  [MacData salt=d3e68e506128d74fb75501835c8c7c6f7ee801d5 it=1200000]
P1 source file (valid MAC)      : PBMAC1 pbkdf2Salt=0000000000000000000000000000000000000000000000000000000000000000 it=1 keyLen=64 prf=1.2.840.113549.2.9 authScheme=1.2.840.113549.2.11  [MacData salt=0000000000000000 it=1]
P2 successful load, aliases=[k]
P3 store under different pw     : PBMAC1 pbkdf2Salt=0000000000000000000000000000000000000000000000000000000000000000 it=1 keyLen=64 prf=1.2.840.113549.2.9 authScheme=1.2.840.113549.2.11  [MacData salt=245a72b5e6a58fc6 it=1]
P3b output MAC recomputes with inherited params + victim pw: true
P3c output re-loads OK, aliases=[k]
P4 after load(null,null) + store: PBMAC1 pbkdf2Salt=0000000000000000000000000000000000000000000000000000000000000000 it=1 keyLen=64 prf=1.2.840.113549.2.9 authScheme=1.2.840.113549.2.11  [MacData salt=082cdba2e09428e7 it=1]
P5 second store, same object    : PBMAC1 pbkdf2Salt=0000000000000000000000000000000000000000000000000000000000000000 it=1 keyLen=64 prf=1.2.840.113549.2.9 authScheme=1.2.840.113549.2.11  [MacData salt=baddbeed94efa336 it=1]
P6 control fresh instance       : PBMAC1 pbkdf2Salt=2d84d9ae4106bef9fe324d69a4d4e8055e641568732480b2cdc2e0c173037355 it=65536 keyLen=64 prf=1.2.840.113549.2.9 authScheme=1.2.840.113549.2.11  [MacData salt=8dae5adf6cf26a88bb5cba7ed39b4ffe6c2e60d1 it=1200000]
F1 load threw   : java.io.IOException: PKCS12 key store mac invalid - wrong password or corrupted file
F2 aliases after load(null,null)=[]
F3 victim's OWN file            : PBMAC1 pbkdf2Salt=0000000000000000000000000000000000000000000000000000000000000000 it=1 keyLen=64 prf=1.2.840.113549.2.9 authScheme=1.2.840.113549.2.11  [MacData salt=51206fb6ec1cea93 it=1]
C0 fresh instance default write : classic macAlg=1.3.14.3.2.26 saltLen=20 salt=573fd59858f1e12235dab3136c08546850183e5a itCount=1200000
C1 source file (valid MAC)      : classic macAlg=1.3.14.3.2.26 saltLen=0 salt= itCount=1
C2 successful load, aliases=[k]
C3 store under different pw     : classic macAlg=1.3.14.3.2.26 saltLen=0 salt= itCount=1
C4 after load(null,null) + store: classic macAlg=1.3.14.3.2.26 saltLen=0 salt= itCount=1
C5 control fresh instance       : classic macAlg=1.3.14.3.2.26 saltLen=20 salt=4b6e3ebeec6d795a94164a2dcca70b1a217d2905 itCount=1200000
Q1 store#1 (pw=store-password-1): PBMAC1 pbkdf2Salt=c87204ee509893967e68736a9538b78fed2b21044f85bffb8c18e8263fbc79e8 it=65536 keyLen=64 prf=1.2.840.113549.2.9 authScheme=1.2.840.113549.2.11  [MacData salt=d6a44ad1b2e471eb9d23c1bfd9b32b1ba64481f3 it=1200000]
Q2 store#2 (pw=store-password-2): PBMAC1 pbkdf2Salt=c87204ee509893967e68736a9538b78fed2b21044f85bffb8c18e8263fbc79e8 it=65536 keyLen=64 prf=1.2.840.113549.2.9 authScheme=1.2.840.113549.2.11  [MacData salt=9d6e76bba3299b906a5d89eb9bb0ae4e8f5cbe94 it=1200000]
Q3 store#3 (pw=store-password-3): PBMAC1 pbkdf2Salt=c87204ee509893967e68736a9538b78fed2b21044f85bffb8c18e8263fbc79e8 it=65536 keyLen=64 prf=1.2.840.113549.2.9 authScheme=1.2.840.113549.2.11  [MacData salt=f1a322ac2fd22cb51b4f1df37d3eca8d92ed75e6 it=1200000]
C6 classic victim output key-bag PBE: encData[1.2.840.113549.1.12.1.6 params=[#08eac65af8a527db5c725d83d8960ddb99ac767b, 600000]]

Why it matters

The primary path is the ordinary re-key or import flow, where the loaded file came from someone else: an application opens a PFX it was sent, adds an entry, stores under its own password. The output is well formed, verifies under that password (row P3b) and re-loads under it (row P3c), so nothing is forged; what changes is the work protecting it. At iterationCount=1 the derivation runs a single PBKDF2 iteration rather than the 65,536 the default branch sets at :2290, and the salt is a value the sender chose and can repeat across recipients, so one precomputation serves every file written from it. The key bags in the classic output of the same sequence still carry the 600,000-iteration PBE (row C6), which leaves the MAC as the cheap half.

Independently of any loaded file, one PKCS12-PBMAC1 object reuses a single PBKDF2 salt across every store() call, including calls under different passwords (rows Q1 to Q3), because :2294 assigns the generated parameters back into macAlgorithm, so that branch runs once per object rather than once per write.

The classic keystore is milder but has the same root: the salt bytes are regenerated on each write at :2115, but their length is inherited (:1035 feeding new byte[saltLength] at :2113), and so is the MAC digest algorithm, the AlgorithmIdentifier latched at :1032 being the one handed to calculatePbeMac at :2129 and written into the output's DigestInfo at :2131. So the case that matters is a source file declaring a zero-length MAC salt, which gives an output with a zero-length MAC salt at itCount=1 (rows C1 and C3, against the fresh instance at C5). The PBMAC1 path is the outlier of the pair, the salt bytes themselves riding along inside macAlgorithm.

A load that throws java.io.IOException: PKCS12 key store mac invalid - wrong password or corrupted file leaves the same values latched, and the load(null, null) a caller issues afterwards does not clear them (rows F1 to F3; row P4 is the same latch after a load that succeeded), so an application that recovers on the same object writes them into a file of its own. That re-initialisation is not optional: before it, store() and aliases() are refused with java.security.KeyStoreException: Uninitialized keystore. A fresh object is unaffected, and the next successful load replaces the values.

What is already intended

Inheriting the MAC iteration count is intended, so it is not part of the ask: PKCS12PBMAC1KeyStoreSpi.java:1688 and PKCS12KeyStoreSpi.java:1689 both carry // a file loaded from disk keeps its own MAC count, otherwise twice the PBE count, and the 1.86 note on org.bouncycastle.pkcs12.store_it_count (docs/releasenotes.md:251) treats that as correct: it records a long-standing divergence in the legacy Ant jdk13 and jdk14 jars, which were "writing at 1,024 iterations" and "never took the MAC count from a file they had loaded". Left over are the PBKDF2 salt, the key length and the PRF on the PBMAC1 side, and the salt length and the MAC digest algorithm on the classic side, none of which is an iteration count. For PBMAC1 the count that actually sets the work factor is the PBKDF2 one carried inside macAlgorithm, not the MacData one; I have taken the documented intent to cover it as well, so the shape below keeps it, though I would not argue if you saw that differently. For PBMAC1 there is also nothing to round trip on the read side: the derivation from :2282 onwards works entirely from the file's pbkdf2Params, and the MacData count is bounds-checked at :1036 and otherwise unused, which is how the 1.85 note on PKCS12PfxPdu.isMacValid reads it too (docs/releasenotes.md:488: "RFC 9579 sec. 6 leaves the MacData salt and iterations unused, so producers may write arbitrary placeholder values"). The keyLength a file may declare already has a floor from #2431, applied inside calculatePbeMac at :2318.

Question

Of those inherited values, the PBKDF2 salt, key length and PRF on the PBMAC1 side and the salt length and digest algorithm on the classic side, which would you want changed, if any? The shape I had in mind is narrower than resetting macAlgorithm: keep the file's PRF, key length and iteration count, and mint a fresh PBKDF2 salt on every write; keeping the PRF rather than discarding it follows an earlier fix in this area, where a PBMAC1 PRF lost on initialisation from protectionAlgorithm was restored (docs/releasenotes.md:867). Two ways to get there, and I do not know which you would prefer.

1. Inside the branch at :2282: always generate the 32-byte pbSalt, taking the count, key length and PRF from macAlgorithm's existing PBKDF2Params where they are present. That needs a way to exempt a salt a caller set deliberately, :1606 being the only place a caller can ask for one, and a plain new PKCS12StoreParameter(out, pw) does not reach it: its default macAlgorithm is idSHA1 (PKCS12StoreParameter.java:249, and :170 for the builder), so the id_PBMAC1 gate at :1604 does not fire.

2. Keep the load side off the write-side fields: verify from load-side locals at :1041 and leave macAlgorithm, itCount and saltLength (:231, :234, :235) as write-side defaults. Bigger, because calculatePbeMac reads the field at :2282 and :2297 and would have to take the file's AlgorithmIdentifier as an argument, but it also covers the ordering and the failed-load residue, nothing being committed until after the comparison at :1044 and the zero-length-password retry at :1052 to :1060. The same split applies to PKCS12KeyStoreSpi at :1032 to :1035.

For the classic keystore the equivalent is to regenerate the MAC salt at the default length rather than the inherited one; if a floor on the length is preferable, PKCS12Util beside validateIterationCount (:405) and validateMacKeyLength (:374) looks like the place. I am happy to send a patch with a test for whichever parts of that you would take.