apps/{ca,req,x509}.c: Improve diag and doc mostly on X.509 extensions, fix multiple instances

This includes a general correction in the code (now using the X509V3_CTX_REPLACE flag)
and adding a prominent clarification in the documentation:

    If multiple entries are processed for the same extension name,
    later entries override earlier ones with the same name.

This is due to an RFC 5280 requirement - the intro of its section 4.2 says:

    A certificate MUST NOT include more than one instance of a particular extension.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13614)
This commit is contained in:
Dr. David von Oheimb 2020-12-07 19:37:46 +01:00
parent 98ba251fe6
commit 1a683b80dc
5 changed files with 70 additions and 57 deletions

View File

@ -138,7 +138,7 @@ static int make_revoked(X509_REVOKED *rev, const char *str);
static int old_entry_print(const ASN1_OBJECT *obj, const ASN1_STRING *str); static int old_entry_print(const ASN1_OBJECT *obj, const ASN1_STRING *str);
static void write_new_certificate(BIO *bp, X509 *x, int output_der, int notext); static void write_new_certificate(BIO *bp, X509 *x, int output_der, int notext);
static CONF *extconf = NULL; static CONF *extfile_conf = NULL;
static int preserve = 0; static int preserve = 0;
static int msie_hack = 0; static int msie_hack = 0;
@ -761,7 +761,7 @@ end_of_options:
/*****************************************************************/ /*****************************************************************/
/* Read extensions config file */ /* Read extensions config file */
if (extfile) { if (extfile) {
if ((extconf = app_load_config(extfile)) == NULL) { if ((extfile_conf = app_load_config(extfile)) == NULL) {
ret = 1; ret = 1;
goto end; goto end;
} }
@ -772,7 +772,7 @@ end_of_options:
/* We can have sections in the ext file */ /* We can have sections in the ext file */
if (extensions == NULL) { if (extensions == NULL) {
extensions = NCONF_get_string(extconf, "default", "extensions"); extensions = NCONF_get_string(extfile_conf, "default", "extensions");
if (extensions == NULL) if (extensions == NULL)
extensions = "default"; extensions = "default";
} }
@ -836,7 +836,20 @@ end_of_options:
goto end; goto end;
} }
if (extconf == NULL) { if (extfile_conf != NULL) {
/* Check syntax of extfile */
X509V3_CTX ctx;
X509V3_set_ctx_test(&ctx);
X509V3_set_nconf(&ctx, extfile_conf);
if (!X509V3_EXT_add_nconf(extfile_conf, &ctx, extensions, NULL)) {
BIO_printf(bio_err,
"Error checking certificate extensions from extfile section %s\n",
extensions);
ret = 1;
goto end;
}
} else {
/* /*
* no '-extfile' option, so we look for extensions in the main * no '-extfile' option, so we look for extensions in the main
* configuration file * configuration file
@ -847,13 +860,13 @@ end_of_options:
ERR_clear_error(); ERR_clear_error();
} }
if (extensions != NULL) { if (extensions != NULL) {
/* Check syntax of file */ /* Check syntax of config file section */
X509V3_CTX ctx; X509V3_CTX ctx;
X509V3_set_ctx_test(&ctx); X509V3_set_ctx_test(&ctx);
X509V3_set_nconf(&ctx, conf); X509V3_set_nconf(&ctx, conf);
if (!X509V3_EXT_add_nconf(conf, &ctx, extensions, NULL)) { if (!X509V3_EXT_add_nconf(conf, &ctx, extensions, NULL)) {
BIO_printf(bio_err, BIO_printf(bio_err,
"Error Loading extension section %s\n", "Error checking certificate extension config section %s\n",
extensions); extensions);
ret = 1; ret = 1;
goto end; goto end;
@ -1131,7 +1144,7 @@ end_of_options:
X509V3_set_nconf(&ctx, conf); X509V3_set_nconf(&ctx, conf);
if (!X509V3_EXT_add_nconf(conf, &ctx, crl_ext, NULL)) { if (!X509V3_EXT_add_nconf(conf, &ctx, crl_ext, NULL)) {
BIO_printf(bio_err, BIO_printf(bio_err,
"Error Loading CRL extension section %s\n", crl_ext); "Error checking CRL extension section %s\n", crl_ext);
ret = 1; ret = 1;
goto end; goto end;
} }
@ -1220,8 +1233,11 @@ end_of_options:
X509V3_set_nconf(&crlctx, conf); X509V3_set_nconf(&crlctx, conf);
if (crl_ext != NULL) if (crl_ext != NULL)
if (!X509V3_EXT_CRL_add_nconf(conf, &crlctx, crl_ext, crl)) if (!X509V3_EXT_CRL_add_nconf(conf, &crlctx, crl_ext, crl)) {
BIO_printf(bio_err,
"Error adding CRL extensions from section %s\n", crl_ext);
goto end; goto end;
}
if (crlnumberfile != NULL) { if (crlnumberfile != NULL) {
tmpser = BN_to_ASN1_INTEGER(crlnumber, NULL); tmpser = BN_to_ASN1_INTEGER(crlnumber, NULL);
if (!tmpser) if (!tmpser)
@ -1312,7 +1328,7 @@ end_of_options:
X509_free(x509); X509_free(x509);
X509_CRL_free(crl); X509_CRL_free(crl);
NCONF_free(conf); NCONF_free(conf);
NCONF_free(extconf); NCONF_free(extfile_conf);
release_engine(e); release_engine(e);
return ret; return ret;
} }
@ -1681,28 +1697,23 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
/* Lets add the extensions, if there are any */ /* Lets add the extensions, if there are any */
if (ext_sect) { if (ext_sect) {
X509V3_CTX ctx; X509V3_CTX ext_ctx;
/* Initialize the context structure */ /* Initialize the context structure */
if (selfsign) X509V3_set_ctx(&ext_ctx, selfsign ? ret : x509,
X509V3_set_ctx(&ctx, ret, ret, req, NULL, 0); ret, req, NULL, X509V3_CTX_REPLACE);
else
X509V3_set_ctx(&ctx, x509, ret, req, NULL, 0);
if (extconf != NULL) { if (extfile_conf != NULL) {
if (verbose) if (verbose)
BIO_printf(bio_err, "Extra configuration file found\n"); BIO_printf(bio_err, "Extra configuration file found\n");
/* Use the extconf configuration db LHASH */ /* Use the extfile_conf configuration db LHASH */
X509V3_set_nconf(&ctx, extconf); X509V3_set_nconf(&ext_ctx, extfile_conf);
/* Test the structure (needed?) */
/* X509V3_set_ctx_test(&ctx); */
/* Adds exts contained in the configuration file */ /* Adds exts contained in the configuration file */
if (!X509V3_EXT_add_nconf(extconf, &ctx, ext_sect, ret)) { if (!X509V3_EXT_add_nconf(extfile_conf, &ext_ctx, ext_sect, ret)) {
BIO_printf(bio_err, BIO_printf(bio_err,
"ERROR: adding extensions in section %s\n", "Error adding certificate extensions from extfile section %s\n",
ext_sect); ext_sect);
goto end; goto end;
} }
@ -1711,11 +1722,11 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
"Successfully added extensions from file.\n"); "Successfully added extensions from file.\n");
} else if (ext_sect) { } else if (ext_sect) {
/* We found extensions to be set from config file */ /* We found extensions to be set from config file */
X509V3_set_nconf(&ctx, lconf); X509V3_set_nconf(&ext_ctx, lconf);
if (!X509V3_EXT_add_nconf(lconf, &ctx, ext_sect, ret)) { if (!X509V3_EXT_add_nconf(lconf, &ext_ctx, ext_sect, ret)) {
BIO_printf(bio_err, BIO_printf(bio_err,
"ERROR: adding extensions in section %s\n", "Error adding certificate extensions from config section %s\n",
ext_sect); ext_sect);
goto end; goto end;
} }

View File

@ -525,7 +525,7 @@ int req_main(int argc, char **argv)
X509V3_set_nconf(&ctx, req_conf); X509V3_set_nconf(&ctx, req_conf);
if (!X509V3_EXT_add_nconf(req_conf, &ctx, extensions, NULL)) { if (!X509V3_EXT_add_nconf(req_conf, &ctx, extensions, NULL)) {
BIO_printf(bio_err, BIO_printf(bio_err,
"Error loading extension section %s\n", extensions); "Error checking x509 extension section %s\n", extensions);
goto end; goto end;
} }
} }
@ -535,7 +535,7 @@ int req_main(int argc, char **argv)
X509V3_set_ctx_test(&ctx); X509V3_set_ctx_test(&ctx);
X509V3_set_nconf(&ctx, addext_conf); X509V3_set_nconf(&ctx, addext_conf);
if (!X509V3_EXT_add_nconf(addext_conf, &ctx, "default", NULL)) { if (!X509V3_EXT_add_nconf(addext_conf, &ctx, "default", NULL)) {
BIO_printf(bio_err, "Error loading extensions defined using -addext\n"); BIO_printf(bio_err, "Error checking extensions defined using -addext\n");
goto end; goto end;
} }
} }
@ -583,7 +583,7 @@ int req_main(int argc, char **argv)
X509V3_set_nconf(&ctx, req_conf); X509V3_set_nconf(&ctx, req_conf);
if (!X509V3_EXT_add_nconf(req_conf, &ctx, req_exts, NULL)) { if (!X509V3_EXT_add_nconf(req_conf, &ctx, req_exts, NULL)) {
BIO_printf(bio_err, BIO_printf(bio_err,
"Error loading request extension section %s\n", "Error checking request extension section %s\n",
req_exts); req_exts);
goto end; goto end;
} }
@ -769,21 +769,21 @@ int req_main(int argc, char **argv)
/* Set up V3 context struct */ /* Set up V3 context struct */
X509V3_set_ctx(&ext_ctx, x509ss, x509ss, NULL, NULL, 0); X509V3_set_ctx(&ext_ctx, x509ss, x509ss, NULL, NULL, X509V3_CTX_REPLACE);
X509V3_set_nconf(&ext_ctx, req_conf); X509V3_set_nconf(&ext_ctx, req_conf);
/* Add extensions */ /* Add extensions */
if (extensions != NULL && !X509V3_EXT_add_nconf(req_conf, if (extensions != NULL && !X509V3_EXT_add_nconf(req_conf,
&ext_ctx, extensions, &ext_ctx, extensions,
x509ss)) { x509ss)) {
BIO_printf(bio_err, "Error loading extension section %s\n", BIO_printf(bio_err, "Error adding x509 extensions from section %s\n",
extensions); extensions);
goto end; goto end;
} }
if (addext_conf != NULL if (addext_conf != NULL
&& !X509V3_EXT_add_nconf(addext_conf, &ext_ctx, "default", && !X509V3_EXT_add_nconf(addext_conf, &ext_ctx, "default",
x509ss)) { x509ss)) {
BIO_printf(bio_err, "Error loading extensions defined via -addext\n"); BIO_printf(bio_err, "Error adding extensions defined via -addext\n");
goto end; goto end;
} }
@ -813,14 +813,14 @@ int req_main(int argc, char **argv)
if (req_exts != NULL if (req_exts != NULL
&& !X509V3_EXT_REQ_add_nconf(req_conf, &ext_ctx, && !X509V3_EXT_REQ_add_nconf(req_conf, &ext_ctx,
req_exts, req)) { req_exts, req)) {
BIO_printf(bio_err, "Error loading extension section %s\n", BIO_printf(bio_err, "Error adding request extensions from section %s\n",
req_exts); req_exts);
goto end; goto end;
} }
if (addext_conf != NULL if (addext_conf != NULL
&& !X509V3_EXT_REQ_add_nconf(addext_conf, &ext_ctx, "default", && !X509V3_EXT_REQ_add_nconf(addext_conf, &ext_ctx, "default",
req)) { req)) {
BIO_printf(bio_err, "Error loading extensions defined via -addext\n"); BIO_printf(bio_err, "Error adding extensions defined via -addext\n");
goto end; goto end;
} }
i = do_X509_REQ_sign(req, pkey, digest, sigopts); i = do_X509_REQ_sign(req, pkey, digest, sigopts);

View File

@ -465,7 +465,7 @@ int x509_main(int argc, char **argv)
goto opthelp; goto opthelp;
checkoffset = (time_t)temp; checkoffset = (time_t)temp;
if ((intmax_t)checkoffset != temp) { if ((intmax_t)checkoffset != temp) {
BIO_printf(bio_err, "%s: checkend time out of range %s\n", BIO_printf(bio_err, "%s: Checkend time out of range %s\n",
prog, opt_arg()); prog, opt_arg());
goto opthelp; goto opthelp;
} }
@ -536,11 +536,11 @@ int x509_main(int argc, char **argv)
CAkeyfile = CAfile; CAkeyfile = CAfile;
} else if (CA_flag && CAkeyfile == NULL) { } else if (CA_flag && CAkeyfile == NULL) {
BIO_printf(bio_err, BIO_printf(bio_err,
"need to specify a CAkey if using the CA command\n"); "Need to specify a CAkey if using the CA command\n");
goto end; goto end;
} else if (!CA_flag && CAkeyfile != NULL) { } else if (!CA_flag && CAkeyfile != NULL) {
BIO_printf(bio_err, BIO_printf(bio_err,
"ignoring -CAkey option since no -CA option is given\n"); "Ignoring -CAkey option since no -CA option is given\n");
} }
if (extfile != NULL) { if (extfile != NULL) {
@ -558,7 +558,7 @@ int x509_main(int argc, char **argv)
X509V3_set_nconf(&ctx2, extconf); X509V3_set_nconf(&ctx2, extconf);
if (!X509V3_EXT_add_nconf(extconf, &ctx2, extsect, NULL)) { if (!X509V3_EXT_add_nconf(extconf, &ctx2, extsect, NULL)) {
BIO_printf(bio_err, BIO_printf(bio_err,
"Error Loading extension section %s\n", extsect); "Error checking extension section %s\n", extsect);
ERR_print_errors(bio_err); ERR_print_errors(bio_err);
goto end; goto end;
} }
@ -572,7 +572,7 @@ int x509_main(int argc, char **argv)
goto end; goto end;
if ((pkey = X509_REQ_get0_pubkey(req)) == NULL) { if ((pkey = X509_REQ_get0_pubkey(req)) == NULL) {
BIO_printf(bio_err, "error unpacking public key\n"); BIO_printf(bio_err, "Error unpacking public key\n");
goto end; goto end;
} }
i = do_X509_REQ_verify(req, pkey, vfyopts); i = do_X509_REQ_verify(req, pkey, vfyopts);
@ -807,7 +807,7 @@ int x509_main(int argc, char **argv)
fdig = EVP_sha1(); fdig = EVP_sha1();
if (!X509_digest(x, fdig, md, &n)) { if (!X509_digest(x, fdig, md, &n)) {
BIO_printf(bio_err, "out of memory\n"); BIO_printf(bio_err, "Out of memory\n");
goto end; goto end;
} }
BIO_printf(out, "%s Fingerprint=", BIO_printf(out, "%s Fingerprint=",
@ -820,7 +820,6 @@ int x509_main(int argc, char **argv)
/* should be in the library */ /* should be in the library */
else if (sign_flag == i && x509req == 0) { else if (sign_flag == i && x509req == 0) {
BIO_printf(bio_err, "Getting Private key\n");
if (Upkey == NULL) { if (Upkey == NULL) {
Upkey = load_key(keyfile, keyformat, 0, Upkey = load_key(keyfile, keyformat, 0,
passin, e, "private key"); passin, e, "private key");
@ -835,7 +834,6 @@ int x509_main(int argc, char **argv)
goto end; goto end;
} }
} else if (CA_flag == i) { } else if (CA_flag == i) {
BIO_printf(bio_err, "Getting CA Private Key\n");
if (CAkeyfile != NULL) { if (CAkeyfile != NULL) {
CApkey = load_key(CAkeyfile, CAkeyformat, CApkey = load_key(CAkeyfile, CAkeyformat,
0, passin, e, "CA private key"); 0, passin, e, "CA private key");
@ -851,9 +849,8 @@ int x509_main(int argc, char **argv)
} else if (x509req == i) { } else if (x509req == i) {
EVP_PKEY *pk; EVP_PKEY *pk;
BIO_printf(bio_err, "Getting request Private Key\n");
if (keyfile == NULL) { if (keyfile == NULL) {
BIO_printf(bio_err, "no request key file specified\n"); BIO_printf(bio_err, "No request key file specified\n");
goto end; goto end;
} else { } else {
pk = load_key(keyfile, keyformat, 0, pk = load_key(keyfile, keyformat, 0,
@ -862,8 +859,6 @@ int x509_main(int argc, char **argv)
goto end; goto end;
} }
BIO_printf(bio_err, "Generating certificate request\n");
rq = X509_to_X509_REQ(x, pk, digest); rq = X509_to_X509_REQ(x, pk, digest);
EVP_PKEY_free(pk); EVP_PKEY_free(pk);
if (rq == NULL) { if (rq == NULL) {
@ -911,11 +906,11 @@ int x509_main(int argc, char **argv)
else else
i = PEM_write_bio_X509(out, x); i = PEM_write_bio_X509(out, x);
} else { } else {
BIO_printf(bio_err, "bad output format specified for outfile\n"); BIO_printf(bio_err, "Bad output format specified for outfile\n");
goto end; goto end;
} }
if (!i) { if (!i) {
BIO_printf(bio_err, "unable to write certificate\n"); BIO_printf(bio_err, "Unable to write certificate\n");
ERR_print_errors(bio_err); ERR_print_errors(bio_err);
goto end; goto end;
} }
@ -965,7 +960,7 @@ static ASN1_INTEGER *x509_load_serial(const char *CAfile,
goto end; goto end;
if (!BN_add_word(serial, 1)) { if (!BN_add_word(serial, 1)) {
BIO_printf(bio_err, "add_word failure\n"); BIO_printf(bio_err, "Serial number increment failure\n");
goto end; goto end;
} }
@ -1059,13 +1054,13 @@ static int callb(int ok, X509_STORE_CTX *ctx)
*/ */
if (ok) { if (ok) {
BIO_printf(bio_err, BIO_printf(bio_err,
"error with certificate to be certified - should be self-signed\n"); "Error with certificate to be certified - should be self-signed\n");
return 0; return 0;
} else { } else {
err_cert = X509_STORE_CTX_get_current_cert(ctx); err_cert = X509_STORE_CTX_get_current_cert(ctx);
print_name(bio_err, NULL, X509_get_subject_name(err_cert), 0); print_name(bio_err, NULL, X509_get_subject_name(err_cert), 0);
BIO_printf(bio_err, BIO_printf(bio_err,
"error with certificate - error %d at depth %d\n%s\n", err, "Error with certificate - error %d at depth %d\n%s\n", err,
X509_STORE_CTX_get_error_depth(ctx), X509_STORE_CTX_get_error_depth(ctx),
X509_verify_cert_error_string(err)); X509_verify_cert_error_string(err));
return 1; return 1;
@ -1089,12 +1084,15 @@ static int sign(X509 *x, EVP_PKEY *pkey, X509 *issuer,
X509_delete_ext(x, 0); X509_delete_ext(x, 0);
} }
if (conf != NULL) { if (conf != NULL) {
X509V3_CTX ctx; X509V3_CTX ext_ctx;
X509V3_set_ctx(&ctx, issuer, x, NULL, NULL, 0); X509V3_set_ctx(&ext_ctx, issuer, x, NULL, NULL, X509V3_CTX_REPLACE);
X509V3_set_nconf(&ctx, conf); X509V3_set_nconf(&ext_ctx, conf);
if (!X509V3_EXT_add_nconf(conf, &ctx, section, x)) if (!X509V3_EXT_add_nconf(conf, &ext_ctx, section, x)) {
BIO_printf(bio_err,
"Error adding extensions from section %s\n", section);
return 0; return 0;
}
} }
return do_X509_sign(x, pkey, digest, sigopts); return do_X509_sign(x, pkey, digest, sigopts);
} }

View File

@ -7,8 +7,9 @@ x509v3_config - X509 V3 certificate extension configuration format
=head1 DESCRIPTION =head1 DESCRIPTION
Several OpenSSL commands can add extensions to a certificate or Several OpenSSL commands can add extensions to a certificate or
certificate request based on the contents of a configuration file. certificate request based on the contents of a configuration file
The syntax of this file is described in L<config(5)>. and CLI options such as B<-addext>.
The syntax of configuration files is described in L<config(5)>.
The commands typically have an option to specify the name of the configuration The commands typically have an option to specify the name of the configuration
file, and a section within that file; see the documentation of the file, and a section within that file; see the documentation of the
individual command for details. individual command for details.
@ -22,6 +23,9 @@ Each entry in the extension section takes the form:
If B<critical> is present then the extension will be marked as critical. If B<critical> is present then the extension will be marked as critical.
If multiple entries are processed for the same extension name,
later entries override earlier ones with the same name.
The format of B<values> depends on the value of B<name>, many have a The format of B<values> depends on the value of B<name>, many have a
type-value pairing where the type and value are separated by a colon. type-value pairing where the type and value are separated by a colon.
There are four main types of extension: There are four main types of extension:

View File

@ -126,7 +126,7 @@ sub test_errors { # actually tests diagnostics of OSSL_STORE
# 3 tests for non-existence of spurious OSSL_STORE ASN.1 parse error output. # 3 tests for non-existence of spurious OSSL_STORE ASN.1 parse error output.
# This requires provoking a failure exit of the app after reading input files. # This requires provoking a failure exit of the app after reading input files.
ok(test_errors("bad output format", "root-cert.pem", '-outform', 'http'), ok(test_errors("Bad output format", "root-cert.pem", '-outform', 'http'),
"load root-cert errors"); "load root-cert errors");
ok(test_errors("RC2-40-CBC", "v3-certs-RC2.p12", '-passin', 'pass:v3-certs'), ok(test_errors("RC2-40-CBC", "v3-certs-RC2.p12", '-passin', 'pass:v3-certs'),
"load v3-certs-RC2 no asn1 errors"); # error msg should mention "RC2-40-CBC" "load v3-certs-RC2 no asn1 errors"); # error msg should mention "RC2-40-CBC"