Restore previous semantics of authorityKeyIdentifier=keyid

Also self-signed certificates should always have an AKID, not
only for "keyid:always" but also for "keyid", and just silently
fall back to not having an AKID if the signer does not have
an SKID extension, as this was previously.

Fixes openssl#29001

Author: bernd-edlinger

crypto/x509/v3_akid.c

@@ -107,7 +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;
+    int same_issuer;
     AUTHORITY_KEYID *akeyid = AUTHORITY_KEYID_new();
 
     if (akeyid == NULL)
@@ -156,44 +156,38 @@ static AUTHORITY_KEYID *v2i_AUTHORITY_KEYID(X509V3_EXT_METHOD *method,
         ERR_raise(ERR_LIB_X509V3, X509V3_R_NO_ISSUER_CERTIFICATE);
         goto err;
     }
-    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();
+    same_issuer = ctx->subject_cert == issuer_cert;
 
     /* unless forced with "always", AKID is suppressed for self-signed certs */
-    if (keyid == 2 || (keyid == 1 && !ss)) {
+    if (keyid != 0) {
         /*
          * prefer any pre-existing subject key identifier of the issuer cert
-         * except issuer cert is same as subject cert and is not self-signed
+         * except issuer cert is same as subject cert and private key is given
          */
-        i = X509_get_ext_by_NID(issuer_cert, NID_subject_key_identifier, -1);
-        if (i >= 0 && (ext = X509_get_ext(issuer_cert, i)) != NULL
-            && !(same_issuer && !ss)) {
-            ikeyid = X509V3_EXT_d2i(ext);
-            if (ASN1_STRING_length(ikeyid) == 0) /* indicating "none" */ {
-                ASN1_OCTET_STRING_free(ikeyid);
-                ikeyid = NULL;
-            }
-        }
-        if (ikeyid == NULL && same_issuer && ctx->issuer_pkey != NULL) {
+        if (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))
                 ikeyid = ossl_x509_pubkey_hash(pubkey);
             X509_PUBKEY_free(pubkey);
+        } else {
+            i = X509_get_ext_by_NID(issuer_cert, NID_subject_key_identifier, -1);
+            if (i >= 0 && (ext = X509_get_ext(issuer_cert, i)) != NULL) {
+                ikeyid = X509V3_EXT_d2i(ext);
+                if (ASN1_STRING_length(ikeyid) == 0) /* indicating "none" */ {
+                    ASN1_OCTET_STRING_free(ikeyid);
+                    ikeyid = NULL;
+                }
+            }
         }
         if (keyid == 2 && ikeyid == NULL) {
             ERR_raise(ERR_LIB_X509V3, X509V3_R_UNABLE_TO_GET_ISSUER_KEYID);
             goto err;
         }
     }
 
-    if (issuer == 2 || (issuer == 1 && !ss && ikeyid == NULL)) {
+    if (issuer == 2 || (issuer == 1 && !same_issuer && ikeyid == NULL)) {
         isname = X509_NAME_dup(X509_get_issuer_name(issuer_cert));
         serial = ASN1_INTEGER_dup(X509_get0_serialNumber(issuer_cert));
         if (isname == NULL || serial == NULL) {
@@ -217,8 +211,6 @@ static AUTHORITY_KEYID *v2i_AUTHORITY_KEYID(X509V3_EXT_METHOD *method,
     }
 
     akeyid->issuer = gens;
-    gen = NULL;
-    gens = NULL;
     akeyid->serial = serial;
     akeyid->keyid = ikeyid;
 

test/recipes/25-test_req.t

@@ -612,7 +612,7 @@ has_AKID($cert, 0); # forced no AKID
 
 $cert = "self-signed_v3_CA_explicit_AKID.pem";
 generate_cert($cert, @v3_ca, "-addext", "authorityKeyIdentifier = keyid");
-has_AKID($cert, 0); # for self-signed cert, AKID suppressed and not forced
+has_AKID($cert, 1); # for self-signed cert, AKID present but not forced
 
 $cert = "self-signed_v3_CA_forced_AKID.pem";
 generate_cert($cert, @v3_ca, "-addext", "authorityKeyIdentifier = keyid:always");
@@ -629,11 +629,11 @@ cert_contains($cert, "Authority Key Identifier: DirName:/CN=CA serial:", 1); # f
 
 $cert = "self-signed_v3_CA_nonforced_keyid_issuer_AKID.pem";
 generate_cert($cert, @v3_ca, "-addext", "authorityKeyIdentifier = keyid, issuer");
-has_AKID($cert, 0); # AKID not present because not forced and cert self-signed
+has_AKID($cert, 1); # AKID present but not forced
 
 $cert = "self-signed_v3_CA_keyid_forced_issuer_AKID.pem";
 generate_cert($cert, @v3_ca, "-addext", "authorityKeyIdentifier = keyid, issuer:always");
-cert_contains($cert, "Authority Key Identifier: DirName:/CN=CA serial:", 1); # issuer AKID forced, with keyid not forced
+cert_contains($cert, "Authority Key Identifier: keyid:.* DirName:/CN=CA serial:", 1); # issuer AKID forced, with keyid not forced
 
 $cert = "self-signed_v3_CA_forced_keyid_issuer_AKID.pem";
 generate_cert($cert, @v3_ca, "-addext", "authorityKeyIdentifier = keyid:always, issuer");