From 9bf1061c44c81059102cd4749f6078b6ce71da9d Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Fri, 27 Aug 2021 11:34:23 +0200 Subject: [PATCH] APPS/x509: Fix generation of AKID via v2i_AUTHORITY_KEYID() Fixes #16300 Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/16442) --- apps/x509.c | 7 ++++++- crypto/x509/v3_akid.c | 25 +++++++++++++++++++------ doc/man5/x509v3_config.pod | 11 +++++++---- test/recipes/25-test_req.t | 2 +- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/apps/x509.c b/apps/x509.c index 1f8a157c0e..b88fb4f5ea 100644 --- a/apps/x509.c +++ b/apps/x509.c @@ -822,7 +822,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 or B or both of them, separated by C<,>. Either or both can have the option B, indicated by putting a colon C<:> between the value and this option. +For self-signed certificates the AKID is suppressed unless B is present. By default the B, B, and B apps behave as if "none" was given for self-signed certificates and "keyid, issuer" otherwise. -If B 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 is present, an error is returned. +If B 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 is present but no value can be obtained, an error is returned. If B is present, and in addition it has the option B specified or B 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); -- GitLab