From: David Benjamin <davidben@google.com>
Date: Sun, 31 Aug 2025 21:25:40 +0000 (-0400)
Subject: Clear the extension list when removing the last extension
X-Git-Tag: openssl-3.0.18~36
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ed20a5ee3cacf1ee8f376e9744bd360bf2975bde;p=thirdparty%2Fopenssl.git

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 9a8d7dc14201aeeed1e77d54208e4af96916fc4f)
---

diff --git a/crypto/x509/x509_ext.c b/crypto/x509/x509_ext.c
index a7b85857bda..bed20bd5aeb 100644
--- a/crypto/x509/x509_ext.c
+++ b/crypto/x509/x509_ext.c
@@ -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)