From ef0449135c4e4e7f04bbeafbd76ce7b5c0518088 Mon Sep 17 00:00:00 2001 From: Shane Lontis Date: Wed, 9 Jun 2021 17:34:55 +1000 Subject: [PATCH 1/1] Fix s_server app to not report an error when using a non DH certificate. Fixes #15071 It always tries loading the cert as DH which previously did not produce an error. The errors are not suppressed for these operations. The output now matches previous versions of OpenSSL. Reviewed-by: Paul Dale Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/15670) --- apps/include/apps.h | 3 ++ apps/lib/apps.c | 76 +++++++++++++++++++++++++++++++++------------ apps/s_server.c | 10 +++--- 3 files changed, 66 insertions(+), 23 deletions(-) diff --git a/apps/include/apps.h b/apps/include/apps.h index 47a5f7f6f4..c9f77f6067 100644 --- a/apps/include/apps.h +++ b/apps/include/apps.h @@ -121,6 +121,9 @@ EVP_PKEY *load_pubkey(const char *uri, int format, int maybe_stdin, const char *pass, ENGINE *e, const char *desc); EVP_PKEY *load_keyparams(const char *uri, int format, int maybe_stdin, const char *keytype, const char *desc); +EVP_PKEY *load_keyparams_suppress(const char *uri, int format, int maybe_stdin, + const char *keytype, const char *desc, + int suppress_decode_errors); char *next_item(char *opt); /* in list separated by comma and/or space */ int load_cert_certs(const char *uri, X509 **pcert, STACK_OF(X509) **pcerts, diff --git a/apps/lib/apps.c b/apps/lib/apps.c index 8604c75251..9aae725fc6 100644 --- a/apps/lib/apps.c +++ b/apps/lib/apps.c @@ -75,6 +75,14 @@ static int set_table_opts(unsigned long *flags, const char *arg, const NAME_EX_TBL * in_tbl); static int set_multi_opts(unsigned long *flags, const char *arg, const NAME_EX_TBL * in_tbl); +static +int load_key_certs_crls_suppress(const char *uri, int format, int maybe_stdin, + const char *pass, const char *desc, + EVP_PKEY **ppkey, EVP_PKEY **ppubkey, + EVP_PKEY **pparams, + X509 **pcert, STACK_OF(X509) **pcerts, + X509_CRL **pcrl, STACK_OF(X509_CRL) **pcrls, + int suppress_decode_errors); int app_init(long mesgwin); @@ -605,27 +613,37 @@ EVP_PKEY *load_pubkey(const char *uri, int format, int maybe_stdin, return pkey; } -EVP_PKEY *load_keyparams(const char *uri, int format, int maybe_stdin, - const char *keytype, const char *desc) +EVP_PKEY *load_keyparams_suppress(const char *uri, int format, int maybe_stdin, + const char *keytype, const char *desc, + int suppress_decode_errors) { EVP_PKEY *params = NULL; if (desc == NULL) desc = "key parameters"; - (void)load_key_certs_crls(uri, format, maybe_stdin, NULL, desc, - NULL, NULL, ¶ms, NULL, NULL, NULL, NULL); + (void)load_key_certs_crls_suppress(uri, format, maybe_stdin, NULL, desc, + NULL, NULL, ¶ms, NULL, NULL, NULL, + NULL, suppress_decode_errors); if (params != NULL && keytype != NULL && !EVP_PKEY_is_a(params, keytype)) { - BIO_printf(bio_err, - "Unable to load %s from %s (unexpected parameters type)\n", - desc, uri); - ERR_print_errors(bio_err); + if (!suppress_decode_errors) { + BIO_printf(bio_err, + "Unable to load %s from %s (unexpected parameters type)\n", + desc, uri); + ERR_print_errors(bio_err); + } EVP_PKEY_free(params); params = NULL; } return params; } +EVP_PKEY *load_keyparams(const char *uri, int format, int maybe_stdin, + const char *keytype, const char *desc) +{ + return load_keyparams_suppress(uri, format, maybe_stdin, keytype, desc, 0); +} + void app_bail_out(char *fmt, ...) { va_list args; @@ -866,12 +884,14 @@ static const char *format2string(int format) * In any case (also on error) the caller is responsible for freeing all members * of *pcerts and *pcrls (as far as they are not NULL). */ -int load_key_certs_crls(const char *uri, int format, int maybe_stdin, - const char *pass, const char *desc, - EVP_PKEY **ppkey, EVP_PKEY **ppubkey, - EVP_PKEY **pparams, - X509 **pcert, STACK_OF(X509) **pcerts, - X509_CRL **pcrl, STACK_OF(X509_CRL) **pcrls) +static +int load_key_certs_crls_suppress(const char *uri, int format, int maybe_stdin, + const char *pass, const char *desc, + EVP_PKEY **ppkey, EVP_PKEY **ppubkey, + EVP_PKEY **pparams, + X509 **pcert, STACK_OF(X509) **pcerts, + X509_CRL **pcrl, STACK_OF(X509_CRL) **pcrls, + int suppress_decode_errors) { PW_CB_DATA uidata; OSSL_STORE_CTX *ctx = NULL; @@ -890,6 +910,9 @@ int load_key_certs_crls(const char *uri, int format, int maybe_stdin, OSSL_PARAM itp[2]; const OSSL_PARAM *params = NULL; + if (suppress_decode_errors) + ERR_set_mark(); + if (ppkey != NULL) { *ppkey = NULL; cnt_expectations++; @@ -1074,12 +1097,14 @@ int load_key_certs_crls(const char *uri, int format, int maybe_stdin, any = 1; failed = "CRL"; } - if (failed != NULL) - BIO_printf(bio_err, "Could not read"); - if (any) - BIO_printf(bio_err, " any"); + if (!suppress_decode_errors) { + if (failed != NULL) + BIO_printf(bio_err, "Could not read"); + if (any) + BIO_printf(bio_err, " any"); + } } - if (failed != NULL) { + if (!suppress_decode_errors && failed != NULL) { if (desc != NULL && strstr(desc, failed) != NULL) { BIO_printf(bio_err, " %s", desc); } else { @@ -1092,9 +1117,22 @@ int load_key_certs_crls(const char *uri, int format, int maybe_stdin, BIO_printf(bio_err, "\n"); ERR_print_errors(bio_err); } + if (suppress_decode_errors) + ERR_pop_to_mark(); return failed == NULL; } +int load_key_certs_crls(const char *uri, int format, int maybe_stdin, + const char *pass, const char *desc, + EVP_PKEY **ppkey, EVP_PKEY **ppubkey, + EVP_PKEY **pparams, + X509 **pcert, STACK_OF(X509) **pcerts, + X509_CRL **pcrl, STACK_OF(X509_CRL) **pcrls) +{ + return load_key_certs_crls_suppress(uri, format, maybe_stdin, pass, desc, + ppkey, ppubkey, pparams, pcert, pcerts, + pcrl, pcrls, 0); +} #define X509V3_EXT_UNKNOWN_MASK (0xfL << 16) /* Return error for unknown extensions */ diff --git a/apps/s_server.c b/apps/s_server.c index 644fe1a905..009ac5a1eb 100644 --- a/apps/s_server.c +++ b/apps/s_server.c @@ -2003,7 +2003,8 @@ int s_server_main(int argc, char *argv[]) if (dhfile != NULL) dhpkey = load_keyparams(dhfile, FORMAT_UNDEF, 0, "DH", "DH parameters"); else if (s_cert_file != NULL) - dhpkey = load_keyparams(s_cert_file, FORMAT_UNDEF, 0, "DH", "DH parameters"); + dhpkey = load_keyparams_suppress(s_cert_file, FORMAT_UNDEF, 0, "DH", + "DH parameters", 1); if (dhpkey != NULL) { BIO_printf(bio_s_out, "Setting temp DH parameters\n"); @@ -2035,9 +2036,10 @@ int s_server_main(int argc, char *argv[]) if (ctx2 != NULL) { if (dhfile != NULL) { - EVP_PKEY *dhpkey2 = load_keyparams(s_cert_file2, FORMAT_UNDEF, - 0, "DH", - "DH parameters"); + EVP_PKEY *dhpkey2 = load_keyparams_suppress(s_cert_file2, + FORMAT_UNDEF, + 0, "DH", + "DH parameters", 1); if (dhpkey2 != NULL) { BIO_printf(bio_s_out, "Setting temp DH parameters\n"); -- 2.34.1