From: Mounir IDRASSI Date: Sun, 7 Jun 2026 06:33:43 +0000 (+0900) Subject: Fix CMS Ed448 signer NULL digest handling X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3f7640a6c015496b6c02ba7d5e5ff89df0bae603;p=thirdparty%2Fopenssl.git Fix CMS Ed448 signer NULL digest handling Reject ossl_cms_adjust_md() failures unconditionally in CMS_add1_signer(), so unsupported Ed448 signing with signed attributes fails cleanly instead of passing a NULL digest to X509_ALGOR_set_md(). Add a defensive NULL digest check to X509_ALGOR_set_md(), and cover the CMS Ed448 signed-attributes/no-attributes cases plus the low-level NULL digest path in regression tests. Reviewed-by: Andrew Dinh Reviewed-by: Jakub Zelenka Reviewed-by: Tomas Mraz MergeDate: Wed Jul 29 16:49:57 2026 (Merged from https://github.com/openssl/openssl/pull/31409) --- diff --git a/crypto/asn1/x_algor.c b/crypto/asn1/x_algor.c index 5050adb8f15..4d7e5f6a0a2 100644 --- a/crypto/asn1/x_algor.c +++ b/crypto/asn1/x_algor.c @@ -87,12 +87,25 @@ void X509_ALGOR_get0(const ASN1_OBJECT **paobj, int *pptype, /* Set up an X509_ALGOR DigestAlgorithmIdentifier from an EVP_MD */ int X509_ALGOR_set_md(X509_ALGOR *alg, const EVP_MD *md) { - int type = md->flags & EVP_MD_FLAG_DIGALGID_ABSENT ? V_ASN1_UNDEF - : V_ASN1_NULL; - int md_type = EVP_MD_type(md); + int type, md_type; + ASN1_OBJECT *obj; - ASN1_OBJECT *obj = (md_type == NID_undef) ? OBJ_txt2obj(EVP_MD_get0_name(md), 0) : OBJ_nid2obj(md_type); - return X509_ALGOR_set0(alg, obj, type, NULL); + if (alg == NULL || md == NULL) + return 0; + + type = md->flags & EVP_MD_FLAG_DIGALGID_ABSENT ? V_ASN1_UNDEF + : V_ASN1_NULL; + md_type = EVP_MD_type(md); + + obj = (md_type == NID_undef) ? OBJ_txt2obj(EVP_MD_get0_name(md), 0) + : OBJ_nid2obj(md_type); + if (obj == NULL) + return 0; + if (!X509_ALGOR_set0(alg, obj, type, NULL)) { + ASN1_OBJECT_free(obj); + return 0; + } + return 1; } int X509_ALGOR_cmp(const X509_ALGOR *a, const X509_ALGOR *b) diff --git a/crypto/cms/cms_sd.c b/crypto/cms/cms_sd.c index 352f75a45ce..e53a5232075 100644 --- a/crypto/cms/cms_sd.c +++ b/crypto/cms/cms_sd.c @@ -631,7 +631,7 @@ CMS_SignerInfo *CMS_add1_signer(CMS_ContentInfo *cms, if (!ossl_cms_set1_SignerIdentifier(si->sid, signer, type, ctx)) goto err; - if (ossl_cms_adjust_md(ctx, pk, &md, &local_md, flags) != 1 && local_md != md) + if (ossl_cms_adjust_md(ctx, pk, &md, &local_md, flags) != 1) goto err; if (!X509_ALGOR_set_md(si->digestAlgorithm, md)) diff --git a/test/cmsapitest.c b/test/cmsapitest.c index e583b33f2c7..977095b3f80 100644 --- a/test/cmsapitest.c +++ b/test/cmsapitest.c @@ -19,6 +19,8 @@ static X509 *cert = NULL; static EVP_PKEY *privkey = NULL; +static X509 *ed448_cert = NULL; +static EVP_PKEY *ed448_privkey = NULL; static char *derin = NULL; static char *too_long_iv_cms_in = NULL; static char *pwri_kek_oob_der_in = NULL; @@ -288,6 +290,47 @@ static int test_CMS_add1_cert(void) return ret; } +static int test_CMS_add1_signer_ed448(const EVP_MD *md, unsigned int flags, + int expect_success) +{ + CMS_ContentInfo *cms = NULL; + CMS_SignerInfo *si = NULL; + int ret = 0; + + if (!TEST_ptr(cms = CMS_ContentInfo_new())) + goto end; + + si = CMS_add1_signer(cms, ed448_cert, ed448_privkey, md, flags); + if (expect_success) { + if (!TEST_ptr(si)) + goto end; + } else if (!TEST_ptr_null(si)) { + goto end; + } + + ret = 1; +end: + if (!expect_success && ret) + ERR_clear_error(); + CMS_ContentInfo_free(cms); + return ret; +} + +static int test_CMS_add1_signer_ed448_signed_attrs(void) +{ + return test_CMS_add1_signer_ed448(NULL, 0, 0); +} + +static int test_CMS_add1_signer_ed448_signed_attrs_md(void) +{ + return test_CMS_add1_signer_ed448(EVP_shake256(), 0, 0); +} + +static int test_CMS_add1_signer_ed448_noattr(void) +{ + return test_CMS_add1_signer_ed448(NULL, CMS_NOATTR, 1); +} + static int test_d2i_CMS_bio_NULL(void) { BIO *bio, *content = NULL; @@ -739,12 +782,12 @@ end: return ret; } -OPT_TEST_DECLARE_USAGE("certfile privkeyfile derfile tooLongIVpem pwriKekOobDer\n") +OPT_TEST_DECLARE_USAGE("certfile privkeyfile derfile tooLongIVpem pwriKekOobDer [ed448certfile ed448privkeyfile]\n") int setup_tests(void) { char *certin = NULL, *privkeyin = NULL; - BIO *certbio = NULL, *privkeybio = NULL; + char *ed448_certin = NULL, *ed448_privkeyin = NULL; if (!test_skip_common_options()) { TEST_error("Error parsing test options\n"); @@ -758,28 +801,28 @@ int setup_tests(void) || !TEST_ptr(pwri_kek_oob_der_in = test_get_argument(4))) return 0; - certbio = BIO_new_file(certin, "r"); - if (!TEST_ptr(certbio)) - return 0; - if (!TEST_true(PEM_read_bio_X509(certbio, &cert, NULL, NULL))) { - BIO_free(certbio); - return 0; - } - BIO_free(certbio); - - privkeybio = BIO_new_file(privkeyin, "r"); - if (!TEST_ptr(privkeybio)) { + if (!TEST_ptr(cert = load_cert_pem(certin, NULL)) + || !TEST_ptr(privkey = load_pkey_pem(privkeyin, NULL))) { X509_free(cert); cert = NULL; + EVP_PKEY_free(privkey); + privkey = NULL; return 0; } - if (!TEST_true(PEM_read_bio_PrivateKey(privkeybio, &privkey, NULL, NULL))) { - BIO_free(privkeybio); - X509_free(cert); - cert = NULL; - return 0; + + if (test_get_argument_count() >= 7) { + ed448_certin = test_get_argument(5); + ed448_privkeyin = test_get_argument(6); + + if (!TEST_ptr(ed448_cert = load_cert_pem(ed448_certin, NULL)) + || !TEST_ptr(ed448_privkey = load_pkey_pem(ed448_privkeyin, NULL))) { + X509_free(ed448_cert); + ed448_cert = NULL; + EVP_PKEY_free(ed448_privkey); + ed448_privkey = NULL; + return 0; + } } - BIO_free(privkeybio); ADD_TEST(test_encrypt_decrypt_aes_cbc); ADD_TEST(test_encrypt_decrypt_aes_128_gcm); @@ -796,6 +839,11 @@ int setup_tests(void) ADD_ALL_TESTS(test_d2i_CMS_decode, 2); ADD_TEST(test_cms_aesgcm_iv_too_long); ADD_TEST(test_pwri_kek_unwrap_short_encrypted_key); + if (ed448_cert != NULL && ed448_privkey != NULL) { + ADD_TEST(test_CMS_add1_signer_ed448_signed_attrs); + ADD_TEST(test_CMS_add1_signer_ed448_signed_attrs_md); + ADD_TEST(test_CMS_add1_signer_ed448_noattr); + } return 1; } @@ -803,4 +851,6 @@ void cleanup_tests(void) { X509_free(cert); EVP_PKEY_free(privkey); + X509_free(ed448_cert); + EVP_PKEY_free(ed448_privkey); } diff --git a/test/recipes/80-test_cmsapi.t b/test/recipes/80-test_cmsapi.t index 3d1dae84646..2813e46b26b 100644 --- a/test/recipes/80-test_cmsapi.t +++ b/test/recipes/80-test_cmsapi.t @@ -16,9 +16,14 @@ plan skip_all => "CMS is disabled in this build" if disabled("cms"); plan tests => 1; +my @ed448_args = disabled("ecx") ? () : ( + srctop_file("test", "certs", "server-ed448-cert.pem"), + srctop_file("test", "certs", "server-ed448-key.pem")); + ok(run(test(["cmsapitest", srctop_file("test", "certs", "servercert.pem"), srctop_file("test", "certs", "serverkey.pem"), srctop_file("test", "recipes", "80-test_cmsapi_data", "encryptedData.der"), srctop_file("test", "recipes", "80-test_cmsapi_data", "encDataWithTooLongIV.pem"), - srctop_file("test", "recipes", "80-test_cmsapi_data", "cms_pwri_kek_oob.der")])), + srctop_file("test", "recipes", "80-test_cmsapi_data", "cms_pwri_kek_oob.der"), + @ed448_args])), "running cmsapitest"); diff --git a/test/x509_internal_test.c b/test/x509_internal_test.c index 1176ef49efa..85ef13036f1 100644 --- a/test/x509_internal_test.c +++ b/test/x509_internal_test.c @@ -688,6 +688,24 @@ err: return test; } +static int test_X509_ALGOR_set_md_null(void) +{ + X509_ALGOR *alg = NULL; + int ret = 0; + + if (!TEST_ptr(alg = X509_ALGOR_new())) + goto err; + + if (!TEST_false(X509_ALGOR_set_md(alg, NULL))) + goto err; + + ret = 1; + +err: + X509_ALGOR_free(alg); + return ret; +} + /* https://github.com/openssl/openssl/issues/26325 */ static const char *kRootExtensionDuplicity[] = { "-----BEGIN CERTIFICATE-----\n", @@ -948,6 +966,7 @@ int setup_tests(void) ADD_TEST(tests_X509_check_crypto); ADD_TEST(tests_x509_check_dpn); ADD_TEST(tests_x509_check_akid); + ADD_TEST(test_X509_ALGOR_set_md_null); ADD_TEST(tests_x509_check_ext_duplicity); ADD_TEST(tests_x509_check_ext_duplicity_nid_undef); ADD_TEST(tests_x509_check_ext_duplicity_nid_dynamic);