apps/{req,x509,ca}.c Make sure certs have SKID and AKID X.509 extensions by default

Fixes #13603

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

5 files changed, 54 insertions, 11 deletions

diff --git a/apps/ca.c b/apps/ca.c
index 2772072b79..f580d97e2d 100755
--- a/apps/ca.c
+++ b/apps/ca.c
@@ -1482,6 +1482,7 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
     OPENSSL_STRING *irow = NULL;
     OPENSSL_STRING *rrow = NULL;
     char buf[25];
+    X509V3_CTX ext_ctx;
 
     for (i = 0; i < DB_NUMBER; i++)
         row[i] = NULL;
@@ -1699,8 +1700,6 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
 
     /* Lets add the extensions, if there are any */
     if (ext_sect) {
-        X509V3_CTX ext_ctx;
-
         /* Initialize the context structure */
         X509V3_set_ctx(&ext_ctx, selfsign ? ret : x509,
                        ret, req, NULL, X509V3_CTX_REPLACE);
@@ -1903,7 +1902,7 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
         !EVP_PKEY_missing_parameters(pkey))
         EVP_PKEY_copy_parameters(pktmp, pkey);
 
-    if (!do_X509_sign(ret, pkey, dgst, sigopts))
+    if (!do_X509_sign(ret, pkey, dgst, sigopts, &ext_ctx))
         goto end;
 
     /* We now just add it to the database as DB_TYPE_VAL('V') */
diff --git a/apps/include/apps.h b/apps/include/apps.h
index 0a8d6f4060..30dc5d85f7 100644
--- a/apps/include/apps.h
+++ b/apps/include/apps.h
@@ -231,7 +231,7 @@ int init_gen_str(EVP_PKEY_CTX **pctx,
                  const char *algname, ENGINE *e, int do_param,
                  OSSL_LIB_CTX *libctx, const char *propq);
 int do_X509_sign(X509 *x, EVP_PKEY *pkey, const EVP_MD *md,
-                 STACK_OF(OPENSSL_STRING) *sigopts);
+                 STACK_OF(OPENSSL_STRING) *sigopts, X509V3_CTX *ext_ctx);
 int do_X509_verify(X509 *x, EVP_PKEY *pkey, STACK_OF(OPENSSL_STRING) *vfyopts);
 int do_X509_REQ_sign(X509_REQ *x, EVP_PKEY *pkey, const EVP_MD *md,
                      STACK_OF(OPENSSL_STRING) *sigopts);
diff --git a/apps/lib/apps.c b/apps/lib/apps.c
index 457dac87bc..6ae35bac73 100644
--- a/apps/lib/apps.c
+++ b/apps/lib/apps.c
@@ -1982,12 +1982,41 @@ static int do_sign_init(EVP_MD_CTX *ctx, EVP_PKEY *pkey,
         && do_pkey_ctx_init(pkctx, sigopts);
 }
 
-/* Ensure RFC 5280 compliance and then sign the certificate info */
+static int adapt_keyid_ext(X509 *cert, X509V3_CTX *ext_ctx,
+                           const char *name, const char *value, int add_default)
+{
+    const STACK_OF(X509_EXTENSION) *exts = X509_get0_extensions(cert);
+    X509_EXTENSION *new_ext = X509V3_EXT_nconf(NULL, ext_ctx, name, value);
+    int idx, rv = 0;
+
+    if (new_ext == NULL)
+        return rv;
+
+    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" */
+
+        if (disabled) {
+            X509_delete_ext(cert, idx);
+            X509_EXTENSION_free(found_ext);
+        } /* else keep existing key identifier, which might be outdated */
+        rv = 1;
+    } else  {
+        rv = !add_default || X509_add_ext(cert, new_ext, -1);
+    }
+    X509_EXTENSION_free(new_ext);
+    return rv;
+}
+
+/* Ensure RFC 5280 compliance, adapt keyIDs as needed, and sign the cert info */
 int do_X509_sign(X509 *cert, EVP_PKEY *pkey, const EVP_MD *md,
-                 STACK_OF(OPENSSL_STRING) *sigopts)
+                 STACK_OF(OPENSSL_STRING) *sigopts, X509V3_CTX *ext_ctx)
 {
     const STACK_OF(X509_EXTENSION) *exts = X509_get0_extensions(cert);
     EVP_MD_CTX *mctx = EVP_MD_CTX_new();
+    int self_sign;
     int rv = 0;
 
     if (sk_X509_EXTENSION_num(exts /* may be NULL */) > 0) {
@@ -1995,6 +2024,21 @@ int do_X509_sign(X509 *cert, EVP_PKEY *pkey, const EVP_MD *md,
         if (!X509_set_version(cert, 2)) /* Make sure cert is X509 v3 */
             goto end;
 
+        /*
+         * Add default SKID before such that default 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();
+        if (!adapt_keyid_ext(cert, ext_ctx, "authorityKeyIdentifier",
+                             "keyid, issuer", !self_sign))
+            goto end;
+
         /* TODO any further measures for ensuring default RFC 5280 compliance */
     }
 
diff --git a/apps/req.c b/apps/req.c
index acd0cd09cb..5a065ad843 100644
--- a/apps/req.c
+++ b/apps/req.c
@@ -796,7 +796,7 @@ int req_main(int argc, char **argv)
                 }
             }
 
-            i = do_X509_sign(x509ss, pkey, digest, sigopts);
+            i = do_X509_sign(x509ss, pkey, digest, sigopts, &ext_ctx);
             if (!i) {
                 ERR_print_errors(bio_err);
                 goto end;
diff --git a/apps/x509.c b/apps/x509.c
index c8fcb7a7ae..34d654c8f2 100644
--- a/apps/x509.c
+++ b/apps/x509.c
@@ -1067,6 +1067,8 @@ static int sign(X509 *x, EVP_PKEY *pkey, X509 *issuer,
                 const EVP_MD *digest, CONF *conf, const char *section,
                 int preserve_dates)
 {
+    X509V3_CTX ext_ctx;
+
     if (!X509_set_issuer_name(x, X509_get_subject_name(issuer)))
         return 0;
 
@@ -1077,10 +1079,8 @@ static int sign(X509 *x, EVP_PKEY *pkey, X509 *issuer,
         while (X509_get_ext_count(x) > 0)
             X509_delete_ext(x, 0);
     }
+    X509V3_set_ctx(&ext_ctx, issuer, x, NULL, NULL, X509V3_CTX_REPLACE);
     if (conf != NULL) {
-        X509V3_CTX ext_ctx;
-
-        X509V3_set_ctx(&ext_ctx, issuer, x, NULL, NULL, X509V3_CTX_REPLACE);
         X509V3_set_nconf(&ext_ctx, conf);
         if (!X509V3_EXT_add_nconf(conf, &ext_ctx, section, x)) {
             BIO_printf(bio_err,
@@ -1088,7 +1088,7 @@ static int sign(X509 *x, EVP_PKEY *pkey, X509 *issuer,
             return 0;
         }
     }
-    return do_X509_sign(x, pkey, digest, sigopts);
+    return do_X509_sign(x, pkey, digest, sigopts, &ext_ctx);
 }
 
 static int purpose_print(BIO *bio, X509 *cert, X509_PURPOSE *pt)