Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify contentinfo content is NULL is handled #1428

Merged
merged 2 commits into from
Feb 2, 2024
Merged

Verify contentinfo content is NULL is handled #1428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f39d68e
Verify contentinfo content is NULL is handled
torben-hansen Jan 31, 2024
8242ae2
Update build files in generated-src
torben-hansen Feb 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions crypto/pkcs8/pkcs12_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,17 @@ static void TestImpl(const char *name, bssl::Span<const uint8_t> der,
}
}

static void TestImplParseFail(const char *name, bssl::Span<const uint8_t> der,
torben-hansen marked this conversation as resolved.
Show resolved Hide resolved
const char *password) {
SCOPED_TRACE(name);
bssl::UniquePtr<STACK_OF(X509)> certs(sk_X509_new_null());
ASSERT_TRUE(certs);

EVP_PKEY *key = nullptr;
CBS pkcs12 = der;
EXPECT_FALSE(PKCS12_get_key_and_certs(&key, certs.get(), &pkcs12, password));
}

static void TestCompat(bssl::Span<const uint8_t> der) {
bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(der.data(), der.size()));
ASSERT_TRUE(bio);
Expand Down Expand Up @@ -140,6 +151,28 @@ TEST(PKCS12Test, TestNoEncryption) {
TestImpl("kNoEncryption", StringToBytes(data), kPassword, nullptr);
}

// The AuthSafe field of the PFX is of type
// ContentInfo https://datatracker.ietf.org/doc/html/rfc7292#appendix-D. It's
// Content field is optional per
// https://datatracker.ietf.org/doc/html/rfc2315#section-7, but we do not
// support that. It must not be absent. Additionally, the Content field of
// AuthSafe contains the AuthenticatedSafe
// https://datatracker.ietf.org/doc/html/rfc7292#section-4.1; a sequence of
// ContentInfo's, where each Content field is Optional, again per RFC2315. We do
// not support this case either, the field cannot be absent.
// Below two test fixtures validates the above. See V1217527752.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would referencing CVE-2024-0727 from OpenSSL provide more context for external consumers?

TEST(PKCS12Test, TestNULLContentInfoRoot) {
// Content in AuthSafe can't be NULL.
std::string data = GetTestData("crypto/pkcs8/test/null_contentinfo_root.p12");
TestImplParseFail("kNoEncryption", StringToBytes(data), nullptr);
}

TEST(PKCS12Test, TestNULLContentInfoChild) {
// Content in ContentInfo from sequence contained in AuthSafe can't be NULL.
std::string data = GetTestData("crypto/pkcs8/test/null_contentinfo_child.p12");
TestImplParseFail("kNoEncryption", StringToBytes(data), nullptr);
}

TEST(PKCS12Test, TestEmptyPassword) {
#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
return; // The MAC check always passes in fuzzer mode.
Expand Down
Binary file added crypto/pkcs8/test/null_contentinfo_child.p12
View file
Open in desktop
Binary file not shown.
Binary file added crypto/pkcs8/test/null_contentinfo_root.p12
View file
Open in desktop
Binary file not shown.
796 changes: 407 additions & 389 deletions generated-src/crypto_test_data.cc
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions sources.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ set(
crypto/pkcs8/test/pbes2_sha256.p12
crypto/pkcs8/test/unicode_password.p12
crypto/pkcs8/test/windows.p12
crypto/pkcs8/test/null_contentinfo_root.p12
crypto/pkcs8/test/null_contentinfo_child.p12
crypto/poly1305/poly1305_tests.txt
crypto/siphash/siphash_tests.txt
crypto/x509/test/basic_constraints_ca.pem
Expand Down
Loading