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)
diff --git a/apps/ca.c b/apps/ca.c index 2488361..1e77bf5 100644 --- a/apps/ca.c +++ b/apps/ca.c
@@ -1709,7 +1709,16 @@ /* 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 9d5db16..6018a83 100644 --- a/apps/include/apps.h +++ b/apps/include/apps.h
@@ -247,6 +247,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 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 b15abac..82eeaea 100644 --- a/apps/lib/apps.c +++ b/apps/lib/apps.c
@@ -2224,8 +2224,8 @@ 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 @@ 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 @@ 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 dcb173f..acc45c4 100644 --- a/apps/pkcs12.c +++ b/apps/pkcs12.c
@@ -555,7 +555,7 @@ /* 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 7997ea7..274f839 100644 --- a/apps/req.c +++ b/apps/req.c
@@ -838,11 +838,9 @@ 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 b88fb4f..ff95821 100644 --- a/apps/x509.c +++ b/apps/x509.c
@@ -810,6 +810,10 @@ 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))
diff --git a/crypto/x509/v3_akid.c b/crypto/x509/v3_akid.c index 43b515f..59ea439 100644 --- a/crypto/x509/v3_akid.c +++ b/crypto/x509/v3_akid.c
@@ -161,8 +161,13 @@ */ 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)) + && !(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) { /* generate fallback AKID, emulating s2i_skey_id(..., "hash") */ X509_PUBKEY *pubkey = NULL; @@ -171,15 +176,13 @@ ikeyid = ossl_x509_pubkey_hash(pubkey); X509_PUBKEY_free(pubkey); } - if ((keyid == 2 || issuer == 0) - && (ikeyid == NULL - || ASN1_STRING_length(ikeyid) <= 2) /* indicating "none" */) { + if (keyid == 2 && ikeyid == NULL) { ERR_raise(ERR_LIB_X509V3, X509V3_R_UNABLE_TO_GET_ISSUER_KEYID); goto err; } } - if (issuer == 2 || (issuer == 1 && ikeyid == NULL)) { + if (issuer == 2 || (issuer == 1 && !ss && 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) {
diff --git a/crypto/x509/v3_conf.c b/crypto/x509/v3_conf.c index 1c11d67..b95c652 100644 --- a/crypto/x509/v3_conf.c +++ b/crypto/x509/v3_conf.c
@@ -311,13 +311,27 @@ { X509_EXTENSION *ext; STACK_OF(CONF_VALUE) *nval; - CONF_VALUE *val; - int i; + const CONF_VALUE *val; + int i, akid = -1, skid = -1; if ((nval = NCONF_get_section(conf, section)) == NULL) return 0; for (i = 0; i < sk_CONF_VALUE_num(nval); i++) { val = sk_CONF_VALUE_value(nval, i); + if (strcmp(val->name, "authorityKeyIdentifier") == 0) + akid = i; + else if (strcmp(val->name, "subjectKeyIdentifier") == 0) + skid = i; + } + for (i = 0; i < sk_CONF_VALUE_num(nval); i++) { + val = sk_CONF_VALUE_value(nval, i); + if (skid > akid && akid >= 0) { + /* make sure SKID is handled before AKID */ + if (i == akid) + val = sk_CONF_VALUE_value(nval, skid); + else if (i == skid) + val = sk_CONF_VALUE_value(nval, akid); + } if ((ext = X509V3_EXT_nconf_int(conf, ctx, val->section, val->name, val->value)) == NULL) return 0;
diff --git a/doc/man5/x509v3_config.pod b/doc/man5/x509v3_config.pod index 2a3afee..0114b45 100644 --- a/doc/man5/x509v3_config.pod +++ b/doc/man5/x509v3_config.pod
@@ -208,6 +208,7 @@ If B<issuer> is present, and in addition it has the option B<always> specified or B<keyid> is not present, then the issuer DN and serial number are copied from the issuer certificate. +If this fails, an error is returned. Examples: