Skip to content

Commit 04e1bf3

Browse files
committed
PKCS12: generate the MAC key derivation salt for every write rather than keeping the one a loaded file carried, floor an inherited PBMAC1 PBKDF2 count at org.bouncycastle.pkcs12.pbkdf2_it_count, and latch nothing from a file that failed its MAC check, relates to github #2450.
1 parent 42773ea commit 04e1bf3

8 files changed

Lines changed: 461 additions & 53 deletions

File tree

‎core/src/main/java/org/bouncycastle/util/Properties.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,22 @@ public class Properties
5555
*/
5656
public static final String PKCS12_STORE_IT_COUNT = "org.bouncycastle.pkcs12.store_it_count";
5757

58+
/**
59+
* The PBKDF2 iteration count the PKCS12 keystore uses for an RFC 9579 PBMAC1 integrity MAC when
60+
* <b>writing</b> a file. Default 65,536.
61+
* <p>
62+
* This is the count that sets the work factor of a PBMAC1 MAC - the MacData count beside it is
63+
* unused ballast under RFC 9579 sec. 6 - and it is also the floor for a count taken from a file
64+
* that has been loaded: a file keeps its own count where that is at least this, and is raised to
65+
* it otherwise, since the file being re-stored is no longer the one its count was chosen for.
66+
* Reading is unaffected: a file's MAC can only be verified with the count it was made with.
67+
* <p>
68+
* A value outside 1 .. 2,500,000 is ignored and the default used, so a mistyped property fails
69+
* towards the default rather than towards a file with no work in its MAC. Read via
70+
* {@link #asInteger(String, int)}.
71+
*/
72+
public static final String PKCS12_PBKDF2_IT_COUNT = "org.bouncycastle.pkcs12.pbkdf2_it_count";
73+
5874
/**
5975
* Maximum time, in seconds, that a downloaded CRL is cached by the internal CrlCache used
6076
* by the CertPath validator and X509RevocationChecker. When set to a positive value, cached

‎core/src/main/jdk1.4/org/bouncycastle/util/Properties.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ public class Properties
2727
public static final String PKCS12_IGNORE_USELESS_PASSWD = "org.bouncycastle.pkcs12.ignore_useless_passwd";
2828
public static final String PKCS12_MAX_IT_COUNT = "org.bouncycastle.pkcs12.max_it_count";
2929
public static final String PKCS12_STORE_IT_COUNT = "org.bouncycastle.pkcs12.store_it_count";
30+
public static final String PKCS12_PBKDF2_IT_COUNT = "org.bouncycastle.pkcs12.pbkdf2_it_count";
3031
public static final String BKS_MAX_IT_COUNT = "org.bouncycastle.bks.max_it_count";
3132
public static final String OPENSSH_MAX_ROUNDS = "org.bouncycastle.openssh.max_rounds";
3233
public static final String X509_CRL_CACHE_TTL = "org.bouncycastle.x509.crl_cache_ttl";

‎docs/releasenotes.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ Date: 2026, TBD
3636
- The CMS RFC 8418 key agreement schemes (dhSinglePass-stdDH-hkdf-sha256/384/512, used with X25519 and X448) derived the key-encryption key with the user keying material in the entityUInfo of the ECC-CMS-SharedInfo but never as the HKDF salt, where RFC 8418 sec. 2.2 requires both - its recipe is salt = ukm, PRK = HKDF-Extract(salt, K), KEK = HKDF-Expand(PRK, DER(ECC-CMS-SharedInfo), SizeInOctets(KEK)). A message carrying a ukm therefore did not interoperate with a conforming implementation in either direction. The ukm is now passed as the salt as well, on both the generating and the receiving side, for those three schemes. A message with a ukm written by 1.86, the only release with RFC 8418 support, is not readable by this release and vice versa; messages without a ukm, and the X9.63-KDF key agreement schemes, are unaffected. The round-trip test now covers both the ukm and no-ukm cases for all six curve and scheme combinations, and checks the key-encryption key against the RFC's own recipe rather than only against BC itself (github #2454).
3737
- A JKS store shorter than the SHA-1 checksum it ends with threw an unchecked ArrayIndexOutOfBoundsException out of KeyStore.load, which declares IOException for a store it cannot read: JKSKeyStoreSpi.validateStream subtracted the digest size from the raw store length without checking it, so the digest update clamped its negative length to zero and the System.arraycopy that lifted the stored checksum out failed on a negative source index. The length is now checked against the checksum plus the 12-byte header before the checksum position is used, and a store too short to carry either is reported as an EOFException. The JKS store is reached through the compatibility probe in AdaptingKeyStoreSpi, so any key store type that probes for it was exposed, and the legacy jdk1.1 and jdk1.4 provider copies carry the same fix (github #2451).
3838
- A custom Argon2BytesGenerator.BlockPool was left to zeroise the blocks it recycled itself, and had no way to know how many blocks to hold: the generator returned each block to the pool with the password-derived data still in it, so only the FixedBlockPool BC ships cleared them, and sizing any other pool meant replicating the internal memory alignment and the block count of the fill step. The generator now clears every block before it goes back, so a pool neither has to clear nor can observe that data, and Argon2BytesGenerator.getBlockCount(memory, lanes) gives the number of blocks a run takes - which the default pool now uses, so it no longer discards and reallocates the four blocks of the fill step on every call. FixedBlockPool drops the two clears it no longer needs, leaving one zeroisation per block per use rather than two, and a generateBytes() that fails part way through now returns and clears the blocks it took, along with its own working buffer, rather than leaving both to the garbage collector (github #2452).
39+
- The PKCS#12 key stores wrote the MAC key-derivation parameters of a file they had loaded into every file they wrote afterwards, under whatever password the caller stored with. For PKCS12-PBMAC1 that carried the loaded file's PBKDF2 salt, iteration count, key length and PRF, because the parameters were minted only when the store held none and were then assigned back, so the branch ran once per store object rather than once per write - which also meant one store reused a single PBKDF2 salt across every write, including writes under different passwords, with no file loaded at all. The classic store inherited the MAC salt length and digest algorithm the same way, so a file declaring a zero-length MAC salt was re-stored with one, and a file it had loaded under RFC 9579 handed on that file's PBKDF2 salt too, both stores reading PBMAC1. The values were also latched before the MAC was verified and were not cleared by the load(null, null) a caller must issue to recover, so a file that failed the check left them behind for the caller's own file. The PBKDF2 salt and the MAC salt are now generated for every write, the MAC salt at no fewer than 8 octets, nothing is latched until the file has verified, and an AlgorithmIdentifier supplied through a PKCS12StoreParameter is still written as it was given. A loaded file's PRF, key length, digest algorithm and MacData iteration count are still kept, the last as before; the PBKDF2 count is kept where it is at least the count being written with and raised to it otherwise, since the file being re-stored is not the one that count was chosen for - RFC 9579's own test vectors ask for 2048. That count is now org.bouncycastle.pkcs12.pbkdf2_it_count, default 65,536, the write-side counterpart for a PBMAC1 MAC of what org.bouncycastle.pkcs12.store_it_count is for the PBE. Reading is unaffected: a file's MAC is verified with the parameters it carries, whatever they are (github #2450).
3940

4041
### 2.1.3 Additional Features and Functionality
4142

‎prov/src/main/java/org/bouncycastle/jcajce/provider/keystore/pkcs12/PKCS12KeyStoreSpi.java‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,14 +1029,13 @@ public void engineLoad(
10291029
// declared IOException by parsing them under the same guard as the MAC itself.
10301030
MacData mData = bag.getMacData();
10311031
DigestInfo dInfo = mData.getMac();
1032-
macAlgorithm = dInfo.getAlgorithmId();
1032+
AlgorithmIdentifier fileMacAlgorithm = dInfo.getAlgorithmId();
10331033
byte[] salt = mData.getSalt();
1034-
itCount = PKCS12Util.validateIterationCount(mData.getIterationCount());
1035-
saltLength = salt.length;
1034+
int fileItCount = PKCS12Util.validateIterationCount(mData.getIterationCount());
10361035

10371036
byte[] data = PKCS12Util.getContentOctets(info);
10381037

1039-
byte[] res = calculatePbeMac(helper, macAlgorithm, salt, itCount, password, false, data);
1038+
byte[] res = calculatePbeMac(helper, fileMacAlgorithm, salt, fileItCount, password, false, data);
10401039
byte[] dig = dInfo.getDigest();
10411040

10421041
if (!Arrays.constantTimeAreEqual(res, dig))
@@ -1048,7 +1047,7 @@ public void engineLoad(
10481047
}
10491048

10501049
// Try with incorrect zero length password
1051-
res = calculatePbeMac(helper, macAlgorithm, salt, itCount, password, true, data);
1050+
res = calculatePbeMac(helper, fileMacAlgorithm, salt, fileItCount, password, true, data);
10521051

10531052
if (!Arrays.constantTimeAreEqual(res, dig))
10541053
{
@@ -1057,6 +1056,13 @@ public void engineLoad(
10571056

10581057
wrongPKCS12Zero = true;
10591058
}
1059+
1060+
// the file has verified: a write may now keep the MAC algorithm, count and salt
1061+
// length it arrived with. The salt bytes are not inherited - they are generated per
1062+
// write - and nothing is latched from a file that failed the check above.
1063+
macAlgorithm = fileMacAlgorithm;
1064+
itCount = fileItCount;
1065+
saltLength = PKCS12Util.getMacSaltLength(salt.length);
10601066
}
10611067
catch (IOException e)
10621068
{
@@ -2126,9 +2132,13 @@ private void doStore(OutputStream stream, char[] password, boolean useDEREncodin
21262132
{
21272133
try
21282134
{
2129-
byte[] res = calculatePbeMac(helper, macAlgorithm, mSalt, macItCount, password, false, data);
2135+
// a PBMAC1 algorithm here came from a file this store loaded - RFC 9579 sec. 7
2136+
// files are read by this store too - and carries that file's PBKDF2 salt with it
2137+
AlgorithmIdentifier writeMacAlgorithm = PKCS12Util.getWriteMacAlgorithm(macAlgorithm, random);
2138+
2139+
byte[] res = calculatePbeMac(helper, writeMacAlgorithm, mSalt, macItCount, password, false, data);
21302140

2131-
DigestInfo dInfo = new DigestInfo(macAlgorithm, res);
2141+
DigestInfo dInfo = new DigestInfo(writeMacAlgorithm, res);
21322142

21332143
mData = new MacData(dInfo, mSalt, macItCount);
21342144
}

‎prov/src/main/java/org/bouncycastle/jcajce/provider/keystore/pkcs12/PKCS12PBMAC1KeyStoreSpi.java‎

Lines changed: 45 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,8 @@ public class PKCS12PBMAC1KeyStoreSpi
229229
private ASN1ObjectIdentifier certAlgorithm;
230230

231231
private AlgorithmIdentifier macAlgorithm = new AlgorithmIdentifier(id_PBMAC1);
232+
// true when macAlgorithm came from a PKCS12StoreParameter rather than from a file we loaded
233+
private boolean callerMacAlgorithm = false;
232234
// The MAC iteration count: taken from a loaded file so that storing it again preserves it,
233235
// and -1 until then, meaning doStore should use the store-time default.
234236
private int itCount = -1;
@@ -1031,14 +1033,13 @@ public void engineLoad(
10311033
// declared IOException by parsing them under the same guard as the MAC itself.
10321034
MacData mData = bag.getMacData();
10331035
DigestInfo dInfo = mData.getMac();
1034-
macAlgorithm = dInfo.getAlgorithmId();
1036+
AlgorithmIdentifier fileMacAlgorithm = dInfo.getAlgorithmId();
10351037
byte[] salt = mData.getSalt();
1036-
itCount = PKCS12Util.validateIterationCount(mData.getIterationCount());
1037-
saltLength = salt.length;
1038+
int fileItCount = PKCS12Util.validateIterationCount(mData.getIterationCount());
10381039

10391040
byte[] data = PKCS12Util.getContentOctets(info);
10401041

1041-
byte[] res = calculatePbeMac(macAlgorithm.getAlgorithm(), salt, itCount, password, false, data);
1042+
byte[] res = calculatePbeMac(fileMacAlgorithm, salt, fileItCount, password, false, data);
10421043
byte[] dig = dInfo.getDigest();
10431044

10441045
if (!Arrays.constantTimeAreEqual(res, dig))
@@ -1050,7 +1051,7 @@ public void engineLoad(
10501051
}
10511052

10521053
// Try with incorrect zero length password
1053-
res = calculatePbeMac(macAlgorithm.getAlgorithm(), salt, itCount, password, true, data);
1054+
res = calculatePbeMac(fileMacAlgorithm, salt, fileItCount, password, true, data);
10541055

10551056
if (!Arrays.constantTimeAreEqual(res, dig))
10561057
{
@@ -1059,6 +1060,14 @@ public void engineLoad(
10591060

10601061
wrongPKCS12Zero = true;
10611062
}
1063+
1064+
// the file has verified: a write may now inherit the shape it arrived in. Neither
1065+
// the PBKDF2 salt inside fileMacAlgorithm nor the MacData salt is inherited with it -
1066+
// both are generated per write - and nothing is latched from a file that failed above.
1067+
macAlgorithm = fileMacAlgorithm;
1068+
callerMacAlgorithm = false;
1069+
itCount = fileItCount;
1070+
saltLength = PKCS12Util.getMacSaltLength(salt.length);
10621071
}
10631072
catch (IOException e)
10641073
{
@@ -1604,6 +1613,7 @@ else if (protParam instanceof KeyStore.PasswordProtection)
16041613
if (bcParam.getMacAlgorithm().getAlgorithm().equals(id_PBMAC1))
16051614
{
16061615
this.macAlgorithm = bcParam.getMacAlgorithm();
1616+
this.callerMacAlgorithm = true;
16071617
// fill the necessary parameters
16081618
PBMAC1Params pbmac1Params = PBMAC1Params.getInstance(this.macAlgorithm.getParameters());
16091619
AlgorithmIdentifier keyDevFunc = pbmac1Params.getKeyDerivationFunc();
@@ -1619,6 +1629,24 @@ else if (protParam instanceof KeyStore.PasswordProtection)
16191629
doStore(bcParam.getOutputStream(), password, bcParam.isForDEREncoding(), bcParam.isOverwriteFriendlyName());
16201630
}
16211631

1632+
/**
1633+
* Return the MAC AlgorithmIdentifier to write a file with.
1634+
* <p>
1635+
* An AlgorithmIdentifier a caller supplied through a {@link PKCS12StoreParameter} is theirs
1636+
* and is returned as it was given; anything else goes through
1637+
* {@link PKCS12Util#getWriteMacAlgorithm(AlgorithmIdentifier, SecureRandom)}, which mints a
1638+
* fresh PBKDF2 salt for every write.
1639+
*/
1640+
private AlgorithmIdentifier getWriteMacAlgorithm()
1641+
{
1642+
if (callerMacAlgorithm)
1643+
{
1644+
return macAlgorithm;
1645+
}
1646+
1647+
return PKCS12Util.getWriteMacAlgorithm(macAlgorithm, random);
1648+
}
1649+
16221650
public void engineStore(OutputStream stream, char[] password)
16231651
throws IOException
16241652
{
@@ -2123,9 +2151,11 @@ private void doStore(OutputStream stream, char[] password, boolean useDEREncodin
21232151
{
21242152
try
21252153
{
2126-
byte[] res = calculatePbeMac(macAlgorithm.getAlgorithm(), mSalt, macItCount, password, false, data);
2154+
AlgorithmIdentifier writeMacAlgorithm = getWriteMacAlgorithm();
21272155

2128-
DigestInfo dInfo = new DigestInfo(macAlgorithm, res);
2156+
byte[] res = calculatePbeMac(writeMacAlgorithm, mSalt, macItCount, password, false, data);
2157+
2158+
DigestInfo dInfo = new DigestInfo(writeMacAlgorithm, res);
21292159

21302160
mData = new MacData(dInfo, mSalt, macItCount);
21312161
}
@@ -2268,32 +2298,24 @@ private Set getUsedCertificateSet()
22682298
return usedSet;
22692299
}
22702300

2301+
/**
2302+
* Calculate the MAC named by macAlgorithm - the whole of it, so this reads no state of its
2303+
* own: on a load that is the AlgorithmIdentifier the file carries, and on a store the one
2304+
* {@link #getWriteMacAlgorithm()} has just built. Nothing here is latched for a later write.
2305+
*/
22712306
private byte[] calculatePbeMac(
2272-
ASN1ObjectIdentifier oid,
2307+
AlgorithmIdentifier macAlgorithm,
22732308
byte[] salt,
22742309
int itCount,
22752310
char[] password,
22762311
boolean wrongPkcs12Zero,
22772312
byte[] data)
22782313
throws Exception
22792314
{
2315+
ASN1ObjectIdentifier oid = macAlgorithm.getAlgorithm();
2316+
22802317
if (PKCSObjectIdentifiers.id_PBMAC1.equals(oid))
22812318
{
2282-
if (macAlgorithm.getParameters() == null)
2283-
{
2284-
byte[] pbSalt = new byte[32];
2285-
helper.createSecureRandom("DEFAULT").nextBytes(pbSalt);
2286-
2287-
// RFC 9579 sec. 5: the derived key SHOULD be the size of the HMAC output, which is 64
2288-
// for the HMAC-SHA-512 auth scheme below. Releases up to 1.86 asked for 256 here; those
2289-
// files still verify, since the length is read back from the file.
2290-
PBKDF2Params pbkdf2Params = new PBKDF2Params(pbSalt, 1 << 16, 64, new AlgorithmIdentifier(PKCSObjectIdentifiers.id_hmacWithSHA256));
2291-
AlgorithmIdentifier keyDevFunc = new AlgorithmIdentifier(PKCSObjectIdentifiers.id_PBKDF2, pbkdf2Params);
2292-
AlgorithmIdentifier authScheme = new AlgorithmIdentifier(id_hmacWithSHA512);
2293-
PBMAC1Params pbmac1Params = new PBMAC1Params(keyDevFunc, authScheme);
2294-
macAlgorithm = new AlgorithmIdentifier(id_PBMAC1, pbmac1Params);
2295-
}
2296-
22972319
PBMAC1Params pbmac1Params = PBMAC1Params.getInstance(macAlgorithm.getParameters());
22982320
if (pbmac1Params == null)
22992321
{

0 commit comments

Comments
 (0)