X509: Fix handling of AKID and SKID extensions according to configuration

Fixes #16300

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/16342)

6 files changed, 31 insertions, 11 deletions

diff --git a/apps/ca.c b/apps/ca.c
index 24883615ed..1e77bf50c5 100644
--- a/apps/ca.c
+++ b/apps/ca.c
@@ -1709,7 +1709,16 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
 
     /* Initialize the context structure */
     X509V3_set_ctx(&ext_ctx, selfsign ? ret : x509,
-                   ret, req, NULL, X509V3_CTX_REPLACE);
+                   ret, NULL /* no need to give req, needed info is in ret */,
+                   NULL, X509V3_CTX_REPLACE);
+    /* prepare fallback for AKID, but only if issuer cert equals subject cert */
+    if (selfsign) {
+        if (!X509V3_set_issuer_pkey(&ext_ctx, pkey))
+            goto end;
+        if (!cert_matches_key(ret, pkey))
+            BIO_printf(bio_err,
+                       "Warning: Signature key and public key of cert do not match\n");
+    }
 
     /* Lets add the extensions, if there are any */
     if (ext_sect) {
diff --git a/apps/include/apps.h b/apps/include/apps.h
index 9d5db16600..6018a83ca4 100644
--- a/apps/include/apps.h
+++ b/apps/include/apps.h
@@ -247,6 +247,7 @@ int x509_req_ctrl_string(X509_REQ *x, const char *value);
 int init_gen_str(EVP_PKEY_CTX **pctx,
                  const char *algname, ENGINE *e, int do_param,
                  OSSL_LIB_CTX *libctx, const char *propq);
+int cert_matches_key(const X509 *cert, const EVP_PKEY *pkey);
 int do_X509_sign(X509 *x, EVP_PKEY *pkey, const char *md,
                  STACK_OF(OPENSSL_STRING) *sigopts, X509V3_CTX *ext_ctx);
 int do_X509_verify(X509 *x, EVP_PKEY *pkey, STACK_OF(OPENSSL_STRING) *vfyopts);
diff --git a/apps/lib/apps.c b/apps/lib/apps.c
index b15abac857..82eeaea249 100644
--- a/apps/lib/apps.c
+++ b/apps/lib/apps.c
@@ -2224,8 +2224,8 @@ static int adapt_keyid_ext(X509 *cert, X509V3_CTX *ext_ctx,
     idx = X509v3_get_ext_by_OBJ(exts, X509_EXTENSION_get_object(new_ext), -1);
     if (idx >= 0) {
         X509_EXTENSION *found_ext = X509v3_get_ext(exts, idx);
-        ASN1_OCTET_STRING *data = X509_EXTENSION_get_data(found_ext);
-        int disabled = ASN1_STRING_length(data) <= 2; /* config said "none" */
+        ASN1_OCTET_STRING *encoded = X509_EXTENSION_get_data(found_ext);
+        int disabled = ASN1_STRING_length(encoded) <= 2; /* indicating "none" */
 
         if (disabled) {
             X509_delete_ext(cert, idx);
@@ -2239,6 +2239,16 @@ static int adapt_keyid_ext(X509 *cert, X509V3_CTX *ext_ctx,
     return rv;
 }
 
+int cert_matches_key(const X509 *cert, const EVP_PKEY *pkey)
+{
+    int match;
+
+    ERR_set_mark();
+    match = X509_check_private_key(cert, pkey);
+    ERR_pop_to_mark();
+    return match;
+}
+
 /* Ensure RFC 5280 compliance, adapt keyIDs as needed, and sign the cert info */
 int do_X509_sign(X509 *cert, EVP_PKEY *pkey, const char *md,
                  STACK_OF(OPENSSL_STRING) *sigopts, X509V3_CTX *ext_ctx)
@@ -2254,16 +2264,14 @@ int do_X509_sign(X509 *cert, EVP_PKEY *pkey, const char *md,
             goto end;
 
         /*
-         * Add default SKID before such that default AKID can make use of it
+         * Add default SKID before AKID such that AKID can make use of it
          * in case the certificate is self-signed
          */
         /* Prevent X509_V_ERR_MISSING_SUBJECT_KEY_IDENTIFIER */
         if (!adapt_keyid_ext(cert, ext_ctx, "subjectKeyIdentifier", "hash", 1))
             goto end;
         /* Prevent X509_V_ERR_MISSING_AUTHORITY_KEY_IDENTIFIER */
-        ERR_set_mark();
-        self_sign = X509_check_private_key(cert, pkey);
-        ERR_pop_to_mark();
+        self_sign = cert_matches_key(cert, pkey);
         if (!adapt_keyid_ext(cert, ext_ctx, "authorityKeyIdentifier",
                              "keyid, issuer", !self_sign))
             goto end;
diff --git a/apps/pkcs12.c b/apps/pkcs12.c
index dcb173f201..acc45c405a 100644
--- a/apps/pkcs12.c
+++ b/apps/pkcs12.c
@@ -555,7 +555,7 @@ int pkcs12_main(int argc, char **argv)
                 /* Look for matching private key */
                 for (i = 0; i < sk_X509_num(certs); i++) {
                     x = sk_X509_value(certs, i);
-                    if (X509_check_private_key(x, key)) {
+                    if (cert_matches_key(x, key)) {
                         ee_cert = x;
                         /* Zero keyid and alias */
                         X509_keyid_set1(ee_cert, NULL, 0);
diff --git a/apps/req.c b/apps/req.c
index 7997ea7649..274f839902 100644
--- a/apps/req.c
+++ b/apps/req.c
@@ -838,11 +838,9 @@ int req_main(int argc, char **argv)
             if (CAcert == NULL) {
                 if (!X509V3_set_issuer_pkey(&ext_ctx, issuer_key))
                     goto end;
-                ERR_set_mark();
-                if (!X509_check_private_key(new_x509, issuer_key))
+                if (!cert_matches_key(new_x509, issuer_key))
                     BIO_printf(bio_err,
                                "Warning: Signature key and public key of cert do not match\n");
-                ERR_pop_to_mark();
             }
             X509V3_set_nconf(&ext_ctx, req_conf);
 
diff --git a/apps/x509.c b/apps/x509.c
index b88fb4f5ea..ff95821bab 100644
--- a/apps/x509.c
+++ b/apps/x509.c
@@ -810,6 +810,10 @@ int x509_main(int argc, char **argv)
             goto end;
         if (!x509toreq && !reqfile && !newcert && !self_signed(ctx, x))
             goto end;
+    } else {
+        if (privkey != NULL && !cert_matches_key(x, privkey))
+            BIO_printf(bio_err,
+                       "Warning: Signature key and public key of cert do not match\n");
     }
 
     if (sno != NULL && !X509_set_serialNumber(x, sno))