From 66f4dc7061c3576c65369bdac85bc8b5320da96b Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Mon, 25 Nov 2024 17:48:00 -0500 Subject: [PATCH] Fix merge issues --- crypto/fipsmodule/rsa/rsa.c | 10 - crypto/pkcs7/bio/cipher_test.cc | 339 -------------------------------- crypto/pkcs7/pkcs7.c | 2 +- crypto/pkcs7/pkcs7_test.cc | 109 +++++++--- crypto/x509/x509_test.cc | 2 - test.c | 25 --- 6 files changed, 88 insertions(+), 399 deletions(-) delete mode 100644 crypto/pkcs7/bio/cipher_test.cc delete mode 100644 test.c diff --git a/crypto/fipsmodule/rsa/rsa.c b/crypto/fipsmodule/rsa/rsa.c index 44355c008f..9372c5da5d 100644 --- a/crypto/fipsmodule/rsa/rsa.c +++ b/crypto/fipsmodule/rsa/rsa.c @@ -969,16 +969,6 @@ int rsa_verify_no_self_test(int hash_nid, const uint8_t *digest, // Check that the computed hash matches the expected hash if (OPENSSL_memcmp(buf, signed_msg, len) != 0) { - printf("BUF 1: "); - for (size_t ii = 0; ii < len; ii++) { - printf("%02X", (unsigned)buf[ii]); - } - printf("\n"); - printf("BUF 2: "); - for (size_t ii = 0; ii < len; ii++) { - printf("%02X", (unsigned)signed_msg[ii]); - } - printf("\n"); OPENSSL_PUT_ERROR(RSA, RSA_R_MISMATCHED_SIGNATURE); goto out; } diff --git a/crypto/pkcs7/bio/cipher_test.cc b/crypto/pkcs7/bio/cipher_test.cc deleted file mode 100644 index 23a9dcdbb7..0000000000 --- a/crypto/pkcs7/bio/cipher_test.cc +++ /dev/null @@ -1,339 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 OR ISC - -#include - -#include -#include -#include -#include -#include - -#include "../../test/test_util.h" -#include "../internal.h" - -// NOTE: need to keep these in sync with cipher BIO source file -#define ENC_MIN_CHUNK_SIZE 256 -#define ENC_BLOCK_SIZE 1024 * 4 - -struct CipherParams { - const char name[40]; - const EVP_CIPHER *(*cipher)(void); -}; - -static const struct CipherParams Ciphers[] = { - {"AES_128_CTR", EVP_aes_128_ctr}, - //{"AES_128_CBC", EVP_aes_128_cbc}, - {"AES_128_OFB", EVP_aes_128_ofb}, - {"AES_256_CTR", EVP_aes_256_ctr}, - {"AES_256_OFB", EVP_aes_256_ofb}, - {"ChaCha20Poly1305", EVP_chacha20_poly1305}, -}; - -class BIOCipherTest : public testing::TestWithParam {}; - -INSTANTIATE_TEST_SUITE_P(PKCS7Test, BIOCipherTest, testing::ValuesIn(Ciphers), - [](const testing::TestParamInfo ¶ms) - -> std::string { return params.param.name; }); - -TEST_P(BIOCipherTest, Cipher) { - uint8_t key[EVP_MAX_KEY_LENGTH]; - uint8_t iv[EVP_MAX_IV_LENGTH]; - uint8_t pt[ENC_BLOCK_SIZE * 2]; - uint8_t pt_decrypted[sizeof(pt)]; - uint8_t ct[sizeof(pt) + EVP_MAX_BLOCK_LENGTH]; // pt + pad - bssl::UniquePtr bio_cipher; - bssl::UniquePtr bio_mem; - std::vector pt_vec, ct_vec, decrypted_pt_vec; - uint8_t buff[2 * sizeof(pt)]; - - const EVP_CIPHER *cipher = GetParam().cipher(); - ASSERT_TRUE(cipher); - - // NOTE: we're using |buff| here to populate some of the variable size pt - // writes - OPENSSL_memset(buff, 'A', sizeof(buff)); - OPENSSL_cleanse(ct, sizeof(ct)); - OPENSSL_cleanse(pt_decrypted, sizeof(pt_decrypted)); - OPENSSL_memset(pt, 'A', sizeof(pt)); - OPENSSL_memset(key, 'B', sizeof(key)); - OPENSSL_memset(iv, 'C', sizeof(iv)); - - // Unsupported or unimplemented CTRL flags and cipher(s) - bio_cipher.reset(BIO_new(BIO_f_cipher())); - ASSERT_TRUE(bio_cipher); - EXPECT_FALSE(BIO_ctrl(bio_cipher.get(), BIO_CTRL_DUP, 0, NULL)); - EXPECT_FALSE(BIO_ctrl(bio_cipher.get(), BIO_CTRL_GET_CALLBACK, 0, NULL)); - EXPECT_FALSE(BIO_ctrl(bio_cipher.get(), BIO_CTRL_SET_CALLBACK, 0, NULL)); - EXPECT_FALSE(BIO_ctrl(bio_cipher.get(), BIO_C_DO_STATE_MACHINE, 0, NULL)); - EXPECT_FALSE(BIO_ctrl(bio_cipher.get(), BIO_C_GET_CIPHER_CTX, 0, NULL)); - EXPECT_FALSE(BIO_ctrl(bio_cipher.get(), BIO_C_SSL_MODE, 0, NULL)); - EXPECT_FALSE(BIO_set_cipher(bio_cipher.get(), EVP_rc4(), key, iv, /*enc*/ 1)); - - // Round-trip using |BIO_write| for encryption with same BIOs, reset between - // encryption/decryption using |BIO_reset|. Fixed size IO. - bio_cipher.reset(BIO_new(BIO_f_cipher())); - ASSERT_TRUE(bio_cipher); - EXPECT_TRUE(BIO_set_cipher(bio_cipher.get(), cipher, key, iv, /*enc*/ 1)); - bio_mem.reset(BIO_new(BIO_s_mem())); - ASSERT_TRUE(bio_mem); - ASSERT_TRUE(BIO_push(bio_cipher.get(), bio_mem.get())); - // Copy |pt| contents to |ct| so we can detect that |ct| gets overwritten - OPENSSL_memcpy(ct, pt, sizeof(pt)); - OPENSSL_cleanse(pt_decrypted, sizeof(pt_decrypted)); - EXPECT_TRUE(BIO_eof(bio_cipher.get())); - EXPECT_EQ(0UL, BIO_wpending(bio_cipher.get())); - EXPECT_TRUE(BIO_write(bio_cipher.get(), pt, sizeof(pt))); - EXPECT_FALSE(BIO_eof(bio_cipher.get())); - EXPECT_EQ(0UL, BIO_wpending(bio_cipher.get())); - EXPECT_TRUE(BIO_flush(bio_cipher.get())); - EXPECT_EQ(0UL, BIO_wpending(bio_cipher.get())); - EXPECT_TRUE(BIO_get_cipher_status(bio_cipher.get())); - int ct_size = BIO_read(bio_mem.get(), ct, sizeof(ct)); - ASSERT_LE((size_t)ct_size, sizeof(ct)); - // first block should now differ - EXPECT_NE(Bytes(pt, EVP_MAX_BLOCK_LENGTH), Bytes(ct, EVP_MAX_BLOCK_LENGTH)); - // Reset both BIOs and decrypt - EXPECT_TRUE(BIO_reset(bio_cipher.get())); // also resets owned |bio_mem| - EXPECT_TRUE(BIO_write(bio_mem.get(), ct, ct_size)); - bio_mem.release(); // |bio_cipher| took ownership - EXPECT_TRUE(BIO_set_cipher(bio_cipher.get(), cipher, key, iv, /*enc*/ 0)); - EXPECT_TRUE(BIO_read(bio_cipher.get(), pt_decrypted, sizeof(pt_decrypted))); - EXPECT_TRUE(BIO_get_cipher_status(bio_cipher.get())); - EXPECT_EQ(Bytes(pt, sizeof(pt)), Bytes(pt_decrypted, sizeof(pt_decrypted))); - - // Test a number of different IO sizes around byte, word, cipher block, - // BIO internal buffer size, and other boundaries. - int io_sizes[] = {1, - 3, - 7, - 8, - 9, - 64, - 923, - 2 * ENC_BLOCK_SIZE, - 15, - 16, - 17, - 31, - 32, - 33, - 511, - 512, - 513, - 1023, - 1024, - 1025, - ENC_MIN_CHUNK_SIZE - 1, - ENC_MIN_CHUNK_SIZE, - ENC_MIN_CHUNK_SIZE + 1, - ENC_BLOCK_SIZE - 1, - ENC_BLOCK_SIZE, - ENC_BLOCK_SIZE + 1}; - - // Round-trip encryption/decryption with successive IOs of different sizes. - bio_cipher.reset(BIO_new(BIO_f_cipher())); - ASSERT_TRUE(bio_cipher); - EXPECT_TRUE(BIO_set_cipher(bio_cipher.get(), cipher, key, iv, /*enc*/ 1)); - bio_mem.reset(BIO_new(BIO_s_mem())); - ASSERT_TRUE(bio_mem); - ASSERT_TRUE(BIO_push(bio_cipher.get(), bio_mem.get())); - for (size_t wsize : io_sizes) { - pt_vec.insert(pt_vec.end(), buff, buff + wsize); - EXPECT_TRUE(BIO_write(bio_cipher.get(), buff, wsize)); - } - EXPECT_TRUE(BIO_flush(bio_cipher.get())); - EXPECT_TRUE(BIO_get_cipher_status(bio_cipher.get())); - while (!BIO_eof(bio_mem.get())) { - size_t bytes_read = BIO_read(bio_mem.get(), buff, sizeof(buff)); - ct_vec.insert(ct_vec.end(), buff, buff + bytes_read); - } - EXPECT_TRUE(BIO_reset(bio_cipher.get())); // also resets owned |bio_mem| - EXPECT_TRUE( - BIO_write(bio_mem.get(), ct_vec.data(), ct_vec.size())); // replace ct - bio_mem.release(); // |bio_cipher| took ownership - EXPECT_TRUE(BIO_set_cipher(bio_cipher.get(), cipher, key, iv, /*enc*/ 0)); - for (size_t rsize : io_sizes) { - EXPECT_TRUE(BIO_read(bio_cipher.get(), buff, rsize)); - decrypted_pt_vec.insert(decrypted_pt_vec.end(), buff, buff + rsize); - } - EXPECT_TRUE(BIO_get_cipher_status(bio_cipher.get())); - EXPECT_EQ(pt_vec.size(), decrypted_pt_vec.size()); - EXPECT_EQ(Bytes(pt_vec.data(), pt_vec.size()), - Bytes(decrypted_pt_vec.data(), decrypted_pt_vec.size())); - - // Induce IO failures in the underlying BIO between subsequent same-size - // operations. The flow of this test is to, for each IO size: - // - // 1. Write/encrypt a chunk of plaintext. - // 2. Disable writes in the underlying BIO and try to write the same plaintext - // chunk again. depending on how large the write size relative to cipher - // BIO's internal buffer size, the write may partially or fully succeed if - // it can be buffered. - // 3. Enable writes in the underlying BIO and complete 2.'s chunk by writing - // any remaining bytes in the chunk - // 4. Flush the cipher BIO to complete the encryption, reset the cipher BIO in - // decrypt mode with the underlying BIO containing the ciphertext. - // 5. Similar to 1., read/decrypt a chunk of ciphertext. - // 6. Similar to 2., disable reads in the underlying BIO. As with 2., this may - // partially or fully succeed depending on how large the read is relative - // to internal buffer sizes. - // 7. Enable reads in the underlying BIO and decrypt the rest of the - // ciphertext. - // 8. Compare original and decrypted plaintexts. - int rsize, wsize; - uint8_t *pos; - for (int io_size : io_sizes) { - pt_vec.clear(); - decrypted_pt_vec.clear(); - bio_cipher.reset(BIO_new(BIO_f_cipher())); - ASSERT_TRUE(bio_cipher); - EXPECT_TRUE(BIO_set_cipher(bio_cipher.get(), cipher, key, iv, /*enc*/ 1)); - bio_mem.reset(BIO_new(BIO_s_mem())); - ASSERT_TRUE(bio_mem); - ASSERT_TRUE(BIO_push(bio_cipher.get(), bio_mem.get())); - // Initial write should fully succeed - pos = &pt[0]; - wsize = BIO_write(bio_cipher.get(), pos, io_size); - if (wsize > 0) { - pt_vec.insert(pt_vec.end(), pos, pos + wsize); - pos += wsize; - } - EXPECT_EQ(io_size, wsize); - // All data should have been flushed - EXPECT_EQ(0UL, BIO_wpending(bio_cipher.get())); - // Set underlying BIO to r/o to induce buffering in |bio_cipher| - auto disable_writes = [](BIO *bio, int oper, const char *argp, size_t len, - int argi, long argl, int bio_ret, - size_t *processed) -> long { - return (oper & BIO_CB_RETURN) || !(oper & BIO_CB_WRITE); - }; - BIO_set_callback_ex(bio_mem.get(), disable_writes); - BIO_set_retry_write(bio_mem.get()); - // Write to |bio_cipher| should still succeed in writing up to - // ENC_BLOCK_SIZE bytes by buffering them - wsize = BIO_write(bio_cipher.get(), buff, io_size); - // First write succeeds due to write buffering up to |ENC_BLOCK_SIZE| bytes - if (io_size >= ENC_BLOCK_SIZE) { - EXPECT_EQ(ENC_BLOCK_SIZE, wsize); - } else { - EXPECT_GT(ENC_BLOCK_SIZE, wsize); - } - if (wsize > 0) { - pt_vec.insert(pt_vec.end(), pos, pos + wsize); - pos += wsize; - } - // Buffer is full, so second write fails - EXPECT_FALSE(BIO_write(bio_cipher.get(), pt, sizeof(pt))); - // Writes still disabled, so flush fails - EXPECT_FALSE(BIO_flush(bio_cipher.get())); - // Now that there's buffered data, |BIO_wpending| should match - EXPECT_EQ((size_t)wsize, BIO_wpending(bio_cipher.get())); - // Re-enable writes - BIO_set_callback_ex(bio_mem.get(), nullptr); - BIO_clear_retry_flags(bio_mem.get()); - if (wsize < io_size) { - const int remaining = io_size - wsize; - pos = buff + wsize; - ASSERT_EQ(remaining, BIO_write(bio_cipher.get(), pos, remaining)); - pt_vec.insert(pt_vec.end(), pos, pos + remaining); - pos += wsize; - } - // Flush should empty the buffered encrypted data - EXPECT_TRUE(BIO_flush(bio_cipher.get())); - EXPECT_EQ(0UL, BIO_wpending(bio_cipher.get())); - EXPECT_TRUE(BIO_get_cipher_status(bio_cipher.get())); - EXPECT_TRUE(BIO_set_cipher(bio_cipher.get(), cipher, key, iv, /*enc*/ 0)); - // Reset BIOs, hydrate ciphertext for decryption - ct_vec.clear(); - while ((rsize = BIO_read(bio_mem.get(), buff, io_size)) > 0) { - ct_vec.insert(ct_vec.end(), buff, buff + rsize); - } - EXPECT_TRUE(BIO_reset(bio_cipher.get())); // also resets owned |bio_mem| - ASSERT_EQ((int)ct_vec.size(), BIO_write(bio_mem.get(), ct_vec.data(), - ct_vec.size())); // replace ct - EXPECT_LE(pt_vec.size(), BIO_pending(bio_cipher.get())); - // First read should fully succeed - rsize = BIO_read(bio_cipher.get(), buff, io_size); - ASSERT_EQ(io_size, rsize); - decrypted_pt_vec.insert(decrypted_pt_vec.end(), buff, buff + rsize); - // Disable reads from underlying BIO - auto disable_reads = [](BIO *bio, int oper, const char *argp, size_t len, - int argi, long argl, int bio_ret, - size_t *processed) -> long { - return (oper & BIO_CB_RETURN) || !(oper & BIO_CB_READ); - }; - BIO_set_callback_ex(bio_mem.get(), disable_reads); - // Set retry flags so |cipher_bio| doesn't give up when the read fails - BIO_set_retry_read(bio_mem.get()); - rsize = BIO_read(bio_cipher.get(), buff, io_size); - decrypted_pt_vec.insert(decrypted_pt_vec.end(), buff, buff + rsize); - EXPECT_EQ(0UL, BIO_pending(bio_cipher.get())); - // Re-enable reads from underlying BIO - BIO_set_callback_ex(bio_mem.get(), nullptr); - BIO_clear_retry_flags(bio_mem.get()); - while ((rsize = BIO_read(bio_cipher.get(), buff, io_size)) > 0) { - decrypted_pt_vec.insert(decrypted_pt_vec.end(), buff, buff + rsize); - } - EXPECT_TRUE(BIO_eof(bio_cipher.get())); - EXPECT_EQ(0UL, BIO_pending(bio_cipher.get())); - EXPECT_TRUE(BIO_get_cipher_status(bio_cipher.get())); - EXPECT_EQ(pt_vec.size(), decrypted_pt_vec.size()); - EXPECT_EQ(Bytes(pt_vec.data(), pt_vec.size()), - Bytes(decrypted_pt_vec.data(), decrypted_pt_vec.size())); - bio_mem.release(); // |bio_cipher| took ownership - } -} - -TEST(BIODeprecatedTest, CipherCbc) { - uint8_t key[EVP_MAX_KEY_LENGTH], iv[EVP_MAX_IV_LENGTH], buff[8 * 1024]; - bssl::UniquePtr bio_cipher, bio_mem; - std::vector pt, ct, decrypted; - - const EVP_CIPHER *cipher = EVP_aes_128_cbc(); - - OPENSSL_memset(key, 'X', sizeof(key)); - OPENSSL_memset(iv, 'Y', sizeof(iv)); - for (int i = 0; i < (int)sizeof(buff); i++) { - int n = i % 16; - char c = n < 10 ? '0' + n : 'A' + (n - 10); - buff[i] = c; - } - - // Round-trip using |BIO_write| for encryption with same BIOs, reset between - // encryption/decryption using |BIO_reset|. Fixed size IO. - bio_cipher.reset(BIO_new(BIO_f_cipher())); - BIO_set_cipher(bio_cipher.get(), cipher, key, iv, /*enc*/ 1); - bio_mem.reset(BIO_new(BIO_s_mem())); - BIO_push(bio_cipher.get(), bio_mem.get()); - int total_bytes = 0; - for (int i = 0; i < 1000; i++) { - int n = (rand() % (sizeof(buff) - 1)) + 1; - n = 4096; - ASSERT_TRUE(BIO_write(bio_cipher.get(), buff, n)); - pt.insert(pt.end(), buff, buff + n); - total_bytes += n; - } - EXPECT_TRUE(BIO_flush(bio_cipher.get())); - EXPECT_TRUE(BIO_get_cipher_status(bio_cipher.get())); - int rsize; - while ((rsize = BIO_read(bio_mem.get(), buff, sizeof(buff))) > 0) { - ct.insert(ct.end(), buff, buff + rsize); - } - // only consider first |pt.size()| bytes of |ct|, exclude pad block - EXPECT_NE(Bytes(pt.data(), pt.size()), Bytes(ct.data(), pt.size())); - // Reset both BIOs and decrypt - EXPECT_TRUE(BIO_reset(bio_cipher.get())); // also resets owned |bio_mem| - EXPECT_TRUE(BIO_write(bio_mem.get(), ct.data(), ct.size())); - bio_mem.release(); // |bio_cipher| took ownership - EXPECT_TRUE(BIO_set_cipher(bio_cipher.get(), cipher, key, iv, /*enc*/ 0)); - EXPECT_FALSE(BIO_eof(bio_cipher.get())); - while ((rsize = BIO_read(bio_cipher.get(), buff, sizeof(buff))) > 0) { - decrypted.insert(decrypted.end(), buff, buff + rsize); - } - EXPECT_TRUE(BIO_eof(bio_cipher.get())); - EXPECT_TRUE(BIO_get_cipher_status(bio_cipher.get())); - EXPECT_EQ(Bytes(pt.data(), pt.size()), - Bytes(decrypted.data(), decrypted.size())); - EXPECT_EQ(total_bytes, (int)decrypted.size()); -} diff --git a/crypto/pkcs7/pkcs7.c b/crypto/pkcs7/pkcs7.c index f88b8f4869..a0fd1d7e69 100644 --- a/crypto/pkcs7/pkcs7.c +++ b/crypto/pkcs7/pkcs7.c @@ -1001,7 +1001,7 @@ OPENSSL_END_ALLOW_DEPRECATED continue; } int sign_nid = OBJ_obj2nid(si->digest_alg->algorithm); - bio_tmp = pkcs7_find_digest(&md_ctx, bio_tmp, sign_nid); + bio_tmp = pkcs7_find_digest(&md_ctx, bio, sign_nid); if (bio_tmp == NULL) { goto err; } diff --git a/crypto/pkcs7/pkcs7_test.cc b/crypto/pkcs7/pkcs7_test.cc index 325a546f66..5166769a61 100644 --- a/crypto/pkcs7/pkcs7_test.cc +++ b/crypto/pkcs7/pkcs7_test.cc @@ -26,7 +26,6 @@ #include "../internal.h" #include "../test/test_util.h" - // kPKCS7NSS contains the certificate chain of mail.google.com, as saved by NSS // using the Chrome UI. static const uint8_t kPKCS7NSS[] = { @@ -1442,10 +1441,8 @@ TEST(PKCS7Test, GettersSetters) { EXPECT_TRUE(PKCS7_type_is_signed(p7_dup.get())); p7_der = kPKCS7SignedWithSignerInfo; - PKCS7 *p7_ptr = nullptr; bssl::UniquePtr bio(BIO_new_mem_buf(p7_der, p7_der_len)); - ASSERT_FALSE(d2i_PKCS7_bio(bio.get(), nullptr)); - p7.reset(d2i_PKCS7_bio(bio.get(), &p7_ptr)); + p7.reset(d2i_PKCS7_bio(bio.get(), nullptr)); ASSERT_TRUE(p7); ASSERT_TRUE(PKCS7_type_is_signed(p7.get())); bio.reset(BIO_new(BIO_s_mem())); @@ -1850,9 +1847,14 @@ TEST(PKCS7Test, TestEnveloped) { bssl::UniquePtr bio; bssl::UniquePtr certs; bssl::UniquePtr rsa_x509; - uint8_t buf[64], decrypted[sizeof(buf)]; + const size_t pt_len = 64; + // NOTE: we make |buf| larger than |pt_len| in case padding gets added. + // without the extra room, we sometimes overflow into the next variable on the + // stack. + uint8_t buf[pt_len + EVP_MAX_BLOCK_LENGTH], decrypted[pt_len]; - OPENSSL_memset(buf, 'A', sizeof(buf)); + OPENSSL_cleanse(buf, sizeof(buf)); + OPENSSL_memset(buf, 'A', pt_len); // parse a cert for use with recipient infos bssl::UniquePtr rsa(RSA_new()); @@ -1872,7 +1874,8 @@ TEST(PKCS7Test, TestEnveloped) { ASSERT_TRUE(X509_set_pubkey(rsa_x509.get(), rsa_pkey.get())); X509_up_ref(rsa_x509.get()); - bio.reset(BIO_new_mem_buf(buf, sizeof(buf))); + // standard success case + bio.reset(BIO_new_mem_buf(buf, pt_len)); p7.reset( PKCS7_encrypt(certs.get(), bio.get(), EVP_aes_128_cbc(), /*flags*/ 0)); EXPECT_TRUE(p7); @@ -1884,10 +1887,10 @@ TEST(PKCS7Test, TestEnveloped) { OPENSSL_cleanse(decrypted, sizeof(decrypted)); ASSERT_EQ((int)sizeof(decrypted), BIO_read(bio.get(), decrypted, sizeof(decrypted))); - EXPECT_EQ(Bytes(buf, sizeof(buf)), Bytes(decrypted, sizeof(decrypted))); + EXPECT_EQ(Bytes(buf, pt_len), Bytes(decrypted, sizeof(decrypted))); // no certs provided for decryption - bio.reset(BIO_new_mem_buf(buf, sizeof(buf))); + bio.reset(BIO_new_mem_buf(buf, pt_len)); p7.reset( PKCS7_encrypt(certs.get(), bio.get(), EVP_aes_128_cbc(), /*flags*/ 0)); EXPECT_TRUE(p7); @@ -1900,7 +1903,7 @@ TEST(PKCS7Test, TestEnveloped) { OPENSSL_cleanse(decrypted, sizeof(decrypted)); ASSERT_EQ((int)sizeof(decrypted), BIO_read(bio.get(), decrypted, sizeof(decrypted))); - EXPECT_EQ(Bytes(buf, sizeof(buf)), Bytes(decrypted, sizeof(decrypted))); + EXPECT_EQ(Bytes(buf, pt_len), Bytes(decrypted, sizeof(decrypted))); // empty plaintext bio.reset(BIO_new(BIO_s_mem())); @@ -1917,15 +1920,40 @@ TEST(PKCS7Test, TestEnveloped) { EXPECT_EQ(0, BIO_read(bio.get(), decrypted, sizeof(decrypted))); EXPECT_FALSE(BIO_should_retry(bio.get())); + // unsupported content type, with and without content + p7.reset(PKCS7_new()); + ASSERT_TRUE(p7); + ASSERT_TRUE(PKCS7_set_type(p7.get(), NID_pkcs7_signed)); + EXPECT_FALSE(PKCS7_decrypt(p7.get(), rsa_pkey.get(), nullptr, bio.get(), 0)); + ASSERT_TRUE(PKCS7_content_new(p7.get(), NID_pkcs7_data)); + EXPECT_FALSE(PKCS7_decrypt(p7.get(), rsa_pkey.get(), nullptr, bio.get(), 0)); + + // test multiple recipients using the same recipient twice. elide |cert| to + // exercise iterative decryption attempt behavior with multiple (2) successful + // decryptions. + sk_X509_push(certs.get(), rsa_x509.get()); + bio.reset(BIO_new_mem_buf(buf, pt_len)); + p7.reset( + PKCS7_encrypt(certs.get(), bio.get(), EVP_aes_128_cbc(), /*flags*/ 0)); + ASSERT_TRUE(p7); + bio.reset(BIO_new(BIO_s_mem())); + // set |rsa_pkey| back to original RSA key + ASSERT_TRUE(EVP_PKEY_set1_RSA(rsa_pkey.get(), rsa.get())); + EXPECT_TRUE(PKCS7_decrypt(p7.get(), rsa_pkey.get(), /*cert*/ nullptr, + bio.get(), + /*flags*/ 0)); + ASSERT_TRUE(sk_X509_pop(certs.get())); + ASSERT_EQ(1LU, sk_X509_num(certs.get())); + // test "MMA" decrypt with mismatched cert pub key/pkey private key and block // cipher used for content encryption - bio.reset(BIO_new_mem_buf(buf, sizeof(buf))); + bio.reset(BIO_new_mem_buf(buf, pt_len)); p7.reset( PKCS7_encrypt(certs.get(), bio.get(), EVP_aes_128_cbc(), /*flags*/ 0)); EXPECT_TRUE(p7); EXPECT_TRUE(PKCS7_type_is_enveloped(p7.get())); bio.reset(BIO_new(BIO_s_mem())); - // set newm RSA key, cert pub key and PKEY private key now mismatch + // set new RSA key, cert pub key and PKEY private key now mismatch rsa.reset(RSA_new()); ASSERT_TRUE(RSA_generate_key_fips(rsa.get(), 2048, nullptr)); ASSERT_TRUE(EVP_PKEY_set1_RSA(rsa_pkey.get(), rsa.get())); @@ -1935,20 +1963,44 @@ TEST(PKCS7Test, TestEnveloped) { // to the output |bio|. The cipher ends up in an unhealthy state due to bad // padding (what should be the final pad block is now just random bytes), so // the overall |PKCS7_decrypt| operation fails. - EXPECT_FALSE(PKCS7_decrypt(p7.get(), rsa_pkey.get(), /*certs*/ nullptr, - bio.get(), - /*flags*/ 0)); - EXPECT_EQ(sizeof(decrypted), BIO_pending(bio.get())); + int decrypt_ok = + PKCS7_decrypt(p7.get(), rsa_pkey.get(), /*certs*/ nullptr, bio.get(), + /*flags*/ 0); + EXPECT_LE(sizeof(decrypted), BIO_pending(bio.get())); OPENSSL_cleanse(decrypted, sizeof(decrypted)); - ASSERT_EQ((int)sizeof(decrypted), - BIO_read(bio.get(), decrypted, sizeof(decrypted))); - EXPECT_NE(Bytes(buf, sizeof(buf)), Bytes(decrypted, sizeof(decrypted))); - EXPECT_EQ(CIPHER_R_BAD_DECRYPT, ERR_GET_REASON(ERR_peek_error())); + // There's a fun edge case here for block ciphers using conventional PKCS#7 + // padding. In this padding scheme, the last byte of the padded plaintext + // determines how many bytes of padding have been appended and must be + // stripped, A random MMA-defense-garbled padded plaintext with last byte of + // 0x01 will trick the EVP API into thinking that byte is a valid padding + // byte, so it (and only it) will be stripped. This leaves the other + // block_size-1 bytes of the padding block in place, resulting in a larger + // "decrypted plaintext" than anticipated. However, this doesn't only apply to + // one byte of padding. With probability 16^-2, it applies to pad 0x02 0x02 + // and so on with increasingly small probabilities. So, we give slack up to + // 16^-4 which means this test will erroneously fail 0.001526% of the time in + // expectation. Ideally we'd find a way to access the padded plaintext and + // account for this deterministically by checking the random "padding" and + // adusting accordingly. + int max_decrypt = + sizeof(decrypted) + EVP_CIPHER_block_size(EVP_aes_128_cbc()); + int decrypted_len = BIO_read(bio.get(), decrypted, max_decrypt); + if (decrypted_len > (int)pt_len) { + EXPECT_LT(max_decrypt - 4, decrypted_len); + EXPECT_TRUE(decrypt_ok); + EXPECT_FALSE(ERR_GET_REASON(ERR_peek_error())); + } else { + EXPECT_EQ((int)sizeof(decrypted), decrypted_len); + EXPECT_FALSE(decrypt_ok); + EXPECT_EQ(CIPHER_R_BAD_DECRYPT, ERR_GET_REASON(ERR_peek_error())); + } + // Of course, plaintext shouldn't equal decrypted in any case here + EXPECT_NE(Bytes(buf, pt_len), Bytes(decrypted, sizeof(decrypted))); // test "MMA" decrypt as above, but with stream cipher. stream cipher has no // padding, so content encryption should "succeed" but return nonsense because // the content decryption key is just randomly generated bytes. - bio.reset(BIO_new_mem_buf(buf, sizeof(buf))); + bio.reset(BIO_new_mem_buf(buf, pt_len)); p7.reset( PKCS7_encrypt(certs.get(), bio.get(), EVP_aes_128_ctr(), /*flags*/ 0)); EXPECT_TRUE(p7); @@ -1963,8 +2015,21 @@ TEST(PKCS7Test, TestEnveloped) { ASSERT_EQ((int)sizeof(decrypted), BIO_read(bio.get(), decrypted, sizeof(decrypted))); // ...but it produces pseudo-random nonsense - EXPECT_NE(Bytes(buf, sizeof(buf)), Bytes(decrypted, sizeof(decrypted))); + EXPECT_NE(Bytes(buf, pt_len), Bytes(decrypted, sizeof(decrypted))); EXPECT_FALSE(ERR_GET_REASON(ERR_peek_error())); + + // mismatched cert + pkey on decrypt + bio.reset(BIO_new_mem_buf(buf, pt_len)); + p7.reset( + PKCS7_encrypt(certs.get(), bio.get(), EVP_aes_128_cbc(), /*flags*/ 0)); + bio.reset(BIO_new(BIO_s_mem())); + bssl::UniquePtr rsa2(RSA_new()); + ASSERT_TRUE(RSA_generate_key_fips(rsa2.get(), 2048, nullptr)); + ASSERT_TRUE(EVP_PKEY_set1_RSA(rsa_pkey.get(), rsa2.get())); + EXPECT_FALSE(PKCS7_decrypt(p7.get(), rsa_pkey.get(), rsa_x509.get(), + bio.get(), + /*flags*/ 0)); + EXPECT_EQ(X509_R_KEY_VALUES_MISMATCH, ERR_GET_REASON(ERR_peek_error())); } TEST(PKCS7Test, TestSigned) { diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc index ac29b76f9d..5d292556ec 100644 --- a/crypto/x509/x509_test.cc +++ b/crypto/x509/x509_test.cc @@ -5212,7 +5212,6 @@ soBsxWI= )"; TEST(X509Test, BER) { - GTEST_SKIP(); // Constructed strings are forbidden in DER. EXPECT_FALSE(CertFromPEM(kConstructedBitString)); EXPECT_FALSE(CertFromPEM(kConstructedOctetString)); @@ -7361,7 +7360,6 @@ TEST(X509Test, X509_OBJECT_heap) { } TEST(X509Test, NameAttributeValues) { - GTEST_SKIP(); // 1.2.840.113554.4.1.72585.0. We use an unrecognized OID because using an // arbitrary ASN.1 type as the value for commonName is invalid. Our parser // does not check this, but best to avoid unrelated errors in tests, in case diff --git a/test.c b/test.c deleted file mode 100644 index dfae13bce6..0000000000 --- a/test.c +++ /dev/null @@ -1,25 +0,0 @@ -#include -#include -#include -#include -#include -#include - -// run with gcc test.c -g3 -l:libcrypto.a -L/usr/lib/x86_64-linux-gnu/ -o test && gdb ./test - -int main(int *argc, char **argv) { - PKCS7 *p7 = PKCS7_new(); - BIO *bio = BIO_new(BIO_s_mem()); - - PKCS7_set_type(p7, NID_pkcs7_digest); - PKCS7_set_digest(p7, EVP_sha256()); - - BIO_set_md(bio, EVP_sha256()); - BIO *chain = PKCS7_dataInit(p7, bio); - printf("INIT: %p\n", chain); - BIO_set_md(chain, EVP_sha256()); - int ret = PKCS7_dataFinal(p7, chain); - printf("FINAL: %d\n", ret); - - return 0; -}