APPS/x509: Fix generation of AKID via v2i_AUTHORITY_KEYID()

Fixes #16300

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16442)

(cherry picked from commit 9bf1061c44c81059102cd4749f6078b6ce71da9d)

4 files changed, 33 insertions, 12 deletions

diff --git a/apps/x509.c b/apps/x509.c
index 203ce919ba..2880ae792a 100644
--- a/apps/x509.c
+++ b/apps/x509.c
@@ -816,7 +816,12 @@ int x509_main(int argc, char **argv)
             goto end;
     }
 
-    X509V3_set_ctx(&ext_ctx, issuer_cert, x, req, NULL, X509V3_CTX_REPLACE);
+    X509V3_set_ctx(&ext_ctx, issuer_cert, x, NULL, NULL, X509V3_CTX_REPLACE);
+    /* prepare fallback for AKID, but only if issuer cert equals subject cert */
+    if (CAfile == NULL) {
+        if (!X509V3_set_issuer_pkey(&ext_ctx, privkey))
+            goto end;
+    }
     if (extconf != NULL && !x509toreq) {
         X509V3_set_nconf(&ext_ctx, extconf);
         if (!X509V3_EXT_add_nconf(extconf, &ext_ctx, extsect, x)) {
diff --git a/crypto/x509/v3_akid.c b/crypto/x509/v3_akid.c
index 5abd35d644..43b515f50c 100644
--- a/crypto/x509/v3_akid.c
+++ b/crypto/x509/v3_akid.c
@@ -107,6 +107,7 @@ static AUTHORITY_KEYID *v2i_AUTHORITY_KEYID(X509V3_EXT_METHOD *method,
     ASN1_INTEGER *serial = NULL;
     X509_EXTENSION *ext;
     X509 *issuer_cert;
+    int same_issuer, ss;
     AUTHORITY_KEYID *akeyid = AUTHORITY_KEYID_new();
 
     if (akeyid == NULL)
@@ -144,14 +145,26 @@ static AUTHORITY_KEYID *v2i_AUTHORITY_KEYID(X509V3_EXT_METHOD *method,
         ERR_raise(ERR_LIB_X509V3, X509V3_R_NO_ISSUER_CERTIFICATE);
         goto err;
     }
-
-    if (keyid != 0) {
-        /* prefer any pre-existing subject key identifier of the issuer cert */
+    same_issuer = ctx->subject_cert == ctx->issuer_cert;
+    ERR_set_mark();
+    if (ctx->issuer_pkey != NULL)
+        ss = X509_check_private_key(ctx->subject_cert, ctx->issuer_pkey);
+    else
+        ss = same_issuer;
+    ERR_pop_to_mark();
+
+    /* unless forced with "always", AKID is suppressed for self-signed certs */
+    if (keyid == 2 || (keyid == 1 && !ss)) {
+        /*
+         * prefer any pre-existing subject key identifier of the issuer cert
+         * except issuer cert is same as subject cert and is not self-signed
+         */
         i = X509_get_ext_by_NID(issuer_cert, NID_subject_key_identifier, -1);
-        if (i >= 0 && (ext = X509_get_ext(issuer_cert, i)) != NULL)
+        if (i >= 0 && (ext = X509_get_ext(issuer_cert, i)) != NULL
+            && !(same_issuer && !ss))
             ikeyid = X509V3_EXT_d2i(ext);
-        if (ikeyid == NULL && ctx->issuer_pkey != NULL) { /* fallback */
-            /* generate AKID from scratch, emulating s2i_skey_id(..., "hash") */
+        if (ikeyid == NULL && same_issuer && ctx->issuer_pkey != NULL) {
+            /* generate fallback AKID, emulating s2i_skey_id(..., "hash") */
             X509_PUBKEY *pubkey = NULL;
 
             if (X509_PUBKEY_set(&pubkey, ctx->issuer_pkey))
diff --git a/doc/man5/x509v3_config.pod b/doc/man5/x509v3_config.pod
index 1d4c4dc3ae..2a3afee27f 100644
--- a/doc/man5/x509v3_config.pod
+++ b/doc/man5/x509v3_config.pod
@@ -194,13 +194,16 @@ Otherwise it may have the value B<keyid> or B<issuer>
 or both of them, separated by C<,>.
 Either or both can have the option B<always>,
 indicated by putting a colon C<:> between the value and this option.
+For self-signed certificates the AKID is suppressed unless B<always> is present.
 By default the B<x509>, B<req>, and B<ca> apps behave as if
 "none" was given for self-signed certificates and "keyid, issuer" otherwise.
 
-If B<keyid> is present, an attempt is made to compute the hash of the public key
-corresponding to the signing key in case the certificate is self-signed,
-or else to copy the subject key identifier (SKID) from the issuer certificate.
-If this fails and the option B<always> is present, an error is returned.
+If B<keyid> is present, an attempt is made to
+copy the subject key identifier (SKID) from the issuer certificate except if
+the issuer certificate is the same as the current one and it is not self-signed.
+The hash of the public key related to the signing key is taken as fallback
+if the issuer certificate is the same as the current certificate.
+If B<always> is present but no value can be obtained, an error is returned.
 
 If B<issuer> is present, and in addition it has the option B<always> specified
 or B<keyid> is not present,
diff --git a/test/recipes/25-test_req.t b/test/recipes/25-test_req.t
index a405810ae2..235b53c61c 100644
--- a/test/recipes/25-test_req.t
+++ b/test/recipes/25-test_req.t
@@ -433,7 +433,7 @@ cert_ext_has_n_different_lines($cert, 0, $SKID_AKID); # no SKID and no AKID
 
 $cert = "self-signed_v3_CA_both_KIDs.pem";
 generate_cert($cert, @v3_ca, "-addext", "subjectKeyIdentifier = hash",
-            "-addext", "authorityKeyIdentifier = keyid");
+            "-addext", "authorityKeyIdentifier = keyid:always");
 cert_ext_has_n_different_lines($cert, 3, $SKID_AKID); # SKID == AKID
 strict_verify($cert, 1);