diff --git a/crypto/x509/v3_akid.c b/crypto/x509/v3_akid.c index 08c751b77cf..d30b8c46884 100644 --- a/crypto/x509/v3_akid.c +++ b/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 self_signed = 0; AUTHORITY_KEYID *akeyid = AUTHORITY_KEYID_new(); if (akeyid == NULL) @@ -156,36 +156,36 @@ 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(); + + if (ctx->subject_cert != NULL && ctx->issuer_pkey != NULL) { + ERR_set_mark(); + self_signed = X509_check_private_key(ctx->subject_cert, + ctx->issuer_pkey); + ERR_pop_to_mark(); + } /* unless forced with "always", AKID is suppressed for self-signed certs */ - if (keyid == 2 || (keyid == 1 && !ss)) { + if (keyid == 2 || (keyid == 1 && !self_signed)) { /* * 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 (ctx->subject_cert == issuer_cert && 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); @@ -193,16 +193,13 @@ static AUTHORITY_KEYID *v2i_AUTHORITY_KEYID(X509V3_EXT_METHOD *method, } } - if (issuer == 2 || (issuer == 1 && !ss && ikeyid == NULL)) { + if (issuer == 2 || (issuer == 1 && ikeyid == NULL && !self_signed)) { isname = X509_NAME_dup(X509_get_issuer_name(issuer_cert)); serial = ASN1_INTEGER_dup(X509_get0_serialNumber(issuer_cert)); if (isname == NULL || serial == NULL) { ERR_raise(ERR_LIB_X509V3, X509V3_R_UNABLE_TO_GET_ISSUER_DETAILS); goto err; } - } - - if (isname != NULL) { if ((gens = sk_GENERAL_NAME_new_null()) == NULL || (gen = GENERAL_NAME_new()) == NULL) { ERR_raise(ERR_LIB_X509V3, ERR_R_ASN1_LIB); @@ -217,8 +214,6 @@ static AUTHORITY_KEYID *v2i_AUTHORITY_KEYID(X509V3_EXT_METHOD *method, } akeyid->issuer = gens; - gen = NULL; - gens = NULL; akeyid->serial = serial; akeyid->keyid = ikeyid; diff --git a/doc/man5/x509v3_config.pod b/doc/man5/x509v3_config.pod index 781891966e3..e942ee1cbbf 100644 --- a/doc/man5/x509v3_config.pod +++ b/doc/man5/x509v3_config.pod @@ -211,13 +211,17 @@ 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. +In order to find out, if the certificate is about to be self-signed, the +private key as given by B is compared to the +public key in the subject_certificate. +When there is no private key available, the AKID will not be suppressed. By default the B, B, and B apps behave as if B was given for self-signed certificates and BC<,> B otherwise. 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 issuer certificate does not have the subject key identifier extension. 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. diff --git a/test/x509_test.c b/test/x509_test.c index 00b7c2e55e8..4ea4285d7d2 100644 --- a/test/x509_test.c +++ b/test/x509_test.c @@ -10,6 +10,7 @@ #define OPENSSL_SUPPRESS_DEPRECATED /* EVP_PKEY_get1/set1_RSA */ #include +#include #include #include #include @@ -284,6 +285,83 @@ err: return ret; } +static int test_x509_old_abi_compat_mode_extension(void) +{ + static const unsigned char commonName[] = "test"; + X509 *x = NULL; + X509_NAME *subject = NULL; + X509_NAME_ENTRY *name_entry = NULL; + X509_EXTENSION *ext = NULL; + X509V3_CTX ctx; + const STACK_OF(X509_EXTENSION) *exts; + ASN1_OCTET_STRING *encoded; + int ret = 0; + + if (!TEST_ptr(x = X509_new()) + || !TEST_int_eq(X509_set_version(x, X509_VERSION_3), 1) + || !TEST_int_eq(ASN1_INTEGER_set(X509_get_serialNumber(x), 1), 1) + || !TEST_ptr(subject = X509_NAME_new())) + goto err; + + name_entry = X509_NAME_ENTRY_create_by_NID(NULL, NID_commonName, + MBSTRING_ASC, commonName, -1); + if (!TEST_ptr(name_entry) + || !TEST_int_eq(X509_NAME_add_entry(subject, name_entry, -1, 0), 1) + || !TEST_int_eq(X509_set_subject_name(x, subject), 1) + || !TEST_int_eq(X509_set_issuer_name(x, subject), 1) + || !TEST_ptr(X509_gmtime_adj(X509_getm_notBefore(x), 0)) + || !TEST_ptr(X509_gmtime_adj(X509_getm_notAfter(x), 24 * 3600)) + || !TEST_int_eq(X509_set_pubkey(x, pubkey), 1)) + goto err; + + X509V3_set_ctx(&ctx, x, x, NULL, NULL, 0); + /* + * By not calling X509V3_set_issuer_pkey(&ctx, privkey); + * here, we are exempt from the 3.x convention, regarding + * the authorityKeyIdentifier=keyid,issuer which may return + * an empty extension for self signed certificates. + * Therefore an empty extension cannot be produced here. + */ + if (!TEST_ptr(ext = X509V3_EXT_conf(NULL, &ctx, "subjectKeyIdentifier", + "hash")) + || !TEST_int_eq(X509_add_ext(x, ext, -1), 1)) + goto err; + + X509_EXTENSION_free(ext); + if (!TEST_ptr(ext = X509V3_EXT_conf(NULL, &ctx, "authorityKeyIdentifier", + "keyid,issuer")) + || !TEST_int_eq(X509_add_ext(x, ext, -1), 1)) + goto err; + + if (!TEST_int_gt(X509_sign(x, privkey, signmd), 0) + || !TEST_int_gt(X509_verify(x, pubkey), 0)) + goto err; + + exts = X509_get0_extensions(x); + if (!TEST_ptr(exts) + || !TEST_int_eq(sk_X509_EXTENSION_num(exts), 2)) + goto err; + + encoded = X509_EXTENSION_get_data(X509v3_get_ext(exts, 0)); + if (!TEST_ptr(encoded) + || !TEST_int_gt(ASN1_STRING_length(encoded), 2)) + goto err; + + encoded = X509_EXTENSION_get_data(X509v3_get_ext(exts, 1)); + if (!TEST_ptr(encoded) + || !TEST_int_gt(ASN1_STRING_length(encoded), 2)) + goto err; + + ret = 1; + +err: + X509_NAME_ENTRY_free(name_entry); + X509_NAME_free(subject); + X509_EXTENSION_free(ext); + X509_free(x); + return ret; +} + OPT_TEST_DECLARE_USAGE("\n") int setup_tests(void) @@ -321,6 +399,7 @@ int setup_tests(void) ADD_TEST(test_x509_delete_last_extension); ADD_TEST(test_x509_crl_delete_last_extension); ADD_TEST(test_x509_revoked_delete_last_extension); + ADD_TEST(test_x509_old_abi_compat_mode_extension); return 1; }