Clear the extension list when removing the last extension

The extensions list in a certificate, CRL, and CRL entry is defined as:

    ... extensions      [3]  EXPLICIT Extensions OPTIONAL ...
    ... crlEntryExtensions      Extensions OPTIONAL ...
    ... crlExtensions           [0]  EXPLICIT Extensions OPTIONAL ...

    Extensions  ::=  SEQUENCE SIZE (1..MAX) OF Extension

This means that a present but empty extensions list is actually invalid.
Rather, if you have no extensions to encode, you are meant to omit the
list altogether. Fix the delete_ext functions to handle this correctly.

This would mostly be moot, as an application adding extensions only to
delete them all would be unusual. However, #13658 implemented a slightly
roundabout design where, to omit SKID/AKID, the library first puts them
in and then the command-line tool detects some placeholder values and
deletes the extension again.

Fixes #28397

Reviewed-by: Paul Dale <ppzgs1@gmail.com>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/28398)

(cherry picked from commit 9a8d7dc142)
This commit is contained in:
David Benjamin 2025-08-31 17:25:40 -04:00 committed by Tomas Mraz
parent 871e71ff6b
commit e32015b8f4
2 changed files with 137 additions and 3 deletions

View File

@ -44,7 +44,15 @@ X509_EXTENSION *X509_CRL_get_ext(const X509_CRL *x, int loc)
X509_EXTENSION *X509_CRL_delete_ext(X509_CRL *x, int loc)
{
return X509v3_delete_ext(x->crl.extensions, loc);
X509_EXTENSION *ret = X509v3_delete_ext(x->crl.extensions, loc);
/* Empty extension lists are omitted. */
if (x->crl.extensions != NULL &&
sk_X509_EXTENSION_num(x->crl.extensions) == 0) {
sk_X509_EXTENSION_pop_free(x->crl.extensions, X509_EXTENSION_free);
x->crl.extensions = NULL;
}
return ret;
}
void *X509_CRL_get_ext_d2i(const X509_CRL *x, int nid, int *crit, int *idx)
@ -91,7 +99,16 @@ X509_EXTENSION *X509_get_ext(const X509 *x, int loc)
X509_EXTENSION *X509_delete_ext(X509 *x, int loc)
{
return X509v3_delete_ext(x->cert_info.extensions, loc);
X509_EXTENSION *ret = X509v3_delete_ext(x->cert_info.extensions, loc);
/* Empty extension lists are omitted. */
if (x->cert_info.extensions != NULL &&
sk_X509_EXTENSION_num(x->cert_info.extensions) == 0) {
sk_X509_EXTENSION_pop_free(x->cert_info.extensions,
X509_EXTENSION_free);
x->cert_info.extensions = NULL;
}
return ret;
}
int X509_add_ext(X509 *x, X509_EXTENSION *ex, int loc)
@ -139,7 +156,15 @@ X509_EXTENSION *X509_REVOKED_get_ext(const X509_REVOKED *x, int loc)
X509_EXTENSION *X509_REVOKED_delete_ext(X509_REVOKED *x, int loc)
{
return X509v3_delete_ext(x->extensions, loc);
X509_EXTENSION *ret = X509v3_delete_ext(x->extensions, loc);
/* Empty extension lists are omitted. */
if (x->extensions != NULL &&
sk_X509_EXTENSION_num(x->extensions) == 0) {
sk_X509_EXTENSION_pop_free(x->extensions, X509_EXTENSION_free);
x->extensions = NULL;
}
return ret;
}
int X509_REVOKED_add_ext(X509_REVOKED *x, X509_EXTENSION *ex, int loc)

View File

@ -176,6 +176,112 @@ static int test_asn1_item_verify(void)
return ret;
}
static int test_x509_delete_last_extension(void)
{
int ret = 0;
X509 *x509 = NULL;
X509_EXTENSION *ext = NULL;
ASN1_OBJECT *obj = NULL;
if (!TEST_ptr((x509 = X509_new()))
/* Initially, there are no extensions and thus no extension list. */
|| !TEST_ptr_null(X509_get0_extensions(x509))
/* Add an extension. */
|| !TEST_ptr((ext = X509_EXTENSION_new()))
|| !TEST_ptr((obj = OBJ_nid2obj(NID_subject_key_identifier)))
|| !TEST_int_eq(X509_EXTENSION_set_object(ext, obj), 1)
|| !TEST_int_eq(X509_add_ext(x509, ext, -1), 1)
/* There should now be an extension list. */
|| !TEST_ptr(X509_get0_extensions(x509))
|| !TEST_int_eq(sk_X509_EXTENSION_num(X509_get0_extensions(x509)), 1))
goto err;
/* Delete the extension. */
X509_EXTENSION_free(X509_delete_ext(x509, 0));
/* The extension list should be NULL again. */
if (!TEST_ptr_null(X509_get0_extensions(x509)))
goto err;
ret = 1;
err:
X509_free(x509);
X509_EXTENSION_free(ext);
return ret;
}
static int test_x509_crl_delete_last_extension(void)
{
int ret = 0;
X509_CRL *crl = NULL;
X509_EXTENSION *ext = NULL;
ASN1_OBJECT *obj = NULL;
if (!TEST_ptr((crl = X509_CRL_new()))
/* Initially, there are no extensions and thus no extension list. */
|| !TEST_ptr_null(X509_CRL_get0_extensions(crl))
/* Add an extension. */
|| !TEST_ptr((ext = X509_EXTENSION_new()))
|| !TEST_ptr((obj = OBJ_nid2obj(NID_subject_key_identifier)))
|| !TEST_int_eq(X509_EXTENSION_set_object(ext, obj), 1)
|| !TEST_int_eq(X509_CRL_add_ext(crl, ext, -1), 1)
/* There should now be an extension list. */
|| !TEST_ptr(X509_CRL_get0_extensions(crl))
|| !TEST_int_eq(sk_X509_EXTENSION_num(X509_CRL_get0_extensions(crl)),
1))
goto err;
/* Delete the extension. */
X509_EXTENSION_free(X509_CRL_delete_ext(crl, 0));
/* The extension list should be NULL again. */
if (!TEST_ptr_null(X509_CRL_get0_extensions(crl)))
goto err;
ret = 1;
err:
X509_CRL_free(crl);
X509_EXTENSION_free(ext);
return ret;
}
static int test_x509_revoked_delete_last_extension(void)
{
int ret = 0;
X509_REVOKED *rev = NULL;
X509_EXTENSION *ext = NULL;
ASN1_OBJECT *obj = NULL;
if (!TEST_ptr((rev = X509_REVOKED_new()))
/* Initially, there are no extensions and thus no extension list. */
|| !TEST_ptr_null(X509_REVOKED_get0_extensions(rev))
/* Add an extension. */
|| !TEST_ptr((ext = X509_EXTENSION_new()))
|| !TEST_ptr((obj = OBJ_nid2obj(NID_subject_key_identifier)))
|| !TEST_int_eq(X509_EXTENSION_set_object(ext, obj), 1)
|| !TEST_int_eq(X509_REVOKED_add_ext(rev, ext, -1), 1)
/* There should now be an extension list. */
|| !TEST_ptr(X509_REVOKED_get0_extensions(rev))
|| !TEST_int_eq(sk_X509_EXTENSION_num(X509_REVOKED_get0_extensions(rev)), 1))
goto err;
/* Delete the extension. */
X509_EXTENSION_free(X509_REVOKED_delete_ext(rev, 0));
/* The extension list should be NULL again. */
if (!TEST_ptr_null(X509_REVOKED_get0_extensions(rev)))
goto err;
ret = 1;
err:
X509_REVOKED_free(rev);
X509_EXTENSION_free(ext);
return ret;
}
OPT_TEST_DECLARE_USAGE("<pss-self-signed-cert.pem>\n")
int setup_tests(void)
@ -210,6 +316,9 @@ int setup_tests(void)
ADD_TEST(test_x509_tbs_cache);
ADD_TEST(test_x509_crl_tbs_cache);
ADD_TEST(test_asn1_item_verify);
ADD_TEST(test_x509_delete_last_extension);
ADD_TEST(test_x509_crl_delete_last_extension);
ADD_TEST(test_x509_revoked_delete_last_extension);
return 1;
}