diff options
author | Dr. David von Oheimb <David.von.Oheimb@siemens.com> | 2022-09-12 20:50:28 +0200 |
---|---|---|
committer | Dr. David von Oheimb <dev@ddvo.net> | 2023-02-24 08:49:26 +0100 |
commit | 65def9de8088ae39d8f251e0b57f1a0f204daa14 (patch) | |
tree | 9b4236604d7d4ed956ec0f18633ed65c5cba5fd8 | |
parent | 6f9e531003fd736e8e96d9a1a57f7763da9722b8 (diff) |
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 <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/19199)
-rw-r--r-- | CHANGES.md | 7 | ||||
-rw-r--r-- | crypto/cms/cms_lib.c | 32 | ||||
-rw-r--r-- | doc/man3/CMS_add0_cert.pod | 25 | ||||
-rw-r--r-- | doc/man3/CMS_sign.pod | 3 | ||||
-rw-r--r-- | doc/man3/CMS_verify.pod | 2 | ||||
-rw-r--r-- | test/cmsapitest.c | 14 |
6 files changed, 56 insertions, 27 deletions
diff --git a/CHANGES.md b/CHANGES.md index d02feefa6b..b5381e9847 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 <https://www.openssl.org/policies/technical/coding-style.html>. 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 2744306959..a339f471e8 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 37b6551ffb..eb105d7296 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<cert> to I<cms>. +CMS_add0_cert() and CMS_add1_cert() add certificate I<cert> to I<cms> +unless it is already present. +This is used by L<CMS_sign_ex()> and L<CMS_sign()> and may be used before +calling L<CMS_verify()> to help chain building in certificate validation. +As the 0 implies, CMS_add0_cert() adds I<cert> internally to I<cms> +and on success it must not be freed up by the caller. +In contrast, the caller of CMS_add1_cert() must free I<cert>. I<cms> 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>. CMS_add0_crl() and CMS_add1_crl() add CRL I<crl> to I<cms>. I<cms> 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<CMS_verify()>. 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<certificates> and I<crls> fields of SignedData structure. For enveloped data they are added to B<OriginatorInfo>. -As the 0 implies, CMS_add0_cert() adds I<cert> internally to I<cms> and it -must not be freed up after the call as opposed to CMS_add1_cert() where I<cert> -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<cms> type is invalid. =head1 SEE ALSO L<ERR_get_error(3)>, -L<CMS_sign(3)>, +L<CMS_sign(3)>, L<CMS_sign_ex(3)>, L<CMS_verify(3)>, L<CMS_encrypt(3)> +=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 0d812756ae..7a0fcbb53c 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<certs> 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 d124234ab1..ddd2a8970c 100644 --- a/doc/man3/CMS_verify.pod +++ b/doc/man3/CMS_verify.pod @@ -40,7 +40,7 @@ it operates on B<CMS SignedData> input in the I<sd> argument, it has some additional parameters described next, and on success it returns the verfied content as a memory BIO. The optional I<extra> 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<crls> 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 ee08c0c974..6e59b48813 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; |