From 57c05c57c3aab2755ec6eeae5d1800ac9cbd2f6d Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Sat, 27 Jun 2020 10:28:45 +0200 Subject: [PATCH] apps: Correct and extend diagnostics of parse_name() Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/12296) --- apps/ca.c | 2 +- apps/cmp.c | 6 ++---- apps/include/apps.h | 3 ++- apps/lib/apps.c | 38 ++++++++++++++++++++++---------------- apps/req.c | 2 +- apps/storeutl.c | 12 ++++-------- apps/x509.c | 3 ++- 7 files changed, 34 insertions(+), 32 deletions(-) diff --git a/apps/ca.c b/apps/ca.c index e001a34190..fef0b82c39 100644 --- a/apps/ca.c +++ b/apps/ca.c @@ -1463,7 +1463,7 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509, row[i] = NULL; if (subj) { - X509_NAME *n = parse_name(subj, chtype, multirdn); + X509_NAME *n = parse_name(subj, chtype, multirdn, "subject"); if (!n) { ERR_print_errors(bio_err); diff --git a/apps/cmp.c b/apps/cmp.c index 1af53510b3..86b1bd6a30 100644 --- a/apps/cmp.c +++ b/apps/cmp.c @@ -1109,12 +1109,10 @@ static int set_name(const char *str, OSSL_CMP_CTX *ctx, const char *desc) { if (str != NULL) { - X509_NAME *n = parse_name(str, MBSTRING_ASC, 0); + X509_NAME *n = parse_name(str, MBSTRING_ASC, 0, desc); - if (n == NULL) { - CMP_err2("cannot parse %s DN '%s'", desc, str); + if (n == NULL) return 0; - } if (!(*set_fn) (ctx, n)) { X509_NAME_free(n); CMP_err("out of memory"); diff --git a/apps/include/apps.h b/apps/include/apps.h index e91cdcdb8f..554d33e1c9 100644 --- a/apps/include/apps.h +++ b/apps/include/apps.h @@ -201,7 +201,8 @@ void free_index(CA_DB *db); int index_name_cmp(const OPENSSL_CSTRING *a, const OPENSSL_CSTRING *b); int parse_yesno(const char *str, int def); -X509_NAME *parse_name(const char *str, long chtype, int multirdn); +X509_NAME *parse_name(const char *str, int chtype, int multirdn, + const char *desc); void policies_print(X509_STORE_CTX *ctx); int bio_to_mem(unsigned char **out, int maxlen, BIO *in); int pkey_ctrl_string(EVP_PKEY_CTX *ctx, const char *value); diff --git a/apps/lib/apps.c b/apps/lib/apps.c index cf99ca0ebf..e8592c4880 100644 --- a/apps/lib/apps.c +++ b/apps/lib/apps.c @@ -1670,7 +1670,8 @@ int parse_yesno(const char *str, int def) * name is expected to be in the format /type0=value0/type1=value1/type2=... * where characters may be escaped by \ */ -X509_NAME *parse_name(const char *cp, long chtype, int canmulti) +X509_NAME *parse_name(const char *cp, int chtype, int canmulti, + const char *desc) { int nextismulti = 0; char *work; @@ -1678,19 +1679,22 @@ X509_NAME *parse_name(const char *cp, long chtype, int canmulti) if (*cp++ != '/') { BIO_printf(bio_err, - "name is expected to be in the format " + "%s: %s name is expected to be in the format " "/type0=value0/type1=value1/type2=... where characters may " "be escaped by \\. This name is not in that format: '%s'\n", - --cp); + opt_getprog(), desc, --cp); return NULL; } n = X509_NAME_new(); - if (n == NULL) + if (n == NULL) { + BIO_printf(bio_err, "%s: Out of memory\n", opt_getprog()); return NULL; + } work = OPENSSL_strdup(cp); if (work == NULL) { - BIO_printf(bio_err, "%s: Error copying name input\n", opt_getprog()); + BIO_printf(bio_err, "%s: Error copying %s name input\n", + opt_getprog(), desc); goto err; } @@ -1705,13 +1709,13 @@ X509_NAME *parse_name(const char *cp, long chtype, int canmulti) /* Collect the type */ while (*cp != '\0' && *cp != '=') *bp++ = *cp++; + *bp++ = '\0'; if (*cp == '\0') { BIO_printf(bio_err, - "%s: Hit end of string before finding the '='\n", - opt_getprog()); + "%s: Missing '=' after RDN type string '%s' in %s name string\n", + opt_getprog(), typestr, desc); goto err; } - *bp++ = '\0'; ++cp; /* Collect the value. */ @@ -1723,8 +1727,8 @@ X509_NAME *parse_name(const char *cp, long chtype, int canmulti) } if (*cp == '\\' && *++cp == '\0') { BIO_printf(bio_err, - "%s: Escape character at end of string\n", - opt_getprog()); + "%s: Escape character at end of %s name string\n", + opt_getprog(), desc); goto err; } } @@ -1737,22 +1741,24 @@ X509_NAME *parse_name(const char *cp, long chtype, int canmulti) /* Parse */ nid = OBJ_txt2nid(typestr); if (nid == NID_undef) { - BIO_printf(bio_err, "%s: Skipping unknown attribute \"%s\"\n", - opt_getprog(), typestr); + BIO_printf(bio_err, + "%s: Skipping unknown %s name attribute \"%s\"\n", + opt_getprog(), desc, typestr); continue; } if (*valstr == '\0') { BIO_printf(bio_err, - "%s: No value provided for Subject Attribute %s, skipped\n", - opt_getprog(), typestr); + "%s: No value provided for %s name attribute \"%s\", skipped\n", + opt_getprog(), desc, typestr); continue; } if (!X509_NAME_add_entry_by_NID(n, nid, chtype, valstr, strlen((char *)valstr), -1, ismulti ? -1 : 0)) { ERR_print_errors(bio_err); - BIO_printf(bio_err, "%s: Error adding name attribute \"/%s=%s\"\n", - opt_getprog(), typestr ,valstr); + BIO_printf(bio_err, + "%s: Error adding %s name attribute \"/%s=%s\"\n", + opt_getprog(), desc, typestr ,valstr); goto err; } } diff --git a/apps/req.c b/apps/req.c index 8931e9829f..46739554bd 100644 --- a/apps/req.c +++ b/apps/req.c @@ -1078,7 +1078,7 @@ static int build_subject(X509_REQ *req, const char *subject, unsigned long chtyp { X509_NAME *n; - if ((n = parse_name(subject, chtype, multirdn)) == NULL) + if ((n = parse_name(subject, chtype, multirdn, "subject")) == NULL) return 0; if (!X509_REQ_set_subject_name(req, n)) { diff --git a/apps/storeutl.c b/apps/storeutl.c index 95af277260..66fd423ab0 100644 --- a/apps/storeutl.c +++ b/apps/storeutl.c @@ -157,11 +157,9 @@ int storeutl_main(int argc, char *argv[]) prog); goto end; } - if ((subject = parse_name(opt_arg(), MBSTRING_UTF8, 1)) == NULL) { - BIO_printf(bio_err, "%s: can't parse subject argument.\n", - prog); + subject = parse_name(opt_arg(), MBSTRING_UTF8, 1, "subject"); + if (subject == NULL) goto end; - } break; case OPT_CRITERION_ISSUER: if (criterion != 0 @@ -177,11 +175,9 @@ int storeutl_main(int argc, char *argv[]) prog); goto end; } - if ((issuer = parse_name(opt_arg(), MBSTRING_UTF8, 1)) == NULL) { - BIO_printf(bio_err, "%s: can't parse issuer argument.\n", - prog); + issuer = parse_name(opt_arg(), MBSTRING_UTF8, 1, "issuer"); + if (issuer == NULL) goto end; - } break; case OPT_CRITERION_SERIAL: if (criterion != 0 diff --git a/apps/x509.c b/apps/x509.c index d8f69c08eb..fbe4b8cefe 100644 --- a/apps/x509.c +++ b/apps/x509.c @@ -536,7 +536,8 @@ int x509_main(int argc, char **argv) "The -new option requires a subject to be set using -subj\n"); goto end; } - if (subj != NULL && (fsubj = parse_name(subj, chtype, multirdn)) == NULL) + if (subj != NULL + && (fsubj = parse_name(subj, chtype, multirdn, "subject")) == NULL) goto end; if (CAkeyfile == NULL && CA_flag && CAformat == FORMAT_PEM) { -- 2.39.2