The branch master has been updated
via 6e2499474cb96b28a51df1da25cc72f1cf342fad (commit)
via 7c64ca71c2ceeb1d47e8499bd351de7d0078ce37 (commit)
from d4d8f163db1d32c98d8f956e6966263a7a22fac1 (commit)
- Log -----------------------------------------------------------------
commit 6e2499474cb96b28a51df1da25cc72f1cf342fad
Author: Dr. David von Oheimb <david.von.ohe...@siemens.com>
Date: Fri Aug 27 18:36:38 2021 +0200
APPS load_key_certs_crls(): Make file access errors much more readable
This reverts part of commit ef0449135c4e4e7f using a less invasive
suppression.
Reviewed-by: Paul Dale <pa...@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16452)
commit 7c64ca71c2ceeb1d47e8499bd351de7d0078ce37
Author: Dr. David von Oheimb <david.von.ohe...@siemens.com>
Date: Fri Aug 27 18:33:56 2021 +0200
OSSL_STORE_open_ex(): Prevent spurious error: unregistered scheme=file
Reviewed-by: Paul Dale <pa...@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16452)
-----------------------------------------------------------------------
Summary of changes:
apps/lib/apps.c | 143 ++++++++++++++++++++++-------------------------
crypto/store/store_lib.c | 4 ++
2 files changed, 72 insertions(+), 75 deletions(-)
diff --git a/apps/lib/apps.c b/apps/lib/apps.c
index 3b0266f158..6c3f3aee00 100644
--- a/apps/lib/apps.c
+++ b/apps/lib/apps.c
@@ -79,15 +79,6 @@ 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);
int chopup_args(ARGS *arg, char *buf)
@@ -460,16 +451,17 @@ X509 *load_cert_pass(const char *uri, int format, int
maybe_stdin,
if (desc == NULL)
desc = "certificate";
- if (IS_HTTPS(uri))
+ if (IS_HTTPS(uri)) {
BIO_printf(bio_err, "Loading %s over HTTPS is unsupported\n", desc);
- else if (IS_HTTP(uri))
+ } else if (IS_HTTP(uri)) {
cert = X509_load_http(uri, NULL, NULL, 0 /* timeout */);
- else
+ if (cert == NULL) {
+ ERR_print_errors(bio_err);
+ BIO_printf(bio_err, "Unable to load %s from %s\n", desc, uri);
+ }
+ } else {
(void)load_key_certs_crls(uri, format, maybe_stdin, pass, desc,
NULL, NULL, NULL, &cert, NULL, NULL, NULL);
- if (cert == NULL) {
- BIO_printf(bio_err, "Unable to load %s\n", desc);
- ERR_print_errors(bio_err);
}
return cert;
}
@@ -481,16 +473,17 @@ X509_CRL *load_crl(const char *uri, int format, int
maybe_stdin,
if (desc == NULL)
desc = "CRL";
- if (IS_HTTPS(uri))
+ if (IS_HTTPS(uri)) {
BIO_printf(bio_err, "Loading %s over HTTPS is unsupported\n", desc);
- else if (IS_HTTP(uri))
+ } else if (IS_HTTP(uri)) {
crl = X509_CRL_load_http(uri, NULL, NULL, 0 /* timeout */);
- else
+ if (crl == NULL) {
+ ERR_print_errors(bio_err);
+ BIO_printf(bio_err, "Unable to load %s from %s\n", desc, uri);
+ }
+ } else {
(void)load_key_certs_crls(uri, format, maybe_stdin, NULL, desc,
NULL, NULL, NULL, NULL, NULL, &crl, NULL);
- if (crl == NULL) {
- BIO_printf(bio_err, "Unable to load %s\n", desc);
- ERR_print_errors(bio_err);
}
return crl;
}
@@ -517,8 +510,8 @@ X509_REQ *load_csr(const char *file, int format, const char
*desc)
end:
if (req == NULL) {
- BIO_printf(bio_err, "Unable to load %s\n", desc);
ERR_print_errors(bio_err);
+ BIO_printf(bio_err, "Unable to load %s\n", desc);
}
BIO_free(in);
return req;
@@ -579,23 +572,23 @@ EVP_PKEY *load_keyparams_suppress(const char *uri, int
format, int maybe_stdin,
int suppress_decode_errors)
{
EVP_PKEY *params = NULL;
+ BIO *bio_bak = bio_err;
if (desc == NULL)
desc = "key parameters";
-
- (void)load_key_certs_crls_suppress(uri, format, maybe_stdin, NULL, desc,
- NULL, NULL, ¶ms, NULL, NULL, NULL,
- NULL, suppress_decode_errors);
+ if (suppress_decode_errors)
+ bio_err = NULL;
+ (void)load_key_certs_crls(uri, format, maybe_stdin, NULL, desc,
+ NULL, NULL, ¶ms, NULL, NULL, NULL, NULL);
if (params != NULL && keytype != NULL && !EVP_PKEY_is_a(params, keytype)) {
- 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);
- }
+ ERR_print_errors(bio_err);
+ BIO_printf(bio_err,
+ "Unable to load %s from %s (unexpected parameters type)\n",
+ desc, uri);
EVP_PKEY_free(params);
params = NULL;
}
+ bio_err = bio_bak;
return params;
}
@@ -680,6 +673,8 @@ int load_cert_certs(const char *uri,
int ret = 0;
char *pass_string;
+ if (desc == NULL)
+ desc = pcerts == NULL ? "certificate" : "certificates";
if (exclude_http && (HAS_CASE_PREFIX(uri, "http://";)
|| HAS_CASE_PREFIX(uri, "https://";))) {
BIO_printf(bio_err, "error: HTTP retrieval not allowed for %s\n",
desc);
@@ -687,8 +682,7 @@ int load_cert_certs(const char *uri,
}
pass_string = get_passwd(pass, desc);
ret = load_key_certs_crls(uri, FORMAT_UNDEF, 0, pass_string, desc,
- NULL, NULL, NULL,
- pcert, pcerts, NULL, NULL);
+ NULL, NULL, NULL, pcert, pcerts, NULL, NULL);
clear_free(pass_string);
if (ret) {
@@ -788,10 +782,12 @@ X509_STORE *load_certstore(char *input, const char *pass,
const char *desc,
int load_certs(const char *uri, int maybe_stdin, STACK_OF(X509) **certs,
const char *pass, const char *desc)
{
- int was_NULL = *certs == NULL;
- int ret = load_key_certs_crls(uri, FORMAT_UNDEF, maybe_stdin,
- pass, desc, NULL, NULL,
- NULL, NULL, certs, NULL, NULL);
+ int ret, was_NULL = *certs == NULL;
+
+ if (desc == NULL)
+ desc = "certificates";
+ ret = load_key_certs_crls(uri, FORMAT_UNDEF, maybe_stdin, pass, desc,
+ NULL, NULL, NULL, NULL, certs, NULL, NULL);
if (!ret && was_NULL) {
OSSL_STACK_OF_X509_free(*certs);
@@ -807,10 +803,12 @@ int load_certs(const char *uri, int maybe_stdin,
STACK_OF(X509) **certs,
int load_crls(const char *uri, STACK_OF(X509_CRL) **crls,
const char *pass, const char *desc)
{
- int was_NULL = *crls == NULL;
- int ret = load_key_certs_crls(uri, FORMAT_UNDEF, 0, pass, desc,
- NULL, NULL, NULL,
- NULL, NULL, NULL, crls);
+ int ret, was_NULL = *crls == NULL;
+
+ if (desc == NULL)
+ desc = "CRLs";
+ ret = load_key_certs_crls(uri, FORMAT_UNDEF, 0, pass, desc,
+ NULL, NULL, NULL, NULL, NULL, NULL, crls);
if (!ret && was_NULL) {
sk_X509_CRL_pop_free(*crls, X509_CRL_free);
@@ -845,14 +843,12 @@ 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).
*/
-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 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)
{
PW_CB_DATA uidata;
OSSL_STORE_CTX *ctx = NULL;
@@ -871,6 +867,7 @@ int load_key_certs_crls_suppress(const char *uri, int
format, int maybe_stdin,
OSSL_PARAM itp[2];
const OSSL_PARAM *params = NULL;
+ ERR_set_mark();
if (ppkey != NULL) {
*ppkey = NULL;
cnt_expectations++;
@@ -913,9 +910,9 @@ int load_key_certs_crls_suppress(const char *uri, int
format, int maybe_stdin,
SET_EXPECT(expect, OSSL_STORE_INFO_CRL);
}
if (cnt_expectations == 0) {
- BIO_printf(bio_err, "Internal error: nothing to load from %s\n",
- uri != NULL ? uri : "<stdin>");
- return 0;
+ BIO_printf(bio_err, "Internal error: no expectation to load");
+ failed = "anything";
+ goto end;
}
uidata.password = pass;
@@ -1051,14 +1048,14 @@ int load_key_certs_crls_suppress(const char *uri, int
format, int maybe_stdin,
any = 1;
failed = "CRL";
}
- if (!suppress_decode_errors) {
- if (failed != NULL)
- BIO_printf(bio_err, "Could not read");
- if (any)
- BIO_printf(bio_err, " any");
- }
+ if (failed != NULL)
+ BIO_printf(bio_err, "Could not read");
+ if (any)
+ BIO_printf(bio_err, " any");
}
- if (!suppress_decode_errors && failed != NULL) {
+ if (failed != NULL) {
+ unsigned long err = ERR_peek_last_error();
+
if (desc != NULL && strstr(desc, failed) != NULL) {
BIO_printf(bio_err, " %s", desc);
} else {
@@ -1068,27 +1065,23 @@ int load_key_certs_crls_suppress(const char *uri, int
format, int maybe_stdin,
}
if (uri != NULL)
BIO_printf(bio_err, " from %s", uri);
+ if (ERR_SYSTEM_ERROR(err)) {
+ /* provide more readable diagnostic output */
+ BIO_printf(bio_err, ": %s", strerror(ERR_GET_REASON(err)));
+ ERR_pop_to_mark();
+ ERR_set_mark();
+ }
BIO_printf(bio_err, "\n");
ERR_print_errors(bio_err);
}
- if (suppress_decode_errors || failed == NULL)
- /* clear any spurious errors */
- ERR_clear_error();
+ if (bio_err == NULL || failed == NULL)
+ /* clear any suppressed or spurious errors */
+ ERR_pop_to_mark();
+ else
+ ERR_clear_last_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 */
#define X509V3_EXT_DEFAULT 0
diff --git a/crypto/store/store_lib.c b/crypto/store/store_lib.c
index 42722a2560..4efa7eea03 100644
--- a/crypto/store/store_lib.c
+++ b/crypto/store/store_lib.c
@@ -114,13 +114,17 @@ OSSL_STORE_open_ex(const char *uri, OSSL_LIB_CTX *libctx,
const char *propq,
scheme = schemes[i];
OSSL_TRACE1(STORE, "Looking up scheme %s\n", scheme);
#ifndef OPENSSL_NO_DEPRECATED_3_0
+ ERR_set_mark();
if ((loader = ossl_store_get0_loader_int(scheme)) != NULL) {
+ ERR_clear_last_mark();
no_loader_found = 0;
if (loader->open_ex != NULL)
loader_ctx = loader->open_ex(loader, uri, libctx, propq,
ui_method, ui_data);
else
loader_ctx = loader->open(loader, uri, ui_method, ui_data);
+ } else {
+ ERR_pop_to_mark();
}
#endif
if (loader == NULL
