Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@WillChilds-Klein
WillChilds-Klein committed Nov 15, 2024
1 parent aa9a47c commit 82a0094
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 38 deletions.
3 changes: 1 addition & 2 deletions crypto/pkcs7/bio/cipher.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ static int enc_write(BIO *b, const char *in, int inl) {

static long enc_ctrl(BIO *b, int cmd, long num, void *ptr) {
GUARD_PTR(b);
EVP_CIPHER_CTX **cipher_ctx;
long ret = 1;

BIO_ENC_CTX *ctx = BIO_get_data(b);
Expand Down Expand Up @@ -243,7 +242,7 @@ static long enc_ctrl(BIO *b, int cmd, long num, void *ptr) {
ret = (long)ctx->ok;
break;
case BIO_C_GET_CIPHER_CTX:
cipher_ctx = (EVP_CIPHER_CTX **)ptr;
EVP_CIPHER_CTX **cipher_ctx = ptr;
if (!cipher_ctx) {
ret = 0;
break;
Expand Down
50 changes: 22 additions & 28 deletions crypto/pkcs7/pkcs7.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -692,8 +692,9 @@ static int pkcs7_encode_rinfo(PKCS7_RECIP_INFO *ri, unsigned char *key,
goto err;
}

if (EVP_PKEY_encrypt_init(pctx) <= 0)
if (EVP_PKEY_encrypt_init(pctx) <= 0) {
goto err;
}

if (EVP_PKEY_encrypt(pctx, NULL, &eklen, key, keylen) <= 0) {
goto err;
Expand Down Expand Up @@ -777,7 +778,7 @@ BIO *PKCS7_dataInit(PKCS7 *p7, BIO *bio) {
}


for (int i = 0; i < (int)sk_X509_ALGOR_num(md_sk); i++) {
for (size_t i = 0; i < sk_X509_ALGOR_num(md_sk); i++) {
if (!pkcs7_bio_add_digest(&out, sk_X509_ALGOR_value(md_sk, i))) {
goto err;
}
Expand Down Expand Up @@ -844,25 +845,19 @@ BIO *PKCS7_dataInit(PKCS7 *p7, BIO *bio) {
}

if (bio == NULL) {
OPENSSL_BEGIN_ALLOW_DEPRECATED
bio = BIO_new(BIO_s_mem());
if (bio == NULL) {
goto err;
}
BIO_set_mem_eof_return(bio, /*eof_value*/ 0);
OPENSSL_BEGIN_ALLOW_DEPRECATED
if (!PKCS7_is_detached(p7) && content && content->length > 0) {
OPENSSL_END_ALLOW_DEPRECATED
OPENSSL_END_ALLOW_DEPRECATED
// |bio |needs a copy of |os->data| instead of a pointer because the data
// will be used after |os |has been freed
bio = BIO_new(BIO_s_mem());
if (bio == NULL) {
goto err;
}
BIO_set_mem_eof_return(bio, /*eof_value*/ 0);
if (BIO_write(bio, content->data, content->length) != content->length) {
goto err;
}
} else {
bio = BIO_new(BIO_s_mem());
if (bio == NULL) {
goto err;
}
BIO_set_mem_eof_return(bio, /*eof_value*/ 0);
}
}
if (out) {
Expand All @@ -880,7 +875,7 @@ OPENSSL_END_ALLOW_DEPRECATED

OPENSSL_BEGIN_ALLOW_DEPRECATED
int PKCS7_is_detached(PKCS7 *p7) {
OPENSSL_END_ALLOW_DEPRECATED
OPENSSL_END_ALLOW_DEPRECATED
GUARD_PTR(p7);
if (PKCS7_type_is_signed(p7)) {
return (p7->d.sign == NULL || p7->d.sign->contents->d.ptr == NULL);
Expand All @@ -896,8 +891,7 @@ static BIO *PKCS7_find_digest(EVP_MD_CTX **pmd, BIO *bio, int nid) {
if (bio == NULL) {
return NULL;
}
BIO_get_md_ctx(bio, pmd);
if (*pmd == NULL) {
if (!BIO_get_md_ctx(bio, pmd) || *pmd == NULL) {
OPENSSL_PUT_ERROR(PKCS7, ERR_R_INTERNAL_ERROR);
return NULL;
}
Expand Down Expand Up @@ -998,13 +992,13 @@ int PKCS7_dataFinal(PKCS7 *p7, BIO *bio) {
break;
case NID_pkcs7_signed:
si_sk = p7->d.sign->signer_info;
OPENSSL_BEGIN_ALLOW_DEPRECATED
OPENSSL_BEGIN_ALLOW_DEPRECATED
content = PKCS7_get_octet_string(p7->d.sign->contents);
OPENSSL_END_ALLOW_DEPRECATED
/* If detached data then the content is excluded */
OPENSSL_BEGIN_ALLOW_DEPRECATED
OPENSSL_END_ALLOW_DEPRECATED
// If detached data then the content is excluded
OPENSSL_BEGIN_ALLOW_DEPRECATED
if (PKCS7_type_is_data(p7->d.sign->contents) && PKCS7_is_detached(p7)) {
OPENSSL_END_ALLOW_DEPRECATED
OPENSSL_END_ALLOW_DEPRECATED
ASN1_OCTET_STRING_free(content);
content = NULL;
p7->d.sign->contents->d.data = NULL;
Expand All @@ -1014,9 +1008,9 @@ OPENSSL_END_ALLOW_DEPRECATED
case NID_pkcs7_digest:
content = PKCS7_get_octet_string(p7->d.digest->contents);
// If detached data, then the content is excluded
OPENSSL_BEGIN_ALLOW_DEPRECATED
OPENSSL_BEGIN_ALLOW_DEPRECATED
if (PKCS7_type_is_data(p7->d.digest->contents) && PKCS7_is_detached(p7)) {
OPENSSL_END_ALLOW_DEPRECATED
OPENSSL_END_ALLOW_DEPRECATED
ASN1_OCTET_STRING_free(content);
content = NULL;
p7->d.digest->contents->d.data = NULL;
Expand Down Expand Up @@ -1074,9 +1068,9 @@ OPENSSL_END_ALLOW_DEPRECATED
}
}

OPENSSL_BEGIN_ALLOW_DEPRECATED
OPENSSL_BEGIN_ALLOW_DEPRECATED
if (!PKCS7_is_detached(p7)) {
OPENSSL_END_ALLOW_DEPRECATED
OPENSSL_END_ALLOW_DEPRECATED
if (content == NULL) {
goto err;
}
Expand All @@ -1093,7 +1087,7 @@ OPENSSL_END_ALLOW_DEPRECATED
// Mark the BIO read only then we can use its copy of the data instead of
// making an extra copy.
BIO_set_flags(bio_tmp, BIO_FLAGS_MEM_RDONLY);
BIO_set_mem_eof_return(bio_tmp, /*eof_value*/0);
BIO_set_mem_eof_return(bio_tmp, /*eof_value*/ 0);
ASN1_STRING_set0(content, (unsigned char *)cont, contlen);
}

Expand Down
5 changes: 4 additions & 1 deletion crypto/pkcs7/pkcs7_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,8 @@ static const uint8_t kPKCS7NSS[] = {
0x00, 0x00, 0x00,
};

// Cribbed from
// https://github.com/bcgit/bc-java/blob/main/pkix/src/test/resources/org/bouncycastle/openssl/test/pkcs7.pem
static const uint8_t kPKCS7EnvelopedData[] = {
0x30, 0x82, 0x09, 0xA2, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D,
0x01, 0x07, 0x03, 0xA0, 0x82, 0x09, 0x93, 0x30, 0x82, 0x09, 0x8F, 0x02,
Expand Down Expand Up @@ -1593,6 +1595,7 @@ TEST(PKCS7Test, DataInitFinal) {
ASSERT_TRUE(RSA_generate_key_fips(rsa.get(), 2048, nullptr));
bssl::UniquePtr<EVP_PKEY> rsa_pkey(EVP_PKEY_new());
ASSERT_TRUE(rsa_pkey);

ASSERT_TRUE(EVP_PKEY_set1_RSA(rsa_pkey.get(), rsa.get()));
bio.reset(BIO_new_mem_buf(kPEMCert, strlen(kPEMCert)));
ASSERT_TRUE(bio);
Expand All @@ -1616,7 +1619,7 @@ TEST(PKCS7Test, DataInitFinal) {
ASSERT_TRUE(p7ri);
EXPECT_TRUE(PKCS7_RECIP_INFO_set(p7ri, rsa_x509.get()));
bio.reset(PKCS7_dataInit(p7.get(), nullptr));
EXPECT_TRUE(bio);
ASSERT_TRUE(bio);
EXPECT_TRUE(PKCS7_dataFinal(p7.get(), bio.get()));

bio.reset(nullptr);
Expand Down
14 changes: 7 additions & 7 deletions include/openssl/pkcs7.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,24 +340,24 @@ OPENSSL_EXPORT OPENSSL_DEPRECATED PKCS7 *PKCS7_sign(X509 *sign_cert,
STACK_OF(X509) *certs,
BIO *data, int flags);

// PKCS7_is_detached returns 1 if |p7| has attached content and 0 otherwise.
// PKCS7_is_detached returns 0 if |p7| has attached content and 1 otherwise.
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);

#if defined(__cplusplus)
Expand Down

0 comments on commit 82a0094

Please sign in to comment.