Increase coverage, fix doc comments

crypto/pkcs7/pkcs7.c

@@ -1112,6 +1112,9 @@ static int pkcs7_final(PKCS7 *p7, BIO *data, int flags) {
 
 PKCS7 *PKCS7_encrypt(STACK_OF(X509) *certs, BIO *in, const EVP_CIPHER *cipher,
                      int flags) {
+  GUARD_PTR(certs);
+  GUARD_PTR(in);
+  GUARD_PTR(cipher);
   PKCS7 *p7;
   BIO *p7bio = NULL;
   X509 *x509;
@@ -1328,7 +1331,7 @@ static BIO *pkcs7_data_decode(PKCS7 *p7, EVP_PKEY *pkey, X509 *pcert) {
   OPENSSL_free(dummy_key);
   out = cipher_bio;
 
-  if (data_body->length > 0) {
+  if (data_body && data_body->length > 0) {
     data_bio = BIO_new_mem_buf(data_body->data, data_body->length);
   } else {
     data_bio = BIO_new(BIO_s_mem());
@@ -1363,14 +1366,12 @@ PKCS7_RECIP_INFO *PKCS7_add_recipient(PKCS7 *p7, X509 *x509) {
 }
 
 int PKCS7_decrypt(PKCS7 *p7, EVP_PKEY *pkey, X509 *cert, BIO *data, int flags) {
+  GUARD_PTR(p7);
+  GUARD_PTR(pkey);
+  GUARD_PTR(data);
   BIO *bio = NULL;
   int ret = 0;
 
-  if (p7 == NULL) {
-    OPENSSL_PUT_ERROR(PKCS7, PKCS7_R_INVALID_NULL_POINTER);
-    goto err;
-  }
-
   switch (OBJ_obj2nid(p7->type)) {
     case NID_pkcs7_enveloped:
     case NID_pkcs7_signedAndEnveloped:

crypto/pkcs7/pkcs7_test.cc

@@ -1767,6 +1767,14 @@ 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 "MMA" decrypt with mismatched cert pub key/pkey private key and block
   // cipher used for content encryption
   bio.reset(BIO_new_mem_buf(buf, pt_len));
@@ -1833,4 +1841,15 @@ TEST(PKCS7Test, TestEnveloped) {
   // ...but it produces pseudo-random nonsense
   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()));
+  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()));
+  EXPECT_FALSE(PKCS7_decrypt(p7.get(), rsa_pkey.get(), rsa_x509.get(), bio.get(),
+                    /*flags*/ 0));
 }

include/openssl/pkcs7.h

@@ -344,19 +344,19 @@ OPENSSL_EXPORT OPENSSL_DEPRECATED int PKCS7_is_detached(PKCS7 *p7);
 
 // PKCS7_dataInit creates or initializes a BIO chain for reading data from or
 // writing data to |p7|. If |bio| is non-null, it is added to the chain.
-// Otherwise, a new BIO is allocated to anchor the chain.
+// Otherwise, a new BIO is allocated and returned to anchor the chain.
 OPENSSL_EXPORT OPENSSL_DEPRECATED BIO *PKCS7_dataInit(PKCS7 *p7, BIO *bio);
 
 // PKCS7_dataFinal serializes data written to |bio|'s chain into |p7|. It should
-// only be called on BIO chains created by PKCS7_dataFinal.
+// only be called on BIO chains created by |PKCS7_dataInit|.
 OPENSSL_EXPORT OPENSSL_DEPRECATED int PKCS7_dataFinal(PKCS7 *p7, BIO *bio);
 
-// PKCS7_set_digest sets |p7|'s digest to |md|. It returns 1 on sucess and 0 if
-// |p7| is of the wrong type.
+// PKCS7_set_digest sets |p7|'s digest to |md|. It returns 1 on success and 0 if
+// |p7| is of the wrong content type.
 OPENSSL_EXPORT OPENSSL_DEPRECATED int PKCS7_set_digest(PKCS7 *p7, const EVP_MD *md);
 
-// PKCS7_get_recipient_info returns a point to a stack containing |p7|'s or NULL
-// if none are present.
+// PKCS7_get_recipient_info returns a pointer to a stack containing |p7|'s
+// |PKCS7_RECIP_INFO| or NULL if none are present.
 OPENSSL_EXPORT OPENSSL_DEPRECATED STACK_OF(PKCS7_RECIP_INFO) *PKCS7_get_recipient_info(PKCS7 *p7);
 
 // BIO_get_cipher_status returns 1 if the cipher is in a healthy state or 0