apps: Correct and extend diagnostics of parse_name()

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/12296)

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) {