Import upstream tests for CVE-2024-0727
BoringSSL is not affected by CVE-2024-0727, but these are good cases to
have in our unit tests.
PKCS#12 is built on top of PKCS#7, a misdesigned, overgeneralized combinator
format. One of the features of PKCS#7 is that the content of every
ContentInfo may be omitted, to indicate that the value is "supplied by other
means". This is commonly used for "detached signatures", where the signature
is supplied separately.
This does not make sense in the context of PKCS#12. But because PKCS#7
combined many unrelated use cases into the same format, so PKCS#12 (and any
other use of PKCS#7) must account for and reject inputs.
Change-Id: I22f19b6c14894003f7515206cd34f968e5503d4a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65747
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
diff --git a/crypto/pkcs8/pkcs12_test.cc b/crypto/pkcs8/pkcs12_test.cc
index 0416ad7..459339d 100644
--- a/crypto/pkcs8/pkcs12_test.cc
+++ b/crypto/pkcs8/pkcs12_test.cc
@@ -656,3 +656,39 @@
ASSERT_EQ(alias, std::string(reinterpret_cast<const char *>(parsed_alias),
static_cast<size_t>(alias_len)));
}
+
+// PKCS#12 is built on top of PKCS#7, a misdesigned, overgeneralized combinator
+// format. One of the features of PKCS#7 is that the content of every
+// ContentInfo may be omitted, to indicate that the value is "supplied by other
+// means". This is commonly used for "detached signatures", where the signature
+// is supplied separately.
+//
+// This does not make sense in the context of PKCS#12. But because PKCS#7
+// combined many unrelated use cases into the same format, so PKCS#12 (and any
+// other use of PKCS#7) must account for and reject inputs.
+TEST(PKCS12Test, MissingContent) {
+ {
+ std::string data = GetTestData("crypto/pkcs8/test/bad1.p12");
+ bssl::UniquePtr<STACK_OF(X509)> certs(sk_X509_new_null());
+ ASSERT_TRUE(certs);
+ EVP_PKEY *key = nullptr;
+ CBS cbs = StringToBytes(data);
+ EXPECT_FALSE(PKCS12_get_key_and_certs(&key, certs.get(), &cbs, ""));
+ }
+ {
+ std::string data = GetTestData("crypto/pkcs8/test/bad2.p12");
+ bssl::UniquePtr<STACK_OF(X509)> certs(sk_X509_new_null());
+ ASSERT_TRUE(certs);
+ EVP_PKEY *key = nullptr;
+ CBS cbs = StringToBytes(data);
+ EXPECT_FALSE(PKCS12_get_key_and_certs(&key, certs.get(), &cbs, ""));
+ }
+ {
+ std::string data = GetTestData("crypto/pkcs8/test/bad3.p12");
+ bssl::UniquePtr<STACK_OF(X509)> certs(sk_X509_new_null());
+ ASSERT_TRUE(certs);
+ EVP_PKEY *key = nullptr;
+ CBS cbs = StringToBytes(data);
+ EXPECT_FALSE(PKCS12_get_key_and_certs(&key, certs.get(), &cbs, ""));
+ }
+}
diff --git a/crypto/pkcs8/test/bad1.p12 b/crypto/pkcs8/test/bad1.p12
new file mode 100644
index 0000000..8f3387c
--- /dev/null
+++ b/crypto/pkcs8/test/bad1.p12
Binary files differ
diff --git a/crypto/pkcs8/test/bad2.p12 b/crypto/pkcs8/test/bad2.p12
new file mode 100644
index 0000000..113cb6f
--- /dev/null
+++ b/crypto/pkcs8/test/bad2.p12
Binary files differ
diff --git a/crypto/pkcs8/test/bad3.p12 b/crypto/pkcs8/test/bad3.p12
new file mode 100644
index 0000000..ef86a1d
--- /dev/null
+++ b/crypto/pkcs8/test/bad3.p12
Binary files differ
diff --git a/sources.cmake b/sources.cmake
index 3652458..2a4a03e 100644
--- a/sources.cmake
+++ b/sources.cmake
@@ -146,6 +146,9 @@
crypto/hpke/hpke_test_vectors.txt
crypto/keccak/keccak_tests.txt
crypto/kyber/kyber_tests.txt
+ crypto/pkcs8/test/bad1.p12
+ crypto/pkcs8/test/bad2.p12
+ crypto/pkcs8/test/bad3.p12
crypto/pkcs8/test/empty_password.p12
crypto/pkcs8/test/empty_password_ber.p12
crypto/pkcs8/test/empty_password_ber_nested.p12
