diff options
author | Dr. David von Oheimb <David.von.Oheimb@siemens.com> | 2020-12-07 19:37:46 +0100 |
---|---|---|
committer | Dr. David von Oheimb <David.von.Oheimb@siemens.com> | 2020-12-10 15:19:55 +0100 |
commit | 1a683b80dc9ad4dcbf206a0617364a9d614a9883 (patch) | |
tree | 489d4cc0bfbb0664cd692e95ab0c175aa8b3ebd3 /apps | |
parent | 98ba251fe6f49fc2ee310f6e559c3431922fa16d (diff) |
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)
Diffstat (limited to 'apps')
-rwxr-xr-x | apps/ca.c | 61 | ||||
-rw-r--r-- | apps/req.c | 16 | ||||
-rw-r--r-- | apps/x509.c | 40 |
3 files changed, 63 insertions, 54 deletions
@@ -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 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 msie_hack = 0; @@ -761,7 +761,7 @@ end_of_options: /*****************************************************************/ /* Read extensions config file */ if (extfile) { - if ((extconf = app_load_config(extfile)) == NULL) { + if ((extfile_conf = app_load_config(extfile)) == NULL) { ret = 1; goto end; } @@ -772,7 +772,7 @@ end_of_options: /* We can have sections in the ext file */ if (extensions == NULL) { - extensions = NCONF_get_string(extconf, "default", "extensions"); + extensions = NCONF_get_string(extfile_conf, "default", "extensions"); if (extensions == NULL) extensions = "default"; } @@ -836,7 +836,20 @@ end_of_options: 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 * configuration file @@ -847,13 +860,13 @@ end_of_options: ERR_clear_error(); } if (extensions != NULL) { - /* Check syntax of file */ + /* Check syntax of config file section */ X509V3_CTX ctx; X509V3_set_ctx_test(&ctx); X509V3_set_nconf(&ctx, conf); if (!X509V3_EXT_add_nconf(conf, &ctx, extensions, NULL)) { BIO_printf(bio_err, - "Error Loading extension section %s\n", + "Error checking certificate extension config section %s\n", extensions); ret = 1; goto end; @@ -1131,7 +1144,7 @@ end_of_options: X509V3_set_nconf(&ctx, conf); if (!X509V3_EXT_add_nconf(conf, &ctx, crl_ext, NULL)) { BIO_printf(bio_err, - "Error Loading CRL extension section %s\n", crl_ext); + "Error checking CRL extension section %s\n", crl_ext); ret = 1; goto end; } @@ -1220,8 +1233,11 @@ end_of_options: X509V3_set_nconf(&crlctx, conf); 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; + } if (crlnumberfile != NULL) { tmpser = BN_to_ASN1_INTEGER(crlnumber, NULL); if (!tmpser) @@ -1312,7 +1328,7 @@ end_of_options: X509_free(x509); X509_CRL_free(crl); NCONF_free(conf); - NCONF_free(extconf); + NCONF_free(extfile_conf); release_engine(e); 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 */ if (ext_sect) { - X509V3_CTX ctx; + X509V3_CTX ext_ctx; /* Initialize the context structure */ - if (selfsign) - X509V3_set_ctx(&ctx, ret, ret, req, NULL, 0); - else - X509V3_set_ctx(&ctx, x509, ret, req, NULL, 0); + X509V3_set_ctx(&ext_ctx, selfsign ? ret : x509, + ret, req, NULL, X509V3_CTX_REPLACE); - if (extconf != NULL) { + if (extfile_conf != NULL) { if (verbose) BIO_printf(bio_err, "Extra configuration file found\n"); - /* Use the extconf configuration db LHASH */ - X509V3_set_nconf(&ctx, extconf); - - /* Test the structure (needed?) */ - /* X509V3_set_ctx_test(&ctx); */ + /* Use the extfile_conf configuration db LHASH */ + X509V3_set_nconf(&ext_ctx, extfile_conf); /* 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, - "ERROR: adding extensions in section %s\n", + "Error adding certificate extensions from extfile section %s\n", ext_sect); goto end; } @@ -1711,11 +1722,11 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509, "Successfully added extensions from file.\n"); } else if (ext_sect) { /* 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, - "ERROR: adding extensions in section %s\n", + "Error adding certificate extensions from config section %s\n", ext_sect); goto end; } diff --git a/apps/req.c b/apps/req.c index bc23c7d3a5..ad79866a5a 100644 --- a/apps/req.c +++ b/apps/req.c @@ -525,7 +525,7 @@ int req_main(int argc, char **argv) X509V3_set_nconf(&ctx, req_conf); if (!X509V3_EXT_add_nconf(req_conf, &ctx, extensions, NULL)) { BIO_printf(bio_err, - "Error loading extension section %s\n", extensions); + "Error checking x509 extension section %s\n", extensions); goto end; } } @@ -535,7 +535,7 @@ int req_main(int argc, char **argv) X509V3_set_ctx_test(&ctx); X509V3_set_nconf(&ctx, addext_conf); 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; } } @@ -583,7 +583,7 @@ int req_main(int argc, char **argv) X509V3_set_nconf(&ctx, req_conf); if (!X509V3_EXT_add_nconf(req_conf, &ctx, req_exts, NULL)) { BIO_printf(bio_err, - "Error loading request extension section %s\n", + "Error checking request extension section %s\n", req_exts); goto end; } @@ -769,21 +769,21 @@ int req_main(int argc, char **argv) /* 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); /* Add extensions */ if (extensions != NULL && !X509V3_EXT_add_nconf(req_conf, &ext_ctx, extensions, x509ss)) { - BIO_printf(bio_err, "Error loading extension section %s\n", + BIO_printf(bio_err, "Error adding x509 extensions from section %s\n", extensions); goto end; } if (addext_conf != NULL && !X509V3_EXT_add_nconf(addext_conf, &ext_ctx, "default", 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; } @@ -813,14 +813,14 @@ int req_main(int argc, char **argv) if (req_exts != NULL && !X509V3_EXT_REQ_add_nconf(req_conf, &ext_ctx, 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); goto end; } if (addext_conf != NULL && !X509V3_EXT_REQ_add_nconf(addext_conf, &ext_ctx, "default", 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; } i = do_X509_REQ_sign(req, pkey, digest, sigopts); diff --git a/apps/x509.c b/apps/x509.c index 42ef448416..8cd84f5afe 100644 --- a/apps/x509.c +++ b/apps/x509.c @@ -465,7 +465,7 @@ int x509_main(int argc, char **argv) goto opthelp; checkoffset = (time_t)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()); goto opthelp; } @@ -536,11 +536,11 @@ int x509_main(int argc, char **argv) CAkeyfile = CAfile; } else if (CA_flag && CAkeyfile == NULL) { 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; } else if (!CA_flag && CAkeyfile != NULL) { 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) { @@ -558,7 +558,7 @@ int x509_main(int argc, char **argv) X509V3_set_nconf(&ctx2, extconf); if (!X509V3_EXT_add_nconf(extconf, &ctx2, extsect, NULL)) { BIO_printf(bio_err, - "Error Loading extension section %s\n", extsect); + "Error checking extension section %s\n", extsect); ERR_print_errors(bio_err); goto end; } @@ -572,7 +572,7 @@ int x509_main(int argc, char **argv) goto end; 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; } i = do_X509_REQ_verify(req, pkey, vfyopts); @@ -807,7 +807,7 @@ int x509_main(int argc, char **argv) fdig = EVP_sha1(); 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; } BIO_printf(out, "%s Fingerprint=", @@ -820,7 +820,6 @@ int x509_main(int argc, char **argv) /* should be in the library */ else if (sign_flag == i && x509req == 0) { - BIO_printf(bio_err, "Getting Private key\n"); if (Upkey == NULL) { Upkey = load_key(keyfile, keyformat, 0, passin, e, "private key"); @@ -835,7 +834,6 @@ int x509_main(int argc, char **argv) goto end; } } else if (CA_flag == i) { - BIO_printf(bio_err, "Getting CA Private Key\n"); if (CAkeyfile != NULL) { CApkey = load_key(CAkeyfile, CAkeyformat, 0, passin, e, "CA private key"); @@ -851,9 +849,8 @@ int x509_main(int argc, char **argv) } else if (x509req == i) { EVP_PKEY *pk; - BIO_printf(bio_err, "Getting request Private Key\n"); 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; } else { pk = load_key(keyfile, keyformat, 0, @@ -862,8 +859,6 @@ int x509_main(int argc, char **argv) goto end; } - BIO_printf(bio_err, "Generating certificate request\n"); - rq = X509_to_X509_REQ(x, pk, digest); EVP_PKEY_free(pk); if (rq == NULL) { @@ -911,11 +906,11 @@ int x509_main(int argc, char **argv) else i = PEM_write_bio_X509(out, x); } 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; } 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); goto end; } @@ -965,7 +960,7 @@ static ASN1_INTEGER *x509_load_serial(const char *CAfile, goto end; 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; } @@ -1059,13 +1054,13 @@ static int callb(int ok, X509_STORE_CTX *ctx) */ if (ok) { 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; } else { err_cert = X509_STORE_CTX_get_current_cert(ctx); print_name(bio_err, NULL, X509_get_subject_name(err_cert), 0); 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_verify_cert_error_string(err)); return 1; @@ -1089,12 +1084,15 @@ static int sign(X509 *x, EVP_PKEY *pkey, X509 *issuer, X509_delete_ext(x, 0); } if (conf != NULL) { - X509V3_CTX ctx; + X509V3_CTX ext_ctx; - X509V3_set_ctx(&ctx, issuer, x, NULL, NULL, 0); - X509V3_set_nconf(&ctx, conf); - if (!X509V3_EXT_add_nconf(conf, &ctx, section, x)) + 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, + "Error adding extensions from section %s\n", section); return 0; + } } return do_X509_sign(x, pkey, digest, sigopts); } |