Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APPS load_key_certs_crls(): Make file access errors much more readable #16452

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
143 changes: 68 additions & 75 deletions apps/lib/apps.c
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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, &params, 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, &params, 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;
}

Expand Down Expand Up @@ -680,15 +673,16 @@ 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);
return ret;
}
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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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)
paulidale marked this conversation as resolved.
Show resolved Hide resolved
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);
Expand Down Expand Up @@ -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;
Expand All @@ -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++;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions crypto/store/store_lib.c
Expand Up @@ -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
Expand Down