Skip to content

Commit db49249

Browse files
committed
Compute the TupleHash element length prefix in long arithmetic and refuse a negative length in left_encode / right_encode, relates to github PR #2462.
1 parent 9c0b4db commit db49249

6 files changed

Lines changed: 158 additions & 4 deletions

File tree

‎CONTRIBUTORS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ We also wish to acknowledge financial and collaborative support from [CISCO](htt
542542
- jmeeder \<https://github.com/jmeeder\> - reporting that RFC 4998 evidence-record generation rejected time stamps from an authority naming the digest with NULL parameters where BC names it with them absent, both of which RFC 5754 requires a receiver to accept (issue #2379).
543543
- rimuln \<https://github.com/rimuln\> - diagnosis and fix for PKCS12 getCertificateAlias returning the alias of an unrelated certificate, tracing it to the alias and certificate enumerations of the keystore's certs table diverging in order once keys() enumerated a copy (issue #2384, PR #2385).
544544
- Yu Bao \<yubao&#064;paypal.com\> - reporting an API gap, on behalf of the PayPal Cyber Security Team, that the high-level OpenPGP message API (OpenPGPMessageProcessor / OpenPGPMessageInputStream) gave a caller no way to bound how far a compressed data packet expands, where the low-level PGPCompressedData it wraps has carried a bounded getDataStream(long) overload all along, and that OpenPGPPolicy exposed no equivalent property to set. Suggesting a protocol whitelist for CRL Distribution Point fetching, which is now the org.bouncycastle.x509.CRLDP_protocols property, and suggesting that an OCSP response was read up to the length the responder declared for itself, now capped by org.bouncycastle.ocsp.max_response_size, and suggesting bounds on the OpenPGP ASCII armor headers, now capped by org.bouncycastle.openpgp.max_armor_header_length and org.bouncycastle.openpgp.max_armor_headers.
545-
- Arpan Sharma \<https://github.com/Arpan0995\> - initial audit of BCPQC provider consistency starting with HQC, which led to the exposure of a number of issues in the JCA provider service interfaces for other BCPQC algorithms. In-depth auditing of PQC signature algorithms in the provider leading to the correction of a number of JCA API compliance issues. Initial implementation of the guard that lets a signature context be set on the composite ML-DSA services before initSign / initVerify, the composite counterpart of the base-engine fix for issue #2396 (issue #2412). Audit of the LMS / HSS stateful private key decoder, establishing that the level count, the one-time index and the persisted tree cache were all accepted without validation (issue #2414); that issue was also the inspiration for the further investigation which found the HSS and XMSS^MT decoders accepting a private key whose declared index disagreed with the traversal state stored beside it, allowing a one-time key to be used twice. Audit of the high-level OpenPGP API (org.bouncycastle.openpgp, org.bouncycastle.openpgp.api) and the bcpg packet layer beneath it, spanning certificate evaluation, message verification, decryption and signature subpacket parsing, which found certificates being used without the certifications RFC 9580 requires of them, malformed but signed content reaching ordinary reader code as unchecked exceptions, a truncated message being read as a clean end of message, and unverified plaintext being released ahead of the integrity check (issues #2417, #2424, #2426). Audit of the recipient side of the CMS RFC 9629 KEMRecipientInfo path, which found three sender-controlled fields leaving methods declared to throw CMSException as unchecked exceptions and the kekLength never being confirmed against the wrap algorithm, and which led to the same translation being added to the neighbouring AuthEnvelopedData constructor and the two streaming parsers (issue #2422). Audit of RSA-PSS parameter handling in the provider, which found Signature.setParameter reporting a rejected trailer field as an unchecked exception and leaving the rejected spec half-applied, so that the parameters reported for a signature were not the ones it had been made with (issue #2421). Audit of the PKCS#12 key store entry-setting boundary, which found setKeyEntry reporting a certificate whose public key the provider cannot resolve as an unchecked exception rather than the KeyStoreException it declares, and which led to the same guard being applied to setCertificateEntry and to the RFC 9579 PBMAC1 store, and to the rejected entry no longer being left behind in the store (issue #2419). Further auditing of the verification and expiration paths of the high-level OpenPGP API, which found a configured algorithm policy not being applied to inline message verification, an expired primary key still offering its subkeys, and a data signature being reported valid past its own expiration time. CMS SignedData and PKIX certification path test coverage for the eighteen Composite ML-DSA parameter sets, which had been reachable through the CMS and cert-path signature algorithm finders with no test exercising either (PR #2437). Identification and fix of the output offset in the AEAD stream cipher data operator that made Grain-128AEAD return corrupted plaintext from a decryption driven in chunks, with the streamed decryption coverage that had been missing (PR #2447). Identification and fix of the RFC 9709 content-encryption algorithm being left wrapped at the CMS recipient key-size, allowed-algorithm and tag-size checks, so that none of them applied to the algorithm the content was encrypted under (PR #2446). Identification and fix of the YubiKey OpenPGP smart-card decryptor factories zeroizing the user PIN array belonging to the KeyPassphraseProvider that supplied it, which left the next private-key operation presenting an all-zero PIN to the card (PR #2444).
545+
- Arpan Sharma \<https://github.com/Arpan0995\> - initial audit of BCPQC provider consistency starting with HQC, which led to the exposure of a number of issues in the JCA provider service interfaces for other BCPQC algorithms. In-depth auditing of PQC signature algorithms in the provider leading to the correction of a number of JCA API compliance issues. Initial implementation of the guard that lets a signature context be set on the composite ML-DSA services before initSign / initVerify, the composite counterpart of the base-engine fix for issue #2396 (issue #2412). Audit of the LMS / HSS stateful private key decoder, establishing that the level count, the one-time index and the persisted tree cache were all accepted without validation (issue #2414); that issue was also the inspiration for the further investigation which found the HSS and XMSS^MT decoders accepting a private key whose declared index disagreed with the traversal state stored beside it, allowing a one-time key to be used twice. Audit of the high-level OpenPGP API (org.bouncycastle.openpgp, org.bouncycastle.openpgp.api) and the bcpg packet layer beneath it, spanning certificate evaluation, message verification, decryption and signature subpacket parsing, which found certificates being used without the certifications RFC 9580 requires of them, malformed but signed content reaching ordinary reader code as unchecked exceptions, a truncated message being read as a clean end of message, and unverified plaintext being released ahead of the integrity check (issues #2417, #2424, #2426). Audit of the recipient side of the CMS RFC 9629 KEMRecipientInfo path, which found three sender-controlled fields leaving methods declared to throw CMSException as unchecked exceptions and the kekLength never being confirmed against the wrap algorithm, and which led to the same translation being added to the neighbouring AuthEnvelopedData constructor and the two streaming parsers (issue #2422). Audit of RSA-PSS parameter handling in the provider, which found Signature.setParameter reporting a rejected trailer field as an unchecked exception and leaving the rejected spec half-applied, so that the parameters reported for a signature were not the ones it had been made with (issue #2421). Audit of the PKCS#12 key store entry-setting boundary, which found setKeyEntry reporting a certificate whose public key the provider cannot resolve as an unchecked exception rather than the KeyStoreException it declares, and which led to the same guard being applied to setCertificateEntry and to the RFC 9579 PBMAC1 store, and to the rejected entry no longer being left behind in the store (issue #2419). Further auditing of the verification and expiration paths of the high-level OpenPGP API, which found a configured algorithm policy not being applied to inline message verification, an expired primary key still offering its subkeys, and a data signature being reported valid past its own expiration time. CMS SignedData and PKIX certification path test coverage for the eighteen Composite ML-DSA parameter sets, which had been reachable through the CMS and cert-path signature algorithm finders with no test exercising either (PR #2437). Identification and fix of the output offset in the AEAD stream cipher data operator that made Grain-128AEAD return corrupted plaintext from a decryption driven in chunks, with the streamed decryption coverage that had been missing (PR #2447). Identification and fix of the RFC 9709 content-encryption algorithm being left wrapped at the CMS recipient key-size, allowed-algorithm and tag-size checks, so that none of them applied to the algorithm the content was encrypted under (PR #2446). Identification and fix of the YubiKey OpenPGP smart-card decryptor factories zeroizing the user PIN array belonging to the KeyPassphraseProvider that supplied it, which left the next private-key operation presenting an all-zero PIN to the card (PR #2444). Identification and fix of the TupleHash element length prefix being computed in int arithmetic, which left an element of 2^28 bytes or more either not returning from the length encoding loop or carrying the prefix of an empty element, and of the same loop not returning on a negative output length (PR #2462).
546546
- Flowdalic \<https://github.com/Flowdalic\> - initial implementation of an AnimalSniffer-based Android API-level compatibility check for the Gradle build (PR #336).
547547
- hannesa2 \<https://github.com/hannesa2\> - initial Dependabot configuration for the Gradle and GitHub Actions ecosystems (PR #883).
548548
- vladhuma \<https://github.com/vladhuma\> - initial implementation of server-side OCSP stapling for the BCJSSE provider, on behalf of Thales Group (PR #1740).

‎core/src/main/java/org/bouncycastle/crypto/digests/XofUtils.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ public class XofUtils
66
{
77
public static byte[] leftEncode(long strLen)
88
{
9+
if (strLen < 0)
10+
{
11+
throw new IllegalArgumentException("'strLen' cannot be negative");
12+
}
13+
914
byte n = 1;
1015

1116
long v = strLen;
@@ -28,6 +33,11 @@ public static byte[] leftEncode(long strLen)
2833

2934
public static byte[] rightEncode(long strLen)
3035
{
36+
if (strLen < 0)
37+
{
38+
throw new IllegalArgumentException("'strLen' cannot be negative");
39+
}
40+
3141
byte n = 1;
3242

3343
long v = strLen;
@@ -57,8 +67,8 @@ static byte[] encode(byte[] in, int inOff, int len)
5767
{
5868
if (in.length == len)
5969
{
60-
return Arrays.concatenate(XofUtils.leftEncode(len * 8), in);
70+
return Arrays.concatenate(XofUtils.leftEncode(len * 8L), in);
6171
}
62-
return Arrays.concatenate(XofUtils.leftEncode(len * 8), Arrays.copyOfRange(in, inOff, inOff + len));
72+
return Arrays.concatenate(XofUtils.leftEncode(len * 8L), Arrays.copyOfRange(in, inOff, inOff + len));
6373
}
6474
}

‎core/src/test/java/org/bouncycastle/crypto/test/RegressionTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ public class RegressionTest
188188
new SP80038GTest(),
189189
new TupleHashTest(),
190190
new ParallelHashTest(),
191+
new XofUtilsTest(),
191192
new CryptoServiceConstraintsTest(),
192193
new SymmetricConstraintsTest(),
193194
new DigestConstraintsTest(),
@@ -216,7 +217,8 @@ public class RegressionTest
216217
new SCryptTest(),
217218
new CramerShoupTest(),
218219
new OpenSSHKeyParsingTests(),
219-
new AsymmetricConstraintsTest()
220+
new AsymmetricConstraintsTest(),
221+
new TupleHashLargeInputTest()
220222
};
221223

222224
public static Test[] openBSDBCryptTests =
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package org.bouncycastle.crypto.test;
2+
3+
import org.bouncycastle.crypto.digests.CSHAKEDigest;
4+
import org.bouncycastle.crypto.digests.TupleHash;
5+
import org.bouncycastle.crypto.digests.XofUtils;
6+
import org.bouncycastle.util.Arrays;
7+
import org.bouncycastle.util.Strings;
8+
import org.bouncycastle.util.test.SimpleTest;
9+
10+
/**
11+
* TupleHash over a single element of 2^28 bytes - the smallest element whose length in bits does
12+
* not fit in an int. The expected value is built by driving cSHAKE with the byte string NIST
13+
* Special Publication 800-185 section 5.3 prescribes, whose encode_string prefix (section 2.3.3)
14+
* is computed here in long arithmetic.
15+
* <p>
16+
* Allocates around 512 MiB transiently, so it belongs in RegressionTest.slowTests.
17+
* </p>
18+
*/
19+
public class TupleHashLargeInputTest
20+
extends SimpleTest
21+
{
22+
private static final int ELEMENT_SIZE = 1 << 28;
23+
24+
public String getName()
25+
{
26+
return "TupleHashLargeInput";
27+
}
28+
29+
public void performTest()
30+
throws Exception
31+
{
32+
byte[] data = new byte[ELEMENT_SIZE];
33+
34+
TupleHash tHash = new TupleHash(128, new byte[0]);
35+
36+
tHash.update(data, 0, data.length);
37+
38+
byte[] res = new byte[tHash.getDigestSize()];
39+
40+
tHash.doFinal(res, 0);
41+
42+
CSHAKEDigest cshake = new CSHAKEDigest(128, Strings.toByteArray("TupleHash"), new byte[0]);
43+
44+
byte[] pre = XofUtils.leftEncode(data.length * 8L);
45+
46+
cshake.update(pre, 0, pre.length);
47+
cshake.update(data, 0, data.length);
48+
49+
byte[] post = XofUtils.rightEncode(res.length * 8L);
50+
51+
cshake.update(post, 0, post.length);
52+
53+
byte[] expected = new byte[res.length];
54+
55+
cshake.doFinal(expected, 0, expected.length);
56+
57+
isTrue("large element encoded at the wrong length", Arrays.areEqual(expected, res));
58+
}
59+
60+
public static void main(
61+
String[] args)
62+
{
63+
runTest(new TupleHashLargeInputTest());
64+
}
65+
}
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package org.bouncycastle.crypto.test;
2+
3+
import org.bouncycastle.crypto.digests.XofUtils;
4+
import org.bouncycastle.util.Arrays;
5+
import org.bouncycastle.util.encoders.Hex;
6+
import org.bouncycastle.util.test.SimpleTest;
7+
8+
/**
9+
* left_encode and right_encode from NIST Special Publication 800-185 sections 2.3.1 and 2.3.2,
10+
* over lengths either side of the point where a bit count stops fitting in an int.
11+
*/
12+
public class XofUtilsTest
13+
extends SimpleTest
14+
{
15+
public String getName()
16+
{
17+
return "XofUtils";
18+
}
19+
20+
public void performTest()
21+
throws Exception
22+
{
23+
testLeftEncode();
24+
testRightEncode();
25+
testNegativeLength();
26+
}
27+
28+
private void testLeftEncode()
29+
{
30+
isTrue("left_encode 0", Arrays.areEqual(Hex.decode("0100"), XofUtils.leftEncode(0)));
31+
isTrue("left_encode 8", Arrays.areEqual(Hex.decode("0108"), XofUtils.leftEncode(8)));
32+
isTrue("left_encode 2048", Arrays.areEqual(Hex.decode("020800"), XofUtils.leftEncode(2048)));
33+
34+
// the bit length of the smallest byte string whose bit length overflows an int, and of
35+
// the largest one a byte array can hold
36+
isTrue("left_encode 2^31", Arrays.areEqual(Hex.decode("0480000000"), XofUtils.leftEncode((1L << 28) * 8)));
37+
isTrue("left_encode 2^32", Arrays.areEqual(Hex.decode("050100000000"), XofUtils.leftEncode(1L << 32)));
38+
isTrue("left_encode max", Arrays.areEqual(Hex.decode("0503fffffff8"), XofUtils.leftEncode(Integer.MAX_VALUE * 8L)));
39+
}
40+
41+
private void testRightEncode()
42+
{
43+
isTrue("right_encode 0", Arrays.areEqual(Hex.decode("0001"), XofUtils.rightEncode(0)));
44+
isTrue("right_encode 512", Arrays.areEqual(Hex.decode("020002"), XofUtils.rightEncode(512)));
45+
isTrue("right_encode 2^32", Arrays.areEqual(Hex.decode("010000000005"), XofUtils.rightEncode(1L << 32)));
46+
}
47+
48+
private void testNegativeLength()
49+
{
50+
testException("'strLen' cannot be negative", "IllegalArgumentException", new TestExceptionOperation()
51+
{
52+
@Override
53+
public void operation()
54+
throws Exception
55+
{
56+
XofUtils.leftEncode(-1L);
57+
}
58+
});
59+
60+
testException("'strLen' cannot be negative", "IllegalArgumentException", new TestExceptionOperation()
61+
{
62+
@Override
63+
public void operation()
64+
throws Exception
65+
{
66+
XofUtils.rightEncode(Long.MIN_VALUE);
67+
}
68+
});
69+
}
70+
71+
public static void main(
72+
String[] args)
73+
{
74+
runTest(new XofUtilsTest());
75+
}
76+
}

0 commit comments

Comments
 (0)