Skip to content

Commit

Permalink
APPS load_key_certs_crls(): Make file access errors much more readable
Browse files Browse the repository at this point in the history 
This reverts part of commit ef04491 using a less invasive suppression.

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #16452)
  • Loading branch information
@DDvO
DDvO committed Jan 7, 2022
1 parent 7c64ca7 commit 6e24994
Showing 1 changed file with 68 additions and 75 deletions.
143 changes: 68 additions & 75 deletions apps/lib/apps.c
View file
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)
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

0 comments on commit 6e24994

Please sign in to comment.