Skip to content

Commit

Permalink
Finish cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@WillChilds-Klein
WillChilds-Klein committed Nov 29, 2024
1 parent 91896f3 commit cfb3f70
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 77 deletions.
4 changes: 2 additions & 2 deletions crypto/pkcs7/pkcs7.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1497,7 +1497,7 @@ static int pkcs7_x509_add_cert_new(STACK_OF(X509) **p_sk, X509 *cert) {
static int pkcs7_x509_add_certs_new(STACK_OF(X509) **p_sk,
STACK_OF(X509) *certs) {
GUARD_PTR(p_sk);
if (!certs) { // |certs| can be null in the caller
if (!certs) { // |certs| can be null in the caller
return 1;
}
for (size_t i = 0; i < sk_X509_num(certs); i++) {
Expand All @@ -1508,7 +1508,7 @@ static int pkcs7_x509_add_certs_new(STACK_OF(X509) **p_sk,
}

static int pkcs7_signature_verify(BIO *in_bio, PKCS7 *p7, PKCS7_SIGNER_INFO *si,
X509 *signer) {
X509 *signer) {
GUARD_PTR(in_bio);
GUARD_PTR(p7);
GUARD_PTR(si);
Expand Down
6 changes: 3 additions & 3 deletions crypto/pkcs7/pkcs7_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2067,16 +2067,16 @@ TEST(PKCS7Test, TestSigned) {
X509_up_ref(leaf.get());
X509_up_ref(leaf.get());

store.reset(X509_STORE_new());
ASSERT_TRUE(X509_STORE_add_cert(store.get(), root.get()));

bio_in.reset(BIO_new_mem_buf(buf, sizeof(buf)));
p7.reset(PKCS7_sign(leaf.get(), leaf_pkey.get(), nullptr, bio_in.get(),
/*flags*/ 0));
ASSERT_TRUE(p7);
EXPECT_TRUE(PKCS7_type_is_signed(p7.get()));
EXPECT_FALSE(PKCS7_is_detached(p7.get()));

store.reset(X509_STORE_new());
ASSERT_TRUE(X509_STORE_add_cert(store.get(), root.get()));

// attached
certs.reset(sk_X509_new_null());
ASSERT_TRUE(sk_X509_push(certs.get(), leaf.get()));
Expand Down
163 changes: 91 additions & 72 deletions crypto/pkcs7/pkcs7_x509.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,8 @@ PKCS7 *d2i_PKCS7_bio(BIO *bio, PKCS7 **out) {
return NULL;
}
const uint8_t *ptr = data;
// d2i_PKCS7 handles indefinite-length BER properly, so use it
// d2i_PKCS7 handles indefinite-length BER properly, so use it instead of
// ASN1_item_d2i_bio
void *ret = d2i_PKCS7(out, &ptr, len);
OPENSSL_free(data);
return ret;
Expand Down Expand Up @@ -376,62 +377,105 @@ static int write_signer_info(CBB *out, const void *arg) {
return ret;
}

static PKCS7_SIGNER_INFO *PKCS7_add_signature(PKCS7 *p7, X509 *x509, EVP_PKEY *pkey,
const EVP_MD *dgst)
{
static PKCS7_SIGNER_INFO *pkcs7_add_signature(PKCS7 *p7, X509 *x509,
EVP_PKEY *pkey) {
PKCS7_SIGNER_INFO *si = NULL;
const EVP_MD *digest = NULL;

if (dgst == NULL) {
// TODO [childw] OSSL defaults to SHA1 per its docs, which is probably insecure
EVP_PKEY_CTX *pkey_ctx = EVP_PKEY_CTX_new(pkey, /*engine*/ NULL);
if (!pkey_ctx) {
goto err;
}
int ok = EVP_PKEY_CTX_get_signature_md(pkey_ctx, &digest);
EVP_PKEY_CTX_free(pkey_ctx);
if (!ok && (EVP_PKEY_id(pkey) == EVP_PKEY_RSA ||
EVP_PKEY_id(pkey) == EVP_PKEY_DSA)) {
// OpenSSL's docs say that this defaults to SHA1, but appears to actually
// default to SHA256 in 1.1.x and 3.x
// https://linux.die.net/man/3/pkcs7_sign
int def_nid = NID_sha256;
// if (EVP_PKEY_get_default_digest_nid(pkey, &def_nid) <= 0)
// goto err;
dgst = EVP_get_digestbynid(def_nid);
if (dgst == NULL) {
OPENSSL_PUT_ERROR(PKCS7, PKCS7_R_NO_DEFAULT_DIGEST);
goto err;
}
// https://github.com/openssl/openssl/blob/79c98fc6ccab49f02528e06cc046ac61f841a753/crypto/rsa/rsa_ameth.c#L438
digest = EVP_sha256();
} else if (!ok) {
OPENSSL_PUT_ERROR(PKCS7, PKCS7_R_NO_DEFAULT_DIGEST);
goto err;
}

if ((si = PKCS7_SIGNER_INFO_new()) == NULL)
goto err;
if (PKCS7_SIGNER_INFO_set(si, x509, pkey, dgst) <= 0)
goto err;
if (!PKCS7_add_signer(p7, si))
if ((si = PKCS7_SIGNER_INFO_new()) == NULL ||
!PKCS7_SIGNER_INFO_set(si, x509, pkey, digest) ||
!PKCS7_add_signer(p7, si)) {
OPENSSL_PUT_ERROR(PKCS7, PKCS7_R_PKCS7_DATASIGN);
goto err;
}
return si;
err:
PKCS7_SIGNER_INFO_free(si);
err:
PKCS7_SIGNER_INFO_free(si);
return NULL;
}

static PKCS7_SIGNER_INFO *PKCS7_sign_add_signer(PKCS7 *p7, X509 *signcert,
EVP_PKEY *pkey, const EVP_MD *md,
int flags)
{
PKCS7_SIGNER_INFO *si = NULL;
STACK_OF(X509_ALGOR) *smcap = NULL;
static PKCS7_SIGNER_INFO *pkcs7_sign_add_signer(PKCS7 *p7, X509 *signcert,
EVP_PKEY *pkey, int flags) {
PKCS7_SIGNER_INFO *si = NULL;
STACK_OF(X509_ALGOR) *smcap = NULL;

if (!X509_check_private_key(signcert, pkey)) {
OPENSSL_PUT_ERROR(PKCS7, PKCS7_R_PRIVATE_KEY_DOES_NOT_MATCH_CERTIFICATE);
return NULL;
}
if (!X509_check_private_key(signcert, pkey)) {
OPENSSL_PUT_ERROR(PKCS7, PKCS7_R_PRIVATE_KEY_DOES_NOT_MATCH_CERTIFICATE);
return NULL;
}

if ((si = PKCS7_add_signature(p7, signcert, pkey, md)) == NULL) {
OPENSSL_PUT_ERROR(PKCS7, PKCS7_R_PKCS7_ADD_SIGNATURE_ERROR);
return NULL;
}
if ((si = pkcs7_add_signature(p7, signcert, pkey)) == NULL) {
OPENSSL_PUT_ERROR(PKCS7, PKCS7_R_PKCS7_ADD_SIGNATURE_ERROR);
return NULL;
}

if (!(flags & PKCS7_NOCERTS)) {
if (!PKCS7_add_certificate(p7, signcert))
goto err;
}

return si;
err:
sk_X509_ALGOR_pop_free(smcap, X509_ALGOR_free);
return NULL;
}

static int pkcs7_do_general_sign(X509 *sign_cert, EVP_PKEY *pkey,
struct stack_st_X509 *certs, BIO *data,
int flags, PKCS7 **ret) {
if ((*ret = PKCS7_new()) == NULL || !PKCS7_set_type(*ret, NID_pkcs7_signed) ||
!PKCS7_content_new(*ret, NID_pkcs7_data)) {
OPENSSL_PUT_ERROR(PKCS7, ERR_R_PKCS7_LIB);
goto err;
}

if (!(flags & PKCS7_NOCERTS)) {
if (!PKCS7_add_certificate(p7, signcert))
goto err;
if (!pkcs7_sign_add_signer(*ret, sign_cert, pkey, flags)) {
OPENSSL_PUT_ERROR(PKCS7, PKCS7_R_PKCS7_ADD_SIGNER_ERROR);
goto err;
}

for (size_t i = 0; i < sk_X509_num(certs); i++) {
if (!PKCS7_add_certificate(*ret, sk_X509_value(certs, i))) {
OPENSSL_PUT_ERROR(PKCS7, PKCS7_R_PKCS7_ADD_SIGNER_ERROR);
goto err;
}
}

return si;
err:
sk_X509_ALGOR_pop_free(smcap, X509_ALGOR_free);
return NULL;
if ((flags & PKCS7_DETACHED) &&
PKCS7_type_is_data((*ret)->d.sign->contents)) {
ASN1_OCTET_STRING_free((*ret)->d.sign->contents->d.data);
(*ret)->d.sign->contents->d.data = NULL;
}

if (!pkcs7_final(*ret, data)) {
goto err;
}

return 1;
err:
if (*ret) {
PKCS7_free(*ret);
}
*ret = NULL;
return 0;
}

PKCS7 *PKCS7_sign(X509 *sign_cert, EVP_PKEY *pkey, STACK_OF(X509) *certs,
Expand Down Expand Up @@ -471,36 +515,11 @@ PKCS7 *PKCS7_sign(X509 *sign_cert, EVP_PKEY *pkey, STACK_OF(X509) *certs,
goto out;
}
OPENSSL_free(si_data.signature);
} else if (sign_cert != NULL && pkey != NULL &&
data != NULL) {
if ((ret = PKCS7_new()) == NULL ||
!PKCS7_set_type(ret, NID_pkcs7_signed) ||
!PKCS7_content_new(ret, NID_pkcs7_data)) {
OPENSSL_PUT_ERROR(PKCS7, ERR_R_PKCS7_LIB);
goto out;
}

if (pkey && !PKCS7_sign_add_signer(ret, sign_cert, pkey, /*md*/NULL, flags)) {
OPENSSL_PUT_ERROR(PKCS7, PKCS7_R_PKCS7_ADD_SIGNER_ERROR);
goto out;
}

for (size_t i = 0; i < sk_X509_num(certs); i++) {
if (!PKCS7_add_certificate(ret, sk_X509_value(certs, i))) {
goto out;
}
}

if (flags & PKCS7_DETACHED) {
if (PKCS7_type_is_data(ret->d.sign->contents)) {
ASN1_OCTET_STRING_free(ret->d.sign->contents->d.data);
ret->d.sign->contents->d.data = NULL;
}
}

if (pkcs7_final(ret, data)) {
goto out;
}
} else if (sign_cert != NULL && pkey != NULL && data != NULL) {
// pkcs7_do_general_sign will either populate |*ret| on success or set it to
// NULL on failure. goto out label regardless to skip CBB/d2i stuff below.
pkcs7_do_general_sign(sign_cert, pkey, certs, data, flags, &ret);
goto out;
} else {
OPENSSL_PUT_ERROR(PKCS7, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
goto out;
Expand Down

0 comments on commit cfb3f70

Please sign in to comment.