From 89d18c7a880fe43a5cebe39837178c9e6161c3cb Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 25 Jan 2024 14:53:57 -0500 Subject: [PATCH 1/2] 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 Commit-Queue: Bob Beck Commit-Queue: David Benjamin Reviewed-by: Bob Beck --- crypto/pkcs8/pkcs12_test.cc | 36 ++++++++++++++++++++++++++++++++++++ crypto/pkcs8/test/bad1.p12 | Bin 0 -> 85 bytes crypto/pkcs8/test/bad2.p12 | Bin 0 -> 104 bytes crypto/pkcs8/test/bad3.p12 | Bin 0 -> 104 bytes sources.cmake | 3 +++ 5 files changed, 39 insertions(+) create mode 100644 crypto/pkcs8/test/bad1.p12 create mode 100644 crypto/pkcs8/test/bad2.p12 create mode 100644 crypto/pkcs8/test/bad3.p12 diff --git a/crypto/pkcs8/pkcs12_test.cc b/crypto/pkcs8/pkcs12_test.cc index 0416ad7b1..459339d75 100644 --- a/crypto/pkcs8/pkcs12_test.cc +++ b/crypto/pkcs8/pkcs12_test.cc @@ -656,3 +656,39 @@ TEST(PKCS12Test, CreateWithAlias) { ASSERT_EQ(alias, std::string(reinterpret_cast(parsed_alias), static_cast(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 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 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 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 0000000000000000000000000000000000000000..8f3387c7e356e4aa374729f3f3939343557b9c09 GIT binary patch literal 85 zcmV-b0IL5mQvv}4Fbf6=Duzgg_YDCD0Wd)@F)$4V31Egu0c8UO0s#d81R(r{)waiY rfR=Py6XX#<$m7-wj)xrauuD`}hF=Ng9=0`~S~)@=J%OiUaM0Oze6 AD*ylh literal 0 HcmV?d00001 diff --git a/crypto/pkcs8/test/bad3.p12 b/crypto/pkcs8/test/bad3.p12 new file mode 100644 index 0000000000000000000000000000000000000000..ef86a1d86fb0bc09471ca2596d82e7d521d973a4 GIT binary patch literal 104 zcmXp=V`5}BkYnT2YV&CO&dbQoxImDF-+oA$5$MVJL*60=F*5iN*C_e&wD%dwCM*q{=+OBX|Z+F7XSHN#>B+I003La BAqM~e literal 0 HcmV?d00001 diff --git a/sources.cmake b/sources.cmake index 365245867..2a4a03eac 100644 --- a/sources.cmake +++ b/sources.cmake @@ -146,6 +146,9 @@ set( 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 From 7f45053d42ae0b5f4d5d96fa471d671c6d1462e9 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sat, 20 Jan 2024 09:54:08 -0500 Subject: [PATCH 2/2] Unexport uint32_t-based DES APIs Per the header, these symbols were private and only exported for decrepit. But decrepit can include internal headers. Change-Id: I1155f4b98252004b80a53efb0a6009400a6c59ac Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65687 Auto-Submit: David Benjamin Commit-Queue: Bob Beck Reviewed-by: Bob Beck --- crypto/des/des.c | 38 ++++++++++++++++++++------------------ crypto/des/internal.h | 14 ++++++++++++++ include/openssl/des.h | 13 ------------- 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/crypto/des/des.c b/crypto/des/des.c index a608accda..9e5c97f41 100644 --- a/crypto/des/des.c +++ b/crypto/des/des.c @@ -378,7 +378,8 @@ void DES_set_odd_parity(DES_cblock *key) { } } -static void DES_encrypt1(uint32_t *data, const DES_key_schedule *ks, int enc) { +static void DES_encrypt1(uint32_t data[2], const DES_key_schedule *ks, + int enc) { uint32_t l, r, t, u; r = data[0]; @@ -442,7 +443,8 @@ static void DES_encrypt1(uint32_t *data, const DES_key_schedule *ks, int enc) { data[1] = r; } -static void DES_encrypt2(uint32_t *data, const DES_key_schedule *ks, int enc) { +static void DES_encrypt2(uint32_t data[2], const DES_key_schedule *ks, + int enc) { uint32_t l, r, t, u; r = data[0]; @@ -499,7 +501,7 @@ static void DES_encrypt2(uint32_t *data, const DES_key_schedule *ks, int enc) { data[1] = CRYPTO_rotr_u32(r, 3); } -void DES_encrypt3(uint32_t *data, const DES_key_schedule *ks1, +void DES_encrypt3(uint32_t data[2], const DES_key_schedule *ks1, const DES_key_schedule *ks2, const DES_key_schedule *ks3) { uint32_t l, r; @@ -508,9 +510,9 @@ void DES_encrypt3(uint32_t *data, const DES_key_schedule *ks1, IP(l, r); data[0] = l; data[1] = r; - DES_encrypt2((uint32_t *)data, ks1, DES_ENCRYPT); - DES_encrypt2((uint32_t *)data, ks2, DES_DECRYPT); - DES_encrypt2((uint32_t *)data, ks3, DES_ENCRYPT); + DES_encrypt2(data, ks1, DES_ENCRYPT); + DES_encrypt2(data, ks2, DES_DECRYPT); + DES_encrypt2(data, ks3, DES_ENCRYPT); l = data[0]; r = data[1]; FP(r, l); @@ -518,7 +520,7 @@ void DES_encrypt3(uint32_t *data, const DES_key_schedule *ks1, data[1] = r; } -void DES_decrypt3(uint32_t *data, const DES_key_schedule *ks1, +void DES_decrypt3(uint32_t data[2], const DES_key_schedule *ks1, const DES_key_schedule *ks2, const DES_key_schedule *ks3) { uint32_t l, r; @@ -527,9 +529,9 @@ void DES_decrypt3(uint32_t *data, const DES_key_schedule *ks1, IP(l, r); data[0] = l; data[1] = r; - DES_encrypt2((uint32_t *)data, ks3, DES_DECRYPT); - DES_encrypt2((uint32_t *)data, ks2, DES_ENCRYPT); - DES_encrypt2((uint32_t *)data, ks1, DES_DECRYPT); + DES_encrypt2(data, ks3, DES_DECRYPT); + DES_encrypt2(data, ks2, DES_ENCRYPT); + DES_encrypt2(data, ks1, DES_DECRYPT); l = data[0]; r = data[1]; FP(r, l); @@ -576,7 +578,7 @@ void DES_ncbc_encrypt(const uint8_t *in, uint8_t *out, size_t len, tin[0] = tin0; tin1 ^= tout1; tin[1] = tin1; - DES_encrypt1((uint32_t *)tin, schedule, DES_ENCRYPT); + DES_encrypt1(tin, schedule, DES_ENCRYPT); tout0 = tin[0]; l2c(tout0, out); tout1 = tin[1]; @@ -588,7 +590,7 @@ void DES_ncbc_encrypt(const uint8_t *in, uint8_t *out, size_t len, tin[0] = tin0; tin1 ^= tout1; tin[1] = tin1; - DES_encrypt1((uint32_t *)tin, schedule, DES_ENCRYPT); + DES_encrypt1(tin, schedule, DES_ENCRYPT); tout0 = tin[0]; l2c(tout0, out); tout1 = tin[1]; @@ -605,7 +607,7 @@ void DES_ncbc_encrypt(const uint8_t *in, uint8_t *out, size_t len, tin[0] = tin0; c2l(in, tin1); tin[1] = tin1; - DES_encrypt1((uint32_t *)tin, schedule, DES_DECRYPT); + DES_encrypt1(tin, schedule, DES_DECRYPT); tout0 = tin[0] ^ xor0; tout1 = tin[1] ^ xor1; l2c(tout0, out); @@ -618,7 +620,7 @@ void DES_ncbc_encrypt(const uint8_t *in, uint8_t *out, size_t len, tin[0] = tin0; c2l(in, tin1); tin[1] = tin1; - DES_encrypt1((uint32_t *)tin, schedule, DES_DECRYPT); + DES_encrypt1(tin, schedule, DES_DECRYPT); tout0 = tin[0] ^ xor0; tout1 = tin[1] ^ xor1; l2cn(tout0, tout1, out, len); @@ -678,7 +680,7 @@ void DES_ede3_cbc_encrypt(const uint8_t *in, uint8_t *out, size_t len, tin[0] = tin0; tin[1] = tin1; - DES_encrypt3((uint32_t *)tin, ks1, ks2, ks3); + DES_encrypt3(tin, ks1, ks2, ks3); tout0 = tin[0]; tout1 = tin[1]; @@ -692,7 +694,7 @@ void DES_ede3_cbc_encrypt(const uint8_t *in, uint8_t *out, size_t len, tin[0] = tin0; tin[1] = tin1; - DES_encrypt3((uint32_t *)tin, ks1, ks2, ks3); + DES_encrypt3(tin, ks1, ks2, ks3); tout0 = tin[0]; tout1 = tin[1]; @@ -716,7 +718,7 @@ void DES_ede3_cbc_encrypt(const uint8_t *in, uint8_t *out, size_t len, tin[0] = tin0; tin[1] = tin1; - DES_decrypt3((uint32_t *)tin, ks1, ks2, ks3); + DES_decrypt3(tin, ks1, ks2, ks3); tout0 = tin[0]; tout1 = tin[1]; @@ -736,7 +738,7 @@ void DES_ede3_cbc_encrypt(const uint8_t *in, uint8_t *out, size_t len, tin[0] = tin0; tin[1] = tin1; - DES_decrypt3((uint32_t *)tin, ks1, ks2, ks3); + DES_decrypt3(tin, ks1, ks2, ks3); tout0 = tin[0]; tout1 = tin[1]; diff --git a/crypto/des/internal.h b/crypto/des/internal.h index 2124fd581..d76f485f0 100644 --- a/crypto/des/internal.h +++ b/crypto/des/internal.h @@ -58,6 +58,7 @@ #define OPENSSL_HEADER_DES_INTERNAL_H #include +#include #include "../internal.h" @@ -231,6 +232,19 @@ how to use xors :-) I got it to its final state. #define HALF_ITERATIONS 8 +// Private functions. +// +// These functions are only exported for use in |decrepit|. + +OPENSSL_EXPORT void DES_decrypt3(uint32_t data[2], const DES_key_schedule *ks1, + const DES_key_schedule *ks2, + const DES_key_schedule *ks3); + +OPENSSL_EXPORT void DES_encrypt3(uint32_t data[2], const DES_key_schedule *ks1, + const DES_key_schedule *ks2, + const DES_key_schedule *ks3); + + #if defined(__cplusplus) } // extern C #endif diff --git a/include/openssl/des.h b/include/openssl/des.h index 539b2c524..2a73a418a 100644 --- a/include/openssl/des.h +++ b/include/openssl/des.h @@ -163,19 +163,6 @@ OPENSSL_EXPORT void DES_ede3_cfb_encrypt(const uint8_t *in, uint8_t *out, DES_cblock *ivec, int enc); -// Private functions. -// -// These functions are only exported for use in |decrepit|. - -OPENSSL_EXPORT void DES_decrypt3(uint32_t *data, const DES_key_schedule *ks1, - const DES_key_schedule *ks2, - const DES_key_schedule *ks3); - -OPENSSL_EXPORT void DES_encrypt3(uint32_t *data, const DES_key_schedule *ks1, - const DES_key_schedule *ks2, - const DES_key_schedule *ks3); - - #if defined(__cplusplus) } // extern C #endif