From: Dr. David von Oheimb Date: Mon, 12 Sep 2022 18:50:28 +0000 (+0200) Subject: CMS_add0_cert: if cert already present, do not throw error but ignore it X-Git-Tag: openssl-3.2.0-alpha1~1231 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=65def9de8088ae39d8f251e0b57f1a0f204daa14;p=thirdparty%2Fopenssl.git CMS_add0_cert: if cert already present, do not throw error but ignore it Also add checks on failing cert/CRL up_ref calls; improve coding style. Reviewed-by: Tomas Mraz Reviewed-by: Paul Dale Reviewed-by: David von Oheimb (Merged from https://github.com/openssl/openssl/pull/19199) --- diff --git a/CHANGES.md b/CHANGES.md index d02feefa6b7..b5381e9847b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -168,6 +168,13 @@ OpenSSL 3.2 *David von Oheimb* + * `CMS_add0_cert()` and `CMS_add1_cert()` no more throw an error + if a certificate to be added is already present. + * `CMS_sign_ex()` and `CMS_sign()` now ignore any duplicate certificates + in their `certs` argument and no longer throw an error for them. + + *David von Oheimb* + * Fixed and extended `util/check-format.pl` for checking adherence to the coding style . The checks are meanwhile more complete and yield fewer false positives. diff --git a/crypto/cms/cms_lib.c b/crypto/cms/cms_lib.c index 2744306959e..a339f471e8f 100644 --- a/crypto/cms/cms_lib.c +++ b/crypto/cms/cms_lib.c @@ -537,9 +537,9 @@ int CMS_add0_cert(CMS_ContentInfo *cms, X509 *cert) for (i = 0; i < sk_CMS_CertificateChoices_num(*pcerts); i++) { cch = sk_CMS_CertificateChoices_value(*pcerts, i); if (cch->type == CMS_CERTCHOICE_CERT) { - if (!X509_cmp(cch->d.certificate, cert)) { - ERR_raise(ERR_LIB_CMS, CMS_R_CERTIFICATE_ALREADY_PRESENT); - return 0; + if (X509_cmp(cch->d.certificate, cert) == 0) { + X509_free(cert); + return 1; /* cert already present */ } } } @@ -553,11 +553,12 @@ int CMS_add0_cert(CMS_ContentInfo *cms, X509 *cert) int CMS_add1_cert(CMS_ContentInfo *cms, X509 *cert) { - int r; - r = CMS_add0_cert(cms, cert); - if (r > 0) - X509_up_ref(cert); - return r; + if (!X509_up_ref(cert)) + return 0; + if (CMS_add0_cert(cms, cert)) + return 1; + X509_free(cert); + return 0; } static STACK_OF(CMS_RevocationInfoChoice) @@ -609,9 +610,9 @@ CMS_RevocationInfoChoice *CMS_add0_RevocationInfoChoice(CMS_ContentInfo *cms) int CMS_add0_crl(CMS_ContentInfo *cms, X509_CRL *crl) { - CMS_RevocationInfoChoice *rch; - rch = CMS_add0_RevocationInfoChoice(cms); - if (!rch) + CMS_RevocationInfoChoice *rch = CMS_add0_RevocationInfoChoice(cms); + + if (rch == NULL) return 0; rch->type = CMS_REVCHOICE_CRL; rch->d.crl = crl; @@ -665,16 +666,15 @@ STACK_OF(X509_CRL) *CMS_get1_crls(CMS_ContentInfo *cms) for (i = 0; i < sk_CMS_RevocationInfoChoice_num(*pcrls); i++) { rch = sk_CMS_RevocationInfoChoice_value(*pcrls, i); if (rch->type == 0) { - if (!crls) { - crls = sk_X509_CRL_new_null(); - if (!crls) + if (crls == NULL) { + if ((crls = sk_X509_CRL_new_null()) == NULL) return NULL; } - if (!sk_X509_CRL_push(crls, rch->d.crl)) { + if (!sk_X509_CRL_push(crls, rch->d.crl) + || !X509_CRL_up_ref(rch->d.crl)) { sk_X509_CRL_pop_free(crls, X509_CRL_free); return NULL; } - X509_CRL_up_ref(rch->d.crl); } } return crls; diff --git a/doc/man3/CMS_add0_cert.pod b/doc/man3/CMS_add0_cert.pod index 37b6551ffb2..eb105d72961 100644 --- a/doc/man3/CMS_add0_cert.pod +++ b/doc/man3/CMS_add0_cert.pod @@ -20,7 +20,13 @@ CMS_add0_crl, CMS_add1_crl, CMS_get1_crls =head1 DESCRIPTION -CMS_add0_cert() and CMS_add1_cert() add certificate I to I. +CMS_add0_cert() and CMS_add1_cert() add certificate I to I +unless it is already present. +This is used by L and L and may be used before +calling L to help chain building in certificate validation. +As the 0 implies, CMS_add0_cert() adds I internally to I +and on success it must not be freed up by the caller. +In contrast, the caller of CMS_add1_cert() must free I. I must be of type signed data or (authenticated) enveloped data. For signed data, such a certificate can be used when signing or verifying to fill in the signer certificate or to provide an extra CA certificate @@ -30,7 +36,8 @@ CMS_get1_certs() returns all certificates in I. CMS_add0_crl() and CMS_add1_crl() add CRL I to I. I must be of type signed data or (authenticated) enveloped data. -For signed data, such a CRL may be used in certificate validation. +For signed data, such a CRL may be used in certificate validation +with L. It may be given both for inclusion when signing a CMS message and when verifying a signed CMS message. @@ -45,13 +52,6 @@ For signed data, certificates and CRLs are added to the I and I fields of SignedData structure. For enveloped data they are added to B. -As the 0 implies, CMS_add0_cert() adds I internally to I and it -must not be freed up after the call as opposed to CMS_add1_cert() where I -must be freed up. - -The same certificate or CRL must not be added to the same cms structure more -than once. - =head1 RETURN VALUES CMS_add0_cert(), CMS_add1_cert() and CMS_add0_crl() and CMS_add1_crl() return @@ -64,9 +64,14 @@ in practice is if the I type is invalid. =head1 SEE ALSO L, -L, +L, L, L, L +=head1 HISTORY + +CMS_add0_cert() and CMS_add1_cert() have been changed in OpenSSL 3.2 +not to throw an error if a certificate to be added is already present. + =head1 COPYRIGHT Copyright 2008-2016 The OpenSSL Project Authors. All Rights Reserved. diff --git a/doc/man3/CMS_sign.pod b/doc/man3/CMS_sign.pod index 0d812756aef..7a0fcbb53c5 100644 --- a/doc/man3/CMS_sign.pod +++ b/doc/man3/CMS_sign.pod @@ -130,6 +130,9 @@ it is supported for embedded data in OpenSSL 1.0.0 and later. The CMS_sign_ex() method was added in OpenSSL 3.0. +Since OpenSSL 3.2, CMS_sign_ex() and CMS_sign() ignore any duplicate +certificates in their I argument and no longer throw an error for them. + =head1 COPYRIGHT Copyright 2008-2020 The OpenSSL Project Authors. All Rights Reserved. diff --git a/doc/man3/CMS_verify.pod b/doc/man3/CMS_verify.pod index d124234ab15..ddd2a8970ce 100644 --- a/doc/man3/CMS_verify.pod +++ b/doc/man3/CMS_verify.pod @@ -40,7 +40,7 @@ it operates on B input in the I argument, it has some additional parameters described next, and on success it returns the verfied content as a memory BIO. The optional I parameter may be used to provide untrusted CA -certificates that may be helpful for chain building in certificate valiation. +certificates that may be helpful for chain building in certificate validation. This list of certificates must not contain duplicates. The optional I parameter may be used to provide extra CRLs. Also the list of CRLs must not contain duplicates. diff --git a/test/cmsapitest.c b/test/cmsapitest.c index ee08c0c9745..6e59b488132 100644 --- a/test/cmsapitest.c +++ b/test/cmsapitest.c @@ -88,6 +88,19 @@ static int test_encrypt_decrypt_aes_256_gcm(void) return test_encrypt_decrypt(EVP_aes_256_gcm()); } +static int test_CMS_add1_cert(void) +{ + CMS_ContentInfo *cms = NULL; + int ret = 0; + + ret = TEST_ptr(cms = CMS_ContentInfo_new()) + && TEST_ptr(CMS_add1_signer(cms, cert, privkey, NULL, 0)) + && TEST_true(CMS_add1_cert(cms, cert)); /* add cert again */ + + CMS_ContentInfo_free(cms); + return ret; +} + static int test_d2i_CMS_bio_NULL(void) { BIO *bio, *content = NULL; @@ -413,6 +426,7 @@ int setup_tests(void) ADD_TEST(test_encrypt_decrypt_aes_128_gcm); ADD_TEST(test_encrypt_decrypt_aes_192_gcm); ADD_TEST(test_encrypt_decrypt_aes_256_gcm); + ADD_TEST(test_CMS_add1_cert); ADD_TEST(test_d2i_CMS_bio_NULL); ADD_ALL_TESTS(test_d2i_CMS_decode, 2); return 1;