CMP app: make -geninfo option accept multiple ITAVs and support string values besides integers

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com>
(Merged from https://github.com/openssl/openssl/pull/21281)

3 files changed, 106 insertions, 61 deletions

diff --git a/apps/cmp.c b/apps/cmp.c
index 67f281e0aa..e0b03c3cb5 100644
--- a/apps/cmp.c
+++ b/apps/cmp.c
@@ -295,9 +295,9 @@ const OPTIONS cmp_options[] = {
     {"profile", OPT_PROFILE, 's',
      "Certificate profile name to place in generalInfo field of request PKIHeader"},
     {"geninfo", OPT_GENINFO, 's',
-     "generalInfo integer values to place in request PKIHeader with given OID"},
+     "Comma-separated list of OID and value to place in generalInfo PKIHeader"},
     {OPT_MORE_STR, 0, 0,
-     "specified in the form <OID>:int:<n>, e.g. \"1.2.3.4:int:56789\""},
+     "of form <OID>:int:<n> or <OID>:str:<s>, e.g. \'1.2.3.4:int:56789, id-kp:str:name'"},
 
     OPT_SECTION("Certificate enrollment"),
     {"newkey", OPT_NEWKEY, 's',
@@ -1794,9 +1794,11 @@ static int setup_request_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine)
         char *next = next_item(opt_policy_oids);
 
         if ((policy = OBJ_txt2obj(opt_policy_oids, 1)) == 0) {
-            CMP_err1("unknown policy OID '%s'", opt_policy_oids);
+            CMP_err1("Invalid -policy_oids arg '%s'", opt_policy_oids);
             return 0;
         }
+        if (OBJ_obj2nid(policy) == NID_undef)
+            CMP_warn1("Unknown -policy_oids arg: %.40s", opt_policy_oids);
 
         if ((pinfo = POLICYINFO_new()) == NULL) {
             ASN1_OBJECT_free(policy);
@@ -1873,62 +1875,94 @@ static int add_certProfile(OSSL_CMP_CTX *ctx, const char *name)
 
 static int handle_opt_geninfo(OSSL_CMP_CTX *ctx)
 {
+    ASN1_OBJECT *obj = NULL;
+    ASN1_TYPE *type = NULL;
     long value;
-    ASN1_OBJECT *type;
-    ASN1_INTEGER *aint;
-    ASN1_TYPE *val;
+    ASN1_INTEGER *aint = NULL;
+    ASN1_UTF8STRING *text = NULL;
     OSSL_CMP_ITAV *itav;
-    char *endstr;
-    char *valptr = strchr(opt_geninfo, ':');
-
-    if (valptr == NULL) {
-        CMP_err("missing ':' in -geninfo option");
-        return 0;
-    }
-    valptr[0] = '\0';
-    valptr++;
-
-    if (!CHECK_AND_SKIP_CASE_PREFIX(valptr, "int:")) {
-        CMP_err("missing 'int:' in -geninfo option");
-        return 0;
-    }
+    char *ptr = opt_geninfo, *oid, *end;
+
+    do {
+        while (isspace(_UC(*ptr)))
+            ptr++;
+        oid = ptr;
+        if ((ptr = strchr(oid, ':')) == NULL) {
+            CMP_err1("Missing ':' in -geninfo arg %.40s", oid);
+            return 0;
+        }
+        *ptr++ = '\0';
+        if ((obj = OBJ_txt2obj(oid, 0)) == NULL) {
+            CMP_err1("Invalid OID in -geninfo arg %.40s", oid);
+            return 0;
+        }
+        if (OBJ_obj2nid(obj) == NID_undef)
+            CMP_warn1("Unknown OID in -geninfo arg: %.40s", oid);
+        if ((type = ASN1_TYPE_new()) == NULL)
+            goto oom;
 
-    value = strtol(valptr, &endstr, 10);
-    if (endstr == valptr || *endstr != '\0') {
-        CMP_err("cannot parse int in -geninfo option");
-        return 0;
-    }
+        if (CHECK_AND_SKIP_CASE_PREFIX(ptr, "int:")) {
+            value = strtol(ptr, &end, 10);
+            if (end == ptr) {
+                CMP_err1("Cannot parse int in -geninfo arg %.40s", ptr);
+                goto err;
+            }
+            ptr = end;
+            if (*ptr != '\0') {
+                if (*ptr != ',') {
+                    CMP_err1("Missing ',' or end of -geninfo arg after int at %.40s",
+                        ptr);
+                    goto err;
+                }
+                ptr++;
+            }
 
-    type = OBJ_txt2obj(opt_geninfo, 1);
-    if (type == NULL) {
-        CMP_err("cannot parse OID in -geninfo option");
-        return 0;
-    }
+            if ((aint = ASN1_INTEGER_new()) == NULL
+                    || !ASN1_INTEGER_set(aint, value))
+                goto oom;
+            ASN1_TYPE_set(type, V_ASN1_INTEGER, aint);
+            aint = NULL;
+
+        } else if (CHECK_AND_SKIP_CASE_PREFIX(ptr, "str:")) {
+            end = strchr(ptr, ',');
+            if (end == NULL)
+                end = ptr + strlen(ptr);
+            else
+                *end++ = '\0';
+            if ((text = ASN1_UTF8STRING_new()) == NULL
+                    || !ASN1_STRING_set(text, ptr, -1))
+                goto oom;
+            ptr = end;
+            ASN1_TYPE_set(type, V_ASN1_UTF8STRING, text);
+            text = NULL;
 
-    if ((aint = ASN1_INTEGER_new()) == NULL)
-        goto oom;
+        } else {
+            CMP_err1("Missing 'int:' or 'str:' in -geninfo arg %.40s", ptr);
+            goto err;
+        }
 
-    val = ASN1_TYPE_new();
-    if (!ASN1_INTEGER_set(aint, value) || val == NULL) {
-        ASN1_INTEGER_free(aint);
-        goto oom;
-    }
-    ASN1_TYPE_set(val, V_ASN1_INTEGER, aint);
-    itav = OSSL_CMP_ITAV_create(type, val);
-    if (itav == NULL) {
-        ASN1_TYPE_free(val);
-        goto oom;
-    }
+        if ((itav = OSSL_CMP_ITAV_create(obj, type)) == NULL) {
+            CMP_err("Unable to create 'OSSL_CMP_ITAV' structure");
+            goto err;
+        }
+        obj = NULL;
+        type = NULL;
 
-    if (!OSSL_CMP_CTX_push0_geninfo_ITAV(ctx, itav)) {
-        OSSL_CMP_ITAV_free(itav);
-        return 0;
-    }
+        if (!OSSL_CMP_CTX_push0_geninfo_ITAV(ctx, itav)) {
+            CMP_err("Failed to add ITAV for geninfo of the PKI message header");
+            OSSL_CMP_ITAV_free(itav);
+            return 0;
+        }
+    } while (*ptr != '\0');
     return 1;
 
  oom:
-    ASN1_OBJECT_free(type);
     CMP_err("out of memory");
+ err:
+    ASN1_OBJECT_free(obj);
+    ASN1_TYPE_free(type);
+    ASN1_INTEGER_free(aint);
+    ASN1_UTF8STRING_free(text);
     return 0;
 }
 
@@ -3147,6 +3181,10 @@ int cmp_main(int argc, char **argv)
     }
     (void)BIO_flush(bio_err); /* prevent interference with opt_help() */
 
+    cmp_ctx = OSSL_CMP_CTX_new(app_get0_libctx(), app_get0_propq());
+    if (cmp_ctx == NULL)
+        goto err;
+
     ret = get_opts(argc, argv);
     if (ret <= 0)
         goto err;
@@ -3167,10 +3205,6 @@ int cmp_main(int argc, char **argv)
         }
     }
 
-    cmp_ctx = OSSL_CMP_CTX_new(app_get0_libctx(), app_get0_propq());
-    if (cmp_ctx == NULL)
-        goto err;
-
     OSSL_CMP_CTX_set_log_verbosity(cmp_ctx, opt_verbosity);
     if (!OSSL_CMP_CTX_set_log_cb(cmp_ctx, print_to_bio_out)) {
         CMP_err1("cannot set up error reporting and logging for %s", prog);
diff --git a/doc/man1/openssl-cmp.pod.in b/doc/man1/openssl-cmp.pod.in
index ee54697c9c..911b63e155 100644
--- a/doc/man1/openssl-cmp.pod.in
+++ b/doc/man1/openssl-cmp.pod.in
@@ -18,7 +18,7 @@ Generic message options:
 [B<-cmd> I<ir|cr|kur|p10cr|rr|genm>]
 [B<-infotype> I<name>]
 [B<-profile> I<name>]
-[B<-geninfo> I<OID:int:N>]
+[B<-geninfo> I<values>]
 
 Certificate enrollment options:
 
@@ -252,10 +252,13 @@ So far, there is specific support for C<caCerts> and C<rootCaCert>.
 Name of a certificate profile to place in
 the PKIHeader generalInfo field of request messages.
 
-=item B<-geninfo> I<OID:int:N>
+=item B<-geninfo> I<values>
 
-generalInfo integer values to place in request PKIHeader with given OID,
-e.g., C<1.2.3.4:int:56789>.
+A comma-separated list of InfoTypeAndValue to place in
+the generalInfo field of the PKIHeader of requests messages.
+Each InfoTypeAndValue gives an OID and an integer or string value
+of the form I<OID>:int:I<number> or I<OID>:str:I<text>,
+e.g., C<'1.2.3.4:int:56789, id-kp:str:name'>.
 
 =back
 
diff --git a/test/recipes/80-test_cmp_http_data/test_commands.csv b/test/recipes/80-test_cmp_http_data/test_commands.csv
index 869bab7c99..425385fe69 100644
--- a/test/recipes/80-test_cmp_http_data/test_commands.csv
+++ b/test/recipes/80-test_cmp_http_data/test_commands.csv
@@ -82,12 +82,20 @@ expected,description, -section,val, -cmd,val,val2, -cacertsout,val,val2, -infoty
 0,profile missing argument, -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -profile,,,,,
 0,profile extra argument, -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -profile,profile1,profile2,,,
 ,,,,,,,,,,,,,,,,,,,
-1,geninfo, -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,1.2.3:int:987,BLANK,,BLANK,
-0,geninfo missing argument, -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,,,,,
-0,geninfo bad syntax: leading '.', -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,.1.2.3:int:987,BLANK,,BLANK,
-0,geninfo bad syntax: missing ':', -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,1.2.3:int987,,,,
-0,geninfo bad syntax: double ':', -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,1.2.3:int::987,,,,
+1,geninfo int,                        -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,1.3:int:987
+1,geninfo str,                        -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,id-kp:str:name
+1,geninfo empty str,                  -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,id-kp:str:
+1,geninfo str and int,                -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo, 'id-kp:str:name, 1.3:int:987'
+0,geninfo missing argument,           -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,,,,,
+0,geninfo bad OID num syntax,         -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,.1.2.3:int:987
+0,geninfo invalid OID number string,  -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,1.333:int:987
+1,geninfo unknown OID number string,  -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,1.33:int:987
+0,geninfo bad OID name: trailing '_', -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,id-kp_:int:987
 0,geninfo bad syntax: missing ':int', -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,1.2.3,,,,
+0,geninfo bad type tag,               -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,1.2.3:xyz:987,,,,
+0,geninfo bad syntax: missing ':',    -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,1.2.3:int987,,,,
+0,geninfo bad int syntax: double ':', -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,1.2.3:int::987,,,,
+0,geninfo bad int syntax: extra char, -section,, -cmd,cr,, -cert,signer.crt, -key,signer.p12, -keypass,pass:12345,BLANK,, -geninfo,1.2.3:int:987@,,,,
 ,,,,,,,,,,,,,,,,,,,
 1,reqout ir+certConf rspout ip+pkiConf, -section,, -cmd,ir,,-reqout,_RESULT_DIR/ir.der _RESULT_DIR/certConf.der,,-rspout,_RESULT_DIR/ip.der _RESULT_DIR/pkiConf.der,,BLANK,,BLANK,
 1,reqout cr rspout cp, -section,, -cmd,cr,,-reqout,_RESULT_DIR/cr.der,,-rspout,_RESULT_DIR/cp.der,,BLANK,,BLANK,