Some more X509 extension add/del polish and fixups

Making better use of X509v3_delete_extension().

This includes two minor bugfixes:
The enc.modified flag was not set on deleting extensions in
X509_REQ and X509_CRL structures.

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
MergeDate: Tue Mar 24 17:29:09 2026
(Merged from https://github.com/openssl/openssl/pull/30350)

(cherry picked from commit 232279db6838a3f4fb850bfd93b9665c2d836299)

diff --git a/crypto/x509/v3_conf.c b/crypto/x509/v3_conf.c
index f223a5cb64b1a741b40680f814e59cbab94943e0..6b6f8457356e62a19d7f47413e12c884c52038fa 100644 (file)
--- a/crypto/x509/v3_conf.c
+++ b/crypto/x509/v3_conf.c
@@ -399,7 +399,7 @@ update_req_extensions(X509_REQ *req, int *pnid, STACK_OF(X509_EXTENSION) *exts)
     }
 
     if (loc != -1) {
-        X509_ATTRIBUTE *att = X509at_delete_attr(req->req_info.attributes, loc);
+        X509_ATTRIBUTE *att = X509_REQ_delete_attr(req, loc);
 
         if (att == NULL)
             goto end;

diff --git a/crypto/x509/x509_ext.c b/crypto/x509/x509_ext.c
index 73106434057f6e4bea8dea6a062eaa6ce8e89fbf..3cd4ac51fc42b679ea224ccdb9507f53ed8b7579 100644 (file)
--- a/crypto/x509/x509_ext.c
+++ b/crypto/x509/x509_ext.c
@@ -42,23 +42,15 @@ const X509_EXTENSION *X509_CRL_get_ext(const X509_CRL *x, int loc)
     return X509v3_get_ext(x->crl.extensions, loc);
 }
 
-static X509_EXTENSION *delete_ext(STACK_OF(X509_EXTENSION) **sk, int loc)
+X509_EXTENSION *X509_CRL_delete_ext(X509_CRL *x, int loc)
 {
-    X509_EXTENSION *ret = X509v3_delete_ext(*sk, loc);
+    X509_EXTENSION *ret = X509v3_delete_extension(&x->crl.extensions, loc);
 
-    /* Empty extension lists are omitted. */
-    if (*sk != NULL && sk_X509_EXTENSION_num(*sk) == 0) {
-        sk_X509_EXTENSION_pop_free(*sk, X509_EXTENSION_free);
-        *sk = NULL;
-    }
+    if (ret != NULL)
+        x->crl.enc.modified = 1;
     return ret;
 }
 
-X509_EXTENSION *X509_CRL_delete_ext(X509_CRL *x, int loc)
-{
-    return delete_ext(&x->crl.extensions, loc);
-}
-
 void *X509_CRL_get_ext_d2i(const X509_CRL *x, int nid, int *crit, int *idx)
 {
     return X509V3_get_d2i(x->crl.extensions, nid, crit, idx);
@@ -176,7 +168,7 @@ const X509_EXTENSION *X509_REVOKED_get_ext(const X509_REVOKED *x, int loc)
 
 X509_EXTENSION *X509_REVOKED_delete_ext(X509_REVOKED *x, int loc)
 {
-    return delete_ext(&x->extensions, loc);
+    return X509v3_delete_extension(&x->extensions, loc);
 }
 
 int X509_REVOKED_add_ext(X509_REVOKED *x, X509_EXTENSION *ex, int loc)

diff --git a/crypto/x509/x509_v3.c b/crypto/x509/x509_v3.c
index 13468d133d9c32e88d968105021f2c5ee93cbada..d3fc11416388ea93351886b972299391a2169db3 100644 (file)
--- a/crypto/x509/x509_v3.c
+++ b/crypto/x509/x509_v3.c
@@ -92,12 +92,7 @@ const X509_EXTENSION *X509v3_get_ext(const STACK_OF(X509_EXTENSION) *x, int loc)
 
 X509_EXTENSION *X509v3_delete_ext(STACK_OF(X509_EXTENSION) *x, int loc)
 {
-    X509_EXTENSION *ret;
-
-    if (x == NULL || sk_X509_EXTENSION_num(x) <= loc || loc < 0)
-        return NULL;
-    ret = sk_X509_EXTENSION_delete(x, loc);
-    return ret;
+    return sk_X509_EXTENSION_delete(x, loc);
 }
 
 X509_EXTENSION *X509v3_delete_extension(STACK_OF(X509_EXTENSION) **x, int loc)

diff --git a/doc/man3/X509v3_get_ext_by_NID.pod b/doc/man3/X509v3_get_ext_by_NID.pod
index c98db91de93bef9033d01a34acc066d1fe9ff255..38a6d6f0af21738e021e239b88a718996c91c466 100644 (file)
--- a/doc/man3/X509v3_get_ext_by_NID.pod
+++ b/doc/man3/X509v3_get_ext_by_NID.pod
@@ -87,7 +87,7 @@ If I<loc> is an invalid index value, NULL is returned.
 X509v3_delete_extension() extends X509v3_delete_ext() by deallocating the
 extension stack I<*x> if it becomes empty, and in that case also setting I<*x>
 to NULL.
-This is a convenience wrapper for cases in which extensions are optional and
+This is a convenience wrapper for cases in which a list of extensions is optional and
 should be omitted if the stack becomes empty.
 
 X509v3_add_ext() inserts extension I<ex> to STACK I<*x> at position I<loc>.

diff --git a/test/x509_test.c b/test/x509_test.c
index 094b25266138eebeb10f9833847b22e488de5183..f5f5cc586a045872db4a54efbbe173be64c1d386 100644 (file)
--- a/test/x509_test.c
+++ b/test/x509_test.c
@@ -339,7 +339,7 @@ static int test_drop_empty_cert_keyids(void)
         || !TEST_int_eq(sk_X509_EXTENSION_num(exts), 1))
         goto err;
 
-    /* Request "empty" SKID and AKID in order to drop any previous values */
+    /* Request "empty" SKID in order to drop any previous value */
     NCONF_free(conf);
     if (!TEST_ptr(conf = NCONF_new(NULL))
         || !TEST_int_ge(BIO_printf(bio, "subjectKeyIdentifier = none\n"), 0)