Commit c55fda746a for openssl.org

commit c55fda746ab5fea93c8531f436bee867d6b30357
Author: Viktor Dukhovni <openssl-users@dukhovni.org>
Date:   Sun Mar 1 02:40:52 2026 +1100

    Polish AKID/SKID handling and related docs

    - Drop empty requestExtensions CSR attributes

      While `attributes` is a required CSR field, its `requestExtensions`
      attribute is optional, and should be avoided if empty.

    - Detail documentation of req extension section selection

    - Fixed req CI test case naming nits

    - Refer to config(5) for meaning of "variable"

    - In code comments, note possibility of fewer extensions after adding
      an ignored empty extension while deleting a previous value.

    - Mention new "nonss" AKID qualifier in CHANGES

    - I x509_config(5) Clarify AKID issuer as fallback (unless ":always")

    - In stock config file, comment proxy cert issuer SKID expectation.

    - Clarify comment on empty SKID/AKID vs. prior value

    - Use B<default> not C<default> for unnamed section

    - Polish (mostly CSR) extension handling

      * In update_req_extensions() drop extraneous duplicate
        X509at_delete_attr() call.
      * Consolidate empty SKID/AKID detection in new
        ossl_ignored_x509_extension().
      * Handle empty SKID/AKID also in X509V3_add1_i2d().
      * In test_drop_empty_csr_keyids() exercise the full NCONF extension
        management stack, using X509_REQ_get_attr_count() to check that
        after "subjectKeyIdentifier = none" not an even an empty extension
        set remains as a CSR attribute (X509_REQ_get_extensions() always
        returns at least an empty stack because NULL signals an error).

    Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
    Reviewed-by: Tim Hudson <tjh@openssl.org>
    Reviewed-by: Neil Horman <nhorman@openssl.org>
    MergeDate: Mon Mar  2 17:04:22 2026
    (Merged from https://github.com/openssl/openssl/pull/30217)

diff --git a/CHANGES.md b/CHANGES.md
index 09c2d4207c..bef6abebb6 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -37,6 +37,10 @@ OpenSSL Releases
    options just like any other extension.  See their documentation and also
    `x509v3_config(5)` for additional details.

+   Updated the syntax of the `subjectKeyIdentifier` (SKID) and
+   `authorityKeyIdentifier` (AKID) extensions, introducing the `nonss` qualifier
+   for the `keyid` and `issuer` keywords.
+
    The x509 "mini-CA" now attempts to find extension settings in the default
    configuration file even if neither the `-extfile` nor the `-extensions`
    option is explicitly specified.  Failure to open the default configuration
diff --git a/apps/openssl-vms.cnf b/apps/openssl-vms.cnf
index da0a530bb5..1378dfe248 100644
--- a/apps/openssl-vms.cnf
+++ b/apps/openssl-vms.cnf
@@ -291,6 +291,7 @@ basicConstraints=CA:FALSE

 # PKIX recommendations harmless if included in all certificates.
 subjectKeyIdentifier=hash
+# The issuer of a proxy certificate should have SKID.
 authorityKeyIdentifier=keyid:always

 # This stuff is for subjectAltName and issuerAltname.
diff --git a/apps/openssl.cnf b/apps/openssl.cnf
index 1de7883d23..c5b7262078 100644
--- a/apps/openssl.cnf
+++ b/apps/openssl.cnf
@@ -291,6 +291,7 @@ basicConstraints=CA:FALSE

 # PKIX recommendations harmless if included in all certificates.
 subjectKeyIdentifier=hash
+# The issuer of a proxy certificate should have SKID.
 authorityKeyIdentifier=keyid:always

 # This stuff is for subjectAltName and issuerAltname.
diff --git a/crypto/x509/v3_conf.c b/crypto/x509/v3_conf.c
index 81cfce8b48..6db7f37d68 100644
--- a/crypto/x509/v3_conf.c
+++ b/crypto/x509/v3_conf.c
@@ -396,11 +396,12 @@ update_req_extensions(X509_REQ *req, int *pnid, STACK_OF(X509_EXTENSION) *exts)

         if (att == NULL)
             goto end;
-        X509at_delete_attr(req->req_info.attributes, loc);
         X509_ATTRIBUTE_free(att);
     }
-    ret = X509_REQ_add1_attr_by_NID(req, *pnid, V_ASN1_SEQUENCE, ext, extlen);
-
+    if (sk_X509_EXTENSION_num(exts) > 0)
+        ret = X509_REQ_add1_attr_by_NID(req, *pnid, V_ASN1_SEQUENCE, ext, extlen);
+    else
+        ret = 1;
 end:
     OPENSSL_free(ext);
     return ret;
diff --git a/crypto/x509/v3_lib.c b/crypto/x509/v3_lib.c
index 8217fcdb2c..68ac36baf7 100644
--- a/crypto/x509/v3_lib.c
+++ b/crypto/x509/v3_lib.c
@@ -15,6 +15,7 @@
 #include <openssl/x509v3.h>

 #include "ext_dat.h"
+#include "x509_local.h"

 static STACK_OF(X509V3_EXT_METHOD) *ext_list = NULL;

@@ -129,6 +130,38 @@ int X509V3_add_standard_extensions(void)
     return 1;
 }

+int ossl_ignored_x509_extension(const X509_EXTENSION *ex, int flags)
+{
+    /*
+     * Empty OCTET STRINGs and empty SEQUENCEs encode to just two bytes of tag
+     * (0x04 or 0x30) and length (0x00).  We use this fact to suppress empty
+     * AKID and SKID extensions that may be briefly generated when processing
+     * the "= none" value or only ":nonss"-qualified AKIDs when the subject is
+     * self-signed.
+     *
+     * The resulting extension is empty, and must not be retained, but does
+     * serve to drop any previous value of the same extension, when called
+     * via
+     * - X509v3_add_extensions(), or
+     * - either of X509V3_add1_i2d() or X509V3_EXT_add_nconf_sk(),
+     *   with a flags (or ctx->flags) value that allows replacement.
+     */
+    if (ex->value.length == 2
+        && (ex->value.data[0] == 0x30 || ex->value.data[0] == 0x04)) {
+        ASN1_OBJECT *obj = ex->object;
+        ASN1_OBJECT *skid = OBJ_nid2obj(NID_subject_key_identifier);
+        ASN1_OBJECT *akid = OBJ_nid2obj(NID_authority_key_identifier);
+
+        if (OBJ_cmp(obj, skid) == 0 || OBJ_cmp(obj, akid) == 0) {
+            if ((flags & X509V3_ADD_SILENT) == 0)
+                ERR_raise_data(ERR_LIB_X509, X509_R_INVALID_EXTENSION,
+                    "Invalid empty X.509 %s extension", obj->sn);
+            return 1;
+        }
+    }
+    return 0;
+}
+
 /* Return an extension internal structure */

 void *X509V3_EXT_d2i(const X509_EXTENSION *ext)
@@ -278,9 +311,13 @@ int X509V3_add1_i2d(STACK_OF(X509_EXTENSION) **x, int nid, void *value,
     /* If extension exists replace it.. */
     if (extidx >= 0) {
         extmp = sk_X509_EXTENSION_value(*x, extidx);
-        X509_EXTENSION_free(extmp);
-        if (!sk_X509_EXTENSION_set(*x, extidx, ext))
+        if (ossl_ignored_x509_extension(ext, X509V3_ADD_SILENT)) {
+            if (!sk_X509_EXTENSION_delete(*x, extidx))
+                return -1;
+        } else if (!sk_X509_EXTENSION_set(*x, extidx, ext)) {
             return -1;
+        }
+        X509_EXTENSION_free(extmp);
         return 1;
     }

diff --git a/crypto/x509/x509_ext.c b/crypto/x509/x509_ext.c
index 55aae29618..2b73e14a60 100644
--- a/crypto/x509/x509_ext.c
+++ b/crypto/x509/x509_ext.c
@@ -117,10 +117,17 @@ int X509_add_ext(X509 *x, const X509_EXTENSION *ex, int loc)

     if (X509v3_add_ext(&exts, ex, loc) == NULL)
         return 0;
+    /*
+     * A duplicate empty SKID/AKID extension can displace a prior non-empty
+     * one, but is then not itself added, so, somewhat counter-intutively,  the
+     * the extension list can become empty after an "add", in which case we must
+     * drop the extension stack entirely, setting it to NULL.  The extensions
+     * list is either non-empty or absent.
+     */
     if (sk_X509_EXTENSION_num(exts) != 0) {
         x->cert_info.extensions = exts;
     } else {
-        sk_X509_EXTENSION_pop_free(exts, X509_EXTENSION_free);
+        sk_X509_EXTENSION_free(exts);
         sk_X509_EXTENSION_pop_free(x->cert_info.extensions, X509_EXTENSION_free);
         x->cert_info.extensions = NULL;
     }
diff --git a/crypto/x509/x509_local.h b/crypto/x509/x509_local.h
index 44260a23b7..6500226ec3 100644
--- a/crypto/x509/x509_local.h
+++ b/crypto/x509/x509_local.h
@@ -183,6 +183,7 @@ DEFINE_STACK_OF(BY_DIR_ENTRY)
 typedef STACK_OF(X509_NAME_ENTRY) STACK_OF_X509_NAME_ENTRY;
 DEFINE_STACK_OF(STACK_OF_X509_NAME_ENTRY)

+int ossl_ignored_x509_extension(const X509_EXTENSION *ex, int flags);
 int ossl_x509_likely_issued(const X509 *issuer, const X509 *subject);
 int ossl_x509_signing_allowed(const X509 *issuer, const X509 *subject);
 int ossl_x509_store_ctx_get_by_subject(const X509_STORE_CTX *ctx, X509_LOOKUP_TYPE type,
diff --git a/crypto/x509/x509_v3.c b/crypto/x509/x509_v3.c
index e343162582..f9ea5f75fe 100644
--- a/crypto/x509/x509_v3.c
+++ b/crypto/x509/x509_v3.c
@@ -120,23 +120,8 @@ STACK_OF(X509_EXTENSION) *X509v3_add_ext(STACK_OF(X509_EXTENSION) **x,
     } else
         sk = *x;

-    /*
-     * Empty OCTET STRINGs and empty SEQUENCEs encode to just two bytes of tag
-     * (0x04 or 0x30) and length (0x00).  We use this fact to suppress empty
-     * AKID and SKID extensions that may be briefly generated when processing
-     * the "= none" value or only ":nonss"-qualified AKIDs when the subject is
-     * self-signed.  The resulting extension is empty, and must not be retained,
-     * but does serve to drop any previous value of the same extension.
-     */
-    if (ex->value.length == 2
-        && (ex->value.data[0] == 0x30 || ex->value.data[0] == 0x04)) {
-        ASN1_OBJECT *obj = ex->object;
-        ASN1_OBJECT *skid = OBJ_nid2obj(NID_subject_key_identifier);
-        ASN1_OBJECT *akid = OBJ_nid2obj(NID_authority_key_identifier);
-
-        if (OBJ_cmp(obj, skid) == 0 || OBJ_cmp(obj, akid) == 0)
-            goto done;
-    }
+    if (ossl_ignored_x509_extension(ex, X509V3_ADD_SILENT))
+        goto done;

     n = sk_X509_EXTENSION_num(sk);
     if (loc > n)
diff --git a/crypto/x509/x_all.c b/crypto/x509/x_all.c
index fa7218ef86..f753282ca1 100644
--- a/crypto/x509/x_all.c
+++ b/crypto/x509/x_all.c
@@ -30,6 +30,7 @@
 #include "crypto/x509.h"
 #include "crypto/x509_acert.h"
 #include "crypto/rsa.h"
+#include "x509_local.h"

 int X509_verify(const X509 *a, EVP_PKEY *r)
 {
@@ -83,23 +84,9 @@ static int bad_keyid_exts(const STACK_OF(X509_EXTENSION) *exts)

     for (i = 0; i < n; ++i) {
         X509_EXTENSION *ext = sk_X509_EXTENSION_value(exts, i);
-        const ASN1_STRING *der = X509_EXTENSION_get_data(ext);
-
-        /*
-         * Empty OCTET STRINGs and empty SEQUENCEs encode to just two bytes of
-         * tag (0x04 or 0x30) and length (0x00).
-         */
-        if (der->length == 2 && (der->data[0] == 0x04 || der->data[0] == 0x30)) {
-            const ASN1_OBJECT *obj = X509_EXTENSION_get_object(ext);
-            const ASN1_OBJECT *skid = OBJ_nid2obj(NID_subject_key_identifier);
-            const ASN1_OBJECT *akid = OBJ_nid2obj(NID_authority_key_identifier);
-
-            if (OBJ_cmp(obj, skid) != 0 && OBJ_cmp(obj, akid) != 0)
-                continue;
-            ERR_raise_data(ERR_LIB_X509, X509_R_INVALID_EXTENSION,
-                "Invalid empty X.509 %s extension", obj->sn);
+
+        if (ossl_ignored_x509_extension(ext, X509V3_ADD_DEFAULT))
             return 1;
-        }
     }
     return 0;
 }
diff --git a/doc/man1/openssl-req.pod.in b/doc/man1/openssl-req.pod.in
index 031fd8ffca..fb46b8218e 100644
--- a/doc/man1/openssl-req.pod.in
+++ b/doc/man1/openssl-req.pod.in
@@ -393,6 +393,16 @@ certificate requests.
 This allows several different sections to be used in the same configuration
 file to specify requests for a variety of purposes.

+When signing a new CSR or X.509 certificate, if neither the the B<-extensions>
+option nor its alias B<-reqexts> are specified, the name of the extension
+section is taken from from C<req> section of the configuration file.
+Specifically, from the value of that section's C<req_extensions> variable (for
+a CSR) or its C<x509_extensions> variable (for a certificate).
+If there is no setting for the variable (name/value assignment, see
+L<config(5)>) in question, the configuration file does not add any extensions.
+The B<-section> option can be used to override C<req> with a custom section in
+which to look for the above C<req_extensions> and C<x509_extensions> variables.
+
 OpenSSL 4.0 removed built-in generation of the B<subjectKeyIdentifier> and
 B<authorityKeyIdentifier> extensions when generating certificates.
 Any desired extensions need to be listed either in the configuration file or
diff --git a/doc/man1/openssl-x509.pod.in b/doc/man1/openssl-x509.pod.in
index eff02af047..b1c3f4f190 100644
--- a/doc/man1/openssl-x509.pod.in
+++ b/doc/man1/openssl-x509.pod.in
@@ -490,9 +490,9 @@ This file must exist, be readable, and all the listed extensions must be valid.

 If only the B<-extfile> I<filename> option is given, the section name to use is
 taken from the value of the variable named C<extensions> in the file's unnamed
-(C<default>) section.
-If no B<extensions> setting is found, the file's C<default> section itself is
-instead used as the list of extensions to add.
+section (also called its B<default> section, see L<config(5)>).
+If no B<extensions> setting is found, the file's B<default> (unnamed) section
+itself is instead used as the list of extensions to add.
 As before, the file must be readable and all the extensions must be valid.

 Prior to OpenSSL 4.0, the B<-extensions> option required that the B<-extfile>
@@ -508,8 +508,8 @@ As before, the file must be readable and all the extensions must be valid.
 If neither of the options are given, an attempt is made to open the default
 configuration file (as detailed above).
 If the file does not exist or cannot be opened, no extensions are added.
-If the unnamed (C<default>) section of the file does not list an B<extensions>
-variable, no extensions are added.
+If the unnamed section (B<default>, see L<config(5)>) of the file does not list
+an B<extensions> variable, no extensions are added.
 Otherwise, the section named by the B<extensions> variable is taken to be the
 list of extensions to add.
 The extensions listed there must all be valid.
@@ -853,6 +853,7 @@ L<openssl-ca(1)>,
 L<openssl-genrsa(1)>,
 L<openssl-gendsa(1)>,
 L<openssl-verify(1)>,
+L<config(5)>,
 L<x509v3_config(5)>

 =head1 HISTORY
diff --git a/doc/man5/config.pod b/doc/man5/config.pod
index 36159c7820..6198ec6f6a 100644
--- a/doc/man5/config.pod
+++ b/doc/man5/config.pod
@@ -89,13 +89,11 @@ A configuration file is divided into a number of I<sections>.  A section
 begins with the section name in square brackets, and ends when a new
 section starts, or at the end of the file.  The section name can consist
 of alphanumeric characters and underscores.
-Whitespace between the name and the brackets is removed.
+Whitespace between the name and the brackets is ignored.

-The first section of a configuration file is special and is referred to
-as the B<default> section. This section is usually unnamed and spans from
-the start of file until the first named section. When a name is being
-looked up, it is first looked up in the current or named section,
-and then the default section if necessary.
+The content at the start of the configuration file that precedes its first
+named section is special and is referred to as the B<default> or B<unnamed>
+section.

 The environment is mapped onto a section called B<ENV>.

diff --git a/doc/man5/x509v3_config.pod b/doc/man5/x509v3_config.pod
index 4ca94f0539..358d67365e 100644
--- a/doc/man5/x509v3_config.pod
+++ b/doc/man5/x509v3_config.pod
@@ -218,7 +218,8 @@ identifier.
 The B<issuer> keyword asks that the AKID include the serial number and issuer
 distinguished name (grandparent name of subject certificate) of the issuer
 certificate.
-By default these are added only as a fallback when no SKID is available in the
+Unless qualified with C<always> (as described below), these are added only as a
+fallback if the B<keyid> is not requested, or no SKID was available in the
 issuer certificate.

 Either or both keywords can be suffixed with an optional qualifier, which can
@@ -237,9 +238,6 @@ When creating a self-signed certificate, be sure to specify a SKID extension if
 you want to have a mandatory (C<always> qualified) B<keyid> in the AKID
 extension.

-If an empty AKID would otherwise be generated, the extension is instead skipped
-entirely, just as with C<none>.
-
 See L<openssl-x509(1)>, L<openssl-req(1)>, and L<openssl-ca(1)> for further
 details relevant to that specific command-line utility.

diff --git a/test/recipes/25-test_req.t b/test/recipes/25-test_req.t
index eb4088949a..43e0c8e4a0 100644
--- a/test/recipes/25-test_req.t
+++ b/test/recipes/25-test_req.t
@@ -570,7 +570,7 @@ my $SKID_AKID = "subjectKeyIdentifier,authorityKeyIdentifier";

 # # SKID

-my $cert = "self-signed_default_SKID_no_explicit_exts.pem";
+my $cert = "self-signed_default_SKID_minimal_exts.pem";
 generate_cert($cert);
 has_version($cert, 3);
 has_SKID($cert, 1);
@@ -618,7 +618,7 @@ has_AKID($cert, 0); # forced no AKID

 $cert = "self-signed_v3_CA_explicit_AKID.pem";
 generate_cert($cert, @v3_ca, "-addext", "authorityKeyIdentifier = keyid:nonss");
-has_AKID($cert, 0); # for self-signed cert, AKID suppressed and not forced
+has_AKID($cert, 0); # for self-signed cert, AKID suppressed since self-signed

 $cert = "self-signed_v3_CA_forced_AKID.pem";
 generate_cert($cert, @v3_ca, "-addext", "authorityKeyIdentifier = keyid:always");
@@ -627,7 +627,7 @@ strict_verify($cert, 1);

 $cert = "self-signed_v3_CA_issuer_AKID.pem";
 generate_cert($cert, @v3_ca, "-addext", "authorityKeyIdentifier = issuer:nonss");
-has_AKID($cert, 0); # suppressed AKID since not forced
+has_AKID($cert, 0); # suppressed AKID since self-signed

 $cert = "self-signed_v3_CA_forced_issuer_AKID.pem";
 generate_cert($cert, @v3_ca, "-addext", "authorityKeyIdentifier = issuer:always");
diff --git a/test/recipes/80-test_ca.t b/test/recipes/80-test_ca.t
index 363fc100b5..f0f11902d7 100644
--- a/test/recipes/80-test_ca.t
+++ b/test/recipes/80-test_ca.t
@@ -82,7 +82,7 @@ SKIP: {
 my $v3_cert = "v3-test.crt";
 ok(run(app(["openssl", "ca", "-batch", "-config", $cnf, "-extensions", "minimal",
             "-in", src_file("x509-check.csr"), "-out", $v3_cert])));
-# although no explicit extensions given:
+# The "minimal" extensions include SKID and AKID.
 has_version($v3_cert, 3);
 has_SKID($v3_cert, 1);
 has_AKID($v3_cert, 1);
diff --git a/test/x509_test.c b/test/x509_test.c
index a3306dc8af..1381a1d33d 100644
--- a/test/x509_test.c
+++ b/test/x509_test.c
@@ -339,6 +339,8 @@ err:
 static int test_drop_empty_csr_keyids(void)
 {
     static const unsigned char commonName[] = "test";
+    BIO *bio = NULL;
+    CONF *conf = NULL;
     X509_REQ *x = NULL;
     X509_NAME *subject = NULL;
     X509_NAME_ENTRY *name_entry = NULL;
@@ -360,39 +362,42 @@ static int test_drop_empty_csr_keyids(void)
         || !TEST_int_eq(X509_REQ_set_pubkey(x, pubkey), 1))
         goto err;

-    X509V3_set_ctx(&ctx, NULL, NULL, x, NULL, 0);
-    if (!TEST_ptr(ext = X509V3_EXT_conf(NULL, &ctx, "subjectKeyIdentifier",
-                      "none"))
-        || !TEST_ptr(X509v3_add_ext(&exts, ext, -1))
-        || !TEST_int_eq(sk_X509_EXTENSION_num(exts), 0))
-        goto err;
-    X509_EXTENSION_free(ext);
-
-    if (!TEST_ptr(ext = X509V3_EXT_conf(NULL, &ctx, "authorityKeyIdentifier",
-                      "none"))
-        || !TEST_ptr(X509v3_add_ext(&exts, ext, -1)))
-        goto err;
-
-    if (!TEST_int_eq(X509_REQ_add_extensions(x, exts), 1)
-        || !TEST_int_eq(sk_X509_EXTENSION_num(exts), 0))
+    /* Add non-empty SKID, CSRs have no issuer, so no AKID */
+    if (!TEST_ptr(bio = BIO_new(BIO_s_mem()))
+        || !TEST_int_ge(BIO_printf(bio, "subjectKeyIdentifier = hash\n"), 0)
+        || !TEST_ptr(conf = NCONF_new(NULL))
+        || !TEST_int_gt(NCONF_load_bio(conf, bio, NULL), 0))
         goto err;
-    sk_X509_EXTENSION_pop_free(exts, X509_EXTENSION_free);
-
-    if (!TEST_ptr(exts = X509_REQ_get_extensions(x))
-        || !TEST_int_eq(sk_X509_EXTENSION_num(exts), 0))
+    (void)BIO_reset(bio);
+
+    X509V3_set_ctx(&ctx, NULL, NULL, x, NULL, X509V3_CTX_REPLACE);
+    X509V3_set_nconf(&ctx, conf);
+    if (!TEST_true(X509V3_EXT_REQ_add_nconf(conf, &ctx, "default", x))
+        || !TEST_int_eq(X509_REQ_get_attr_count(x), 1)
+        || !TEST_ptr(exts = X509_REQ_get_extensions(x))
+        || !TEST_int_eq(sk_X509_EXTENSION_num(exts), 1))
         goto err;
     sk_X509_EXTENSION_pop_free(exts, X509_EXTENSION_free);
     exts = NULL;

-    if (!TEST_int_gt(X509_REQ_sign(x, privkey, signmd), 0))
+    /* Request an "empty" SKID in order to drop the previous SKID */
+    NCONF_free(conf);
+    if (!TEST_ptr(conf = NCONF_new(NULL))
+        || !TEST_int_ge(BIO_printf(bio, "subjectKeyIdentifier = none\n"), 0)
+        || !TEST_int_gt(NCONF_load_bio(conf, bio, NULL), 0))
         goto err;

-    if (!TEST_ptr(exts = X509_REQ_get_extensions(x))
-        || !TEST_int_eq(sk_X509_EXTENSION_num(exts), 0))
+    X509V3_set_nconf(&ctx, conf);
+    if (!TEST_true(X509V3_EXT_REQ_add_nconf(conf, &ctx, "default", x))
+        || !TEST_int_gt(X509_REQ_sign(x, privkey, signmd), 0)
+        || !TEST_int_eq(X509_REQ_get_attr_count(x), 0))
         goto err;

     ret = 1;
+
 err:
+    BIO_free(bio);
+    NCONF_free(conf);
     X509_NAME_ENTRY_free(name_entry);
     X509_NAME_free(subject);
     X509_EXTENSION_free(ext);