From a7e4ca5b4e1932cb91ea21047403c87a033e524a Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Tue, 9 Jun 2020 10:21:58 +0200 Subject: [PATCH] Add warning to key/param generating apps on potential delay due to missing entropy This also introduces app_keygen() and app_paramgen() and cleans up err reporting. Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/12095) --- apps/dgst.c | 105 ++++++++++++++++++-------------------------- apps/dhparam.c | 6 +-- apps/dsaparam.c | 11 ++--- apps/gendsa.c | 7 +-- apps/genpkey.c | 19 ++------ apps/genrsa.c | 8 +--- apps/include/apps.h | 3 ++ apps/lib/apps.c | 36 ++++++++++++++- apps/req.c | 29 ++++-------- 9 files changed, 99 insertions(+), 125 deletions(-) diff --git a/apps/dgst.c b/apps/dgst.c index 7ac1013303..0fa668511a 100644 --- a/apps/dgst.c +++ b/apps/dgst.c @@ -106,7 +106,7 @@ int dgst_main(int argc, char **argv) const char *md_name = NULL; OPTION_CHOICE o; int separator = 0, debug = 0, keyform = FORMAT_UNDEF, siglen = 0; - int i, ret = 1, out_bin = -1, want_pub = 0, do_verify = 0; + int i, ret = EXIT_FAILURE, out_bin = -1, want_pub = 0, do_verify = 0; int xoflen = 0; unsigned char *buf = NULL, *sigbuf = NULL; int engine_impl = 0; @@ -125,7 +125,7 @@ int dgst_main(int argc, char **argv) goto end; case OPT_HELP: opt_help(dgst_options); - ret = 0; + ret = EXIT_SUCCESS; goto end; case OPT_LIST: BIO_printf(bio_out, "Supported digests:\n"); @@ -134,7 +134,7 @@ int dgst_main(int argc, char **argv) OBJ_NAME_do_all_sorted(OBJ_NAME_TYPE_MD_METH, show_digests, &dec); BIO_printf(bio_out, "\n"); - ret = 0; + ret = EXIT_SUCCESS; goto end; case OPT_C: separator = 1; @@ -244,10 +244,8 @@ int dgst_main(int argc, char **argv) in = BIO_new(BIO_s_file()); bmd = BIO_new(BIO_f_md()); - if ((in == NULL) || (bmd == NULL)) { - ERR_print_errors(bio_err); + if (in == NULL || bmd == NULL) goto end; - } if (debug) { BIO_set_callback(in, BIO_debug_callback); @@ -272,7 +270,7 @@ int dgst_main(int argc, char **argv) goto end; if ((!(mac_name == NULL) + !(keyfile == NULL) + !(hmac_key == NULL)) > 1) { - BIO_printf(bio_err, "MAC and Signing key cannot both be specified\n"); + BIO_printf(bio_err, "MAC and signing key cannot both be specified\n"); goto end; } @@ -302,31 +300,24 @@ int dgst_main(int argc, char **argv) if (mac_name != NULL) { EVP_PKEY_CTX *mac_ctx = NULL; - int r = 0; + if (!init_gen_str(&mac_ctx, mac_name, impl, 0, NULL, NULL)) - goto mac_end; + goto end; if (macopts != NULL) { - char *macopt; for (i = 0; i < sk_OPENSSL_STRING_num(macopts); i++) { - macopt = sk_OPENSSL_STRING_value(macopts, i); + char *macopt = sk_OPENSSL_STRING_value(macopts, i); + if (pkey_ctrl_string(mac_ctx, macopt) <= 0) { - BIO_printf(bio_err, - "MAC parameter error \"%s\"\n", macopt); - ERR_print_errors(bio_err); - goto mac_end; + EVP_PKEY_CTX_free(mac_ctx); + BIO_printf(bio_err, "MAC parameter error \"%s\"\n", macopt); + goto end; } } } - if (EVP_PKEY_keygen(mac_ctx, &sigkey) <= 0) { - BIO_puts(bio_err, "Error generating key\n"); - ERR_print_errors(bio_err); - goto mac_end; - } - r = 1; - mac_end: + + sigkey = app_keygen(mac_ctx, mac_name, 0, 0 /* not verbose */); + /* Verbose output would make external-tests gost-engine fail */ EVP_PKEY_CTX_free(mac_ctx); - if (r == 0) - goto end; } if (hmac_key != NULL) { @@ -342,28 +333,27 @@ int dgst_main(int argc, char **argv) if (sigkey != NULL) { EVP_MD_CTX *mctx = NULL; EVP_PKEY_CTX *pctx = NULL; - int r; + int res; + if (!BIO_get_md_ctx(bmd, &mctx)) { BIO_printf(bio_err, "Error getting context\n"); - ERR_print_errors(bio_err); goto end; } if (do_verify) - r = EVP_DigestVerifyInit(mctx, &pctx, md, impl, sigkey); + res = EVP_DigestVerifyInit(mctx, &pctx, md, impl, sigkey); else - r = EVP_DigestSignInit(mctx, &pctx, md, impl, sigkey); - if (!r) { + res = EVP_DigestSignInit(mctx, &pctx, md, impl, sigkey); + if (res == 0) { BIO_printf(bio_err, "Error setting context\n"); - ERR_print_errors(bio_err); goto end; } if (sigopts != NULL) { - char *sigopt; for (i = 0; i < sk_OPENSSL_STRING_num(sigopts); i++) { - sigopt = sk_OPENSSL_STRING_value(sigopts, i); + char *sigopt = sk_OPENSSL_STRING_value(sigopts, i); + if (pkey_ctrl_string(pctx, sigopt) <= 0) { - BIO_printf(bio_err, "parameter error \"%s\"\n", sigopt); - ERR_print_errors(bio_err); + BIO_printf(bio_err, "Signature parameter error \"%s\"\n", + sigopt); goto end; } } @@ -374,23 +364,21 @@ int dgst_main(int argc, char **argv) EVP_MD_CTX *mctx = NULL; if (!BIO_get_md_ctx(bmd, &mctx)) { BIO_printf(bio_err, "Error getting context\n"); - ERR_print_errors(bio_err); goto end; } if (md == NULL) md = (EVP_MD *)EVP_sha256(); if (!EVP_DigestInit_ex(mctx, md, impl)) { BIO_printf(bio_err, "Error setting digest\n"); - ERR_print_errors(bio_err); goto end; } } if (sigfile != NULL && sigkey != NULL) { BIO *sigbio = BIO_new_file(sigfile, "rb"); + if (sigbio == NULL) { BIO_printf(bio_err, "Error opening signature file %s\n", sigfile); - ERR_print_errors(bio_err); goto end; } siglen = EVP_PKEY_size(sigkey); @@ -399,7 +387,6 @@ int dgst_main(int argc, char **argv) BIO_free(sigbio); if (siglen <= 0) { BIO_printf(bio_err, "Error reading signature file %s\n", sigfile); - ERR_print_errors(bio_err); goto end; } } @@ -431,27 +418,28 @@ int dgst_main(int argc, char **argv) siglen, NULL, md_name, "stdin"); } else { const char *sig_name = NULL; - if (!out_bin) { + + if (out_bin == 0) { if (sigkey != NULL) sig_name = EVP_PKEY_get0_type_name(sigkey); } - ret = 0; + ret = EXIT_SUCCESS; for (i = 0; i < argc; i++) { - int r; if (BIO_read_filename(in, argv[i]) <= 0) { perror(argv[i]); - ret++; + ret = EXIT_FAILURE; continue; } else { - r = do_fp(out, buf, inp, separator, out_bin, xoflen, - sigkey, sigbuf, siglen, sig_name, md_name, argv[i]); + if (do_fp(out, buf, inp, separator, out_bin, xoflen, + sigkey, sigbuf, siglen, sig_name, md_name, argv[i])) + ret = EXIT_FAILURE; } - if (r) - ret = r; (void)BIO_reset(bmd); } } end: + if (ret != EXIT_SUCCESS) + ERR_print_errors(bio_err); OPENSSL_clear_free(buf, BUFSIZE); BIO_free(in); OPENSSL_free(passin); @@ -537,14 +525,13 @@ int do_fp(BIO *out, unsigned char *buf, BIO *bp, int sep, int binout, int xoflen const char *file) { size_t len = BUFSIZE; - int i, backslash = 0, ret = 1; + int i, backslash = 0, ret = EXIT_FAILURE; unsigned char *allocated_buf = NULL; while (BIO_pending(bp) || !BIO_eof(bp)) { i = BIO_read(bp, (char *)buf, BUFSIZE); if (i < 0) { - BIO_printf(bio_err, "Read Error in %s\n", file); - ERR_print_errors(bio_err); + BIO_printf(bio_err, "Read error in %s\n", file); goto end; } if (i == 0) @@ -557,14 +544,13 @@ int do_fp(BIO *out, unsigned char *buf, BIO *bp, int sep, int binout, int xoflen if (i > 0) { BIO_printf(out, "Verified OK\n"); } else if (i == 0) { - BIO_printf(out, "Verification Failure\n"); + BIO_printf(out, "Verification failure\n"); goto end; } else { - BIO_printf(bio_err, "Error Verifying Data\n"); - ERR_print_errors(bio_err); + BIO_printf(bio_err, "Error verifying data\n"); goto end; } - ret = 0; + ret = EXIT_SUCCESS; goto end; } if (key != NULL) { @@ -573,8 +559,7 @@ int do_fp(BIO *out, unsigned char *buf, BIO *bp, int sep, int binout, int xoflen BIO_get_md_ctx(bp, &ctx); if (!EVP_DigestSignFinal(ctx, NULL, &tmplen)) { - BIO_printf(bio_err, "Error Signing Data\n"); - ERR_print_errors(bio_err); + BIO_printf(bio_err, "Error getting maximum length of signed data\n"); goto end; } if (tmplen > BUFSIZE) { @@ -583,8 +568,7 @@ int do_fp(BIO *out, unsigned char *buf, BIO *bp, int sep, int binout, int xoflen buf = allocated_buf; } if (!EVP_DigestSignFinal(ctx, buf, &len)) { - BIO_printf(bio_err, "Error Signing Data\n"); - ERR_print_errors(bio_err); + BIO_printf(bio_err, "Error signing data\n"); goto end; } } else if (xoflen > 0) { @@ -600,15 +584,12 @@ int do_fp(BIO *out, unsigned char *buf, BIO *bp, int sep, int binout, int xoflen if (!EVP_DigestFinalXOF(ctx, buf, len)) { BIO_printf(bio_err, "Error Digesting Data\n"); - ERR_print_errors(bio_err); goto end; } } else { len = BIO_gets(bp, (char *)buf, BUFSIZE); - if ((int)len < 0) { - ERR_print_errors(bio_err); + if ((int)len < 0) goto end; - } } if (binout) { @@ -643,7 +624,7 @@ int do_fp(BIO *out, unsigned char *buf, BIO *bp, int sep, int binout, int xoflen BIO_printf(out, "\n"); } - ret = 0; + ret = EXIT_SUCCESS; end: if (allocated_buf != NULL) OPENSSL_clear_free(allocated_buf, len); diff --git a/apps/dhparam.c b/apps/dhparam.c index 5bb4b7f04a..ba3119b2ce 100644 --- a/apps/dhparam.c +++ b/apps/dhparam.c @@ -217,11 +217,7 @@ int dhparam_main(int argc, char **argv) } } - if (!EVP_PKEY_paramgen(ctx, &tmppkey)) { - BIO_printf(bio_err, "Error, %s generation failed\n", alg); - goto end; - } - + tmppkey = app_paramgen(ctx, alg); EVP_PKEY_CTX_free(ctx); ctx = NULL; if (dsaparam) { diff --git a/apps/dsaparam.c b/apps/dsaparam.c index d7fb736b98..df98532459 100644 --- a/apps/dsaparam.c +++ b/apps/dsaparam.c @@ -11,6 +11,7 @@ #include #include +#include "apps.h" #include #include #include "apps.h" @@ -176,10 +177,7 @@ int dsaparam_main(int argc, char **argv) "Error, DSA key generation setting bit length failed\n"); goto end; } - if (EVP_PKEY_paramgen(ctx, ¶ms) <= 0) { - BIO_printf(bio_err, "Error, DSA key generation failed\n"); - goto end; - } + params = app_paramgen(ctx, "DSA"); } else { params = load_keyparams(infile, informat, 1, "DSA", "DSA parameters"); } @@ -218,10 +216,7 @@ int dsaparam_main(int argc, char **argv) "Error, unable to initialise for key generation\n"); goto end; } - if (!EVP_PKEY_keygen(ctx, &pkey)) { - BIO_printf(bio_err, "Error, unable to generate key\n"); - goto end; - } + pkey = app_keygen(ctx, "DSA", numbits, verbose); assert(private); if (outformat == FORMAT_ASN1) i = i2d_PrivateKey_bio(out, pkey); diff --git a/apps/gendsa.c b/apps/gendsa.c index f4bd0fe09e..a7857c478c 100644 --- a/apps/gendsa.c +++ b/apps/gendsa.c @@ -145,12 +145,7 @@ int gendsa_main(int argc, char **argv) BIO_printf(bio_err, "unable to set up for key generation\n"); goto end; } - if (verbose) - BIO_printf(bio_err, "Generating DSA key, %d bits\n", nbits); - if (EVP_PKEY_keygen(ctx, &pkey) <= 0) { - BIO_printf(bio_err, "unable to generate key\n"); - goto end; - } + pkey = app_keygen(ctx, "DSA", nbits, verbose); assert(private); if (!PEM_write_bio_PrivateKey(out, pkey, enc, NULL, 0, NULL, passout)) { diff --git a/apps/genpkey.c b/apps/genpkey.c index c187cc2a70..38b1100658 100644 --- a/apps/genpkey.c +++ b/apps/genpkey.c @@ -188,19 +188,8 @@ int genpkey_main(int argc, char **argv) EVP_PKEY_CTX_set_cb(ctx, genpkey_cb); EVP_PKEY_CTX_set_app_data(ctx, bio_err); - if (do_param) { - if (EVP_PKEY_paramgen(ctx, &pkey) <= 0) { - BIO_puts(bio_err, "Error generating parameters\n"); - ERR_print_errors(bio_err); - goto end; - } - } else { - if (EVP_PKEY_keygen(ctx, &pkey) <= 0) { - BIO_puts(bio_err, "Error generating key\n"); - ERR_print_errors(bio_err); - goto end; - } - } + pkey = do_param ? app_paramgen(ctx, algname) + : app_keygen(ctx, algname, 0, 0 /* not verbose */); if (do_param) { rv = PEM_write_bio_Parameters(out, pkey); @@ -219,7 +208,6 @@ int genpkey_main(int argc, char **argv) if (rv <= 0) { BIO_puts(bio_err, "Error writing key\n"); - ERR_print_errors(bio_err); ret = 1; } @@ -231,13 +219,14 @@ int genpkey_main(int argc, char **argv) if (rv <= 0) { BIO_puts(bio_err, "Error printing key\n"); - ERR_print_errors(bio_err); ret = 1; } } end: sk_OPENSSL_STRING_free(keyopt); + if (ret != 0) + ERR_print_errors(bio_err); EVP_PKEY_free(pkey); EVP_PKEY_CTX_free(ctx); EVP_CIPHER_free(cipher); diff --git a/apps/genrsa.c b/apps/genrsa.c index 0e84687b32..e709ea38ce 100644 --- a/apps/genrsa.c +++ b/apps/genrsa.c @@ -201,13 +201,7 @@ opthelp: BIO_printf(bio_err, "Error setting number of primes\n"); goto end; } - if (verbose) - BIO_printf(bio_err, "Generating RSA private key, %d bit long modulus (%d primes)\n", - num, primes); - if (!EVP_PKEY_keygen(ctx, &pkey)) { - BIO_printf(bio_err, "Error generating RSA key\n"); - goto end; - } + pkey = app_keygen(ctx, "RSA", num, verbose); if (verbose) { BIGNUM *e = NULL; diff --git a/apps/include/apps.h b/apps/include/apps.h index 829c49e34e..47a5f7f6f4 100644 --- a/apps/include/apps.h +++ b/apps/include/apps.h @@ -334,6 +334,9 @@ void app_params_free(OSSL_PARAM *params); int app_provider_load(OSSL_LIB_CTX *libctx, const char *provider_name); void app_providers_cleanup(void); +EVP_PKEY *app_keygen(EVP_PKEY_CTX *ctx, const char *alg, int bits, int verbose); +EVP_PKEY *app_paramgen(EVP_PKEY_CTX *ctx, const char *alg); + OSSL_LIB_CTX *app_get0_libctx(void); int app_set_propq(const char *arg); const char *app_get0_propq(void); diff --git a/apps/lib/apps.c b/apps/lib/apps.c index fa63410359..12a17fceed 100644 --- a/apps/lib/apps.c +++ b/apps/lib/apps.c @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -629,7 +630,7 @@ void app_bail_out(char *fmt, ...) BIO_vprintf(bio_err, fmt, args); va_end(args); ERR_print_errors(bio_err); - exit(1); + exit(EXIT_FAILURE); } void *app_malloc(size_t sz, const char *what) @@ -3258,3 +3259,36 @@ void app_params_free(OSSL_PARAM *params) OPENSSL_free(params); } } + +EVP_PKEY *app_keygen(EVP_PKEY_CTX *ctx, const char *alg, int bits, int verbose) +{ + EVP_PKEY *res = NULL; + + if (verbose && alg != NULL) { + BIO_printf(bio_err, "Generating %s key", alg); + if (bits > 0) + BIO_printf(bio_err, " with %d bits\n", bits); + else + BIO_printf(bio_err, "\n"); + } + if (!RAND_status()) + BIO_printf(bio_err, "Warning: generating random key material may take a long time\n" + "if the system has a poor entropy source\n"); + if (EVP_PKEY_keygen(ctx, &res) <= 0) + app_bail_out("%s: Error generating %s key\n", opt_getprog(), + alg != NULL ? alg : "asymmetric"); + return res; +} + +EVP_PKEY *app_paramgen(EVP_PKEY_CTX *ctx, const char *alg) +{ + EVP_PKEY *res = NULL; + + if (!RAND_status()) + BIO_printf(bio_err, "Warning: generating random key parameters may take a long time\n" + "if the system has a poor entropy source\n"); + if (EVP_PKEY_paramgen(ctx, &res) <= 0) + app_bail_out("%s: Generating %s key parameters failed\n", + opt_getprog(), alg != NULL ? alg : "asymmetric"); + return res; +} diff --git a/apps/req.c b/apps/req.c index 11222cb397..67cefa7e87 100644 --- a/apps/req.c +++ b/apps/req.c @@ -511,15 +511,12 @@ int req_main(int argc, char **argv) if (p == NULL) ERR_clear_error(); if (p != NULL) { - BIO *oid_bio; + BIO *oid_bio = BIO_new_file(p, "r"); - oid_bio = BIO_new_file(p, "r"); if (oid_bio == NULL) { - if (verbose) { + if (verbose) BIO_printf(bio_err, "Problems opening '%s' for extra OIDs\n", p); - ERR_print_errors(bio_err); - } } else { OBJ_create_objects(oid_bio); BIO_free(oid_bio); @@ -627,9 +624,8 @@ int req_main(int argc, char **argv) if (newreq && pkey == NULL) { app_RAND_load_conf(req_conf, section); - if (!NCONF_get_number(req_conf, section, BITS, &newkey_len)) { + if (!NCONF_get_number(req_conf, section, BITS, &newkey_len)) newkey_len = DEFAULT_KEY_LENGTH; - } genctx = set_keygen_ctx(keyalg, &keyalgstr, &newkey_len, gen_eng); if (genctx == NULL) @@ -639,8 +635,7 @@ int req_main(int argc, char **argv) && (EVP_PKEY_CTX_is_a(genctx, "RSA") || EVP_PKEY_CTX_is_a(genctx, "RSA-PSS") || EVP_PKEY_CTX_is_a(genctx, "DSA"))) { - BIO_printf(bio_err, "Private key length is too short,\n"); - BIO_printf(bio_err, "it needs to be at least %d bits, not %ld.\n", + BIO_printf(bio_err, "Private key length too short, needs to be at least %d bits, not %ld.\n", MIN_KEY_LENGTH, newkey_len); goto end; } @@ -673,15 +668,10 @@ int req_main(int argc, char **argv) } } - BIO_printf(bio_err, "Generating a %s private key\n", keyalgstr); - EVP_PKEY_CTX_set_cb(genctx, genpkey_cb); EVP_PKEY_CTX_set_app_data(genctx, bio_err); - if (EVP_PKEY_keygen(genctx, &pkey) <= 0) { - BIO_puts(bio_err, "Error generating key\n"); - goto end; - } + pkey = app_keygen(genctx, keyalgstr, newkey_len, verbose); EVP_PKEY_CTX_free(genctx); genctx = NULL; @@ -927,14 +917,12 @@ int req_main(int argc, char **argv) i = do_X509_REQ_verify(req, tpubkey, vfyopts); - if (i < 0) { + if (i < 0) goto end; - } else if (i == 0) { + if (i == 0) BIO_printf(bio_err, "Certificate request self-signature verify failure\n"); - ERR_print_errors(bio_err); - } else { /* i > 0 */ + else /* i > 0 */ BIO_printf(bio_err, "Certificate request self-signature verify OK\n"); - } } if (noout && !text && !modulus && !subject && !pubkey) { @@ -1389,7 +1377,6 @@ static int add_attribute_object(X509_REQ *req, char *text, const char *def, if (!X509_REQ_add1_attr_by_NID(req, nid, chtype, (unsigned char *)buf, -1)) { BIO_printf(bio_err, "Error adding attribute\n"); - ERR_print_errors(bio_err); ret = 0; } -- GitLab