guest/generate: fix multiple input/output format specifier argument p…

…arsing

Currently, it is ASSUMED but not enforced that the first argument to
`secvarctl generate` is a format specifier in the form `a:b`, where a,b
correspond to the formats to be generated/converted. This argument parse
logic is applied to ANY bare/positional arguments applied to `generate`,
including repeated ones.

Therefore, any of the following cases can occur:
 1. multiple format specifiers can be supplied:
   `secvarctl generate a:b b:c c:d -n NAME -c file.crt`
 2. invalid/erroneous arguments can be mis-interpreted as a format specifier:
   `secvarctl generate a:b -n NAME -c file.crt -a 0`,
     where `-a` does not take in an argument, thus yielding "invalid format"
 3. format specifiers can be ANYWHERE in the arg list:
   `secvarctl generate -n NAME -c file.crt a:b`

This commit addresses case 1 and 2 by only allowing format specifiers to be
declared ONCE in an argument list. Furthermore, the format specifier is
validated while parsing, so in the event the erroneous positional
argument is read in first, the parser will immediately complain.

Case 3 is NOT addressed by this commit, however is a consideration for
future refactoring of the argument parsing logic.

Signed-off-by: Eric Richter <[email protected]>

backends/guest/guest_svc_generate.c

@@ -483,6 +483,19 @@ static int parse_options(int key, char *arg, struct argp_state *state)
 		args->append_flag = 1;
 		break;
 	case ARGP_KEY_ARG:
+		/* there should only be one format specifier, error if another is supplied */
+		if (args->input_form && args->output_form) {
+			prlog(PR_ERR, "ERROR: unknown additional positional argument %s\n", arg);
+			rc = ARG_PARSE_FAIL;
+			break;
+		}
+		/* both forms should be either set or NULL, this should never be reached. */
+		if (!args->input_form ^ !args->output_form) {
+			prlog(PR_ERR,
+			      "ERROR: only one of input_form/output_form is set, this should not happen\n");
+			rc = ARG_PARSE_FAIL;
+			break;
+		}
 		/* check if reset key is desired */
 		if (!strcmp(arg, "reset")) {
 			args->input_form = "reset";
@@ -493,14 +506,22 @@ static int parse_options(int key, char *arg, struct argp_state *state)
 		/* else set input and output formats */
 		args->input_form = strtok(arg, ":");
 		args->output_form = strtok(NULL, ":");
+
+		/* verify both input and output forms are parsed correctly, error otherwise */
+		if (!args->input_form || !args->output_form) {
+			prlog(PR_ERR,
+			      "ERROR: '%s' is not in the correct '<input_format>:<output_format>' form, see usage...\n",
+			      arg);
+			rc = ARG_PARSE_FAIL;
+		}
 		break;
 	case ARGP_KEY_SUCCESS:
 		/* check that all essential args are given and valid */
 		if (args->help_flag)
 			break;
 		else if (args->input_form == NULL || args->output_form == NULL)
-			prlog(PR_ERR, "ERROR: incorrect '<input_format>:<output_format>', see "
-				      "usage...\n");
+			prlog(PR_ERR,
+			      "ERROR: invalid or missing '<input_format>:<output_format>', see usage...\n");
 		else if (args->time && validate_time(args->time))
 			prlog(PR_ERR, "invalid timestamp flag '-t YYYY-MM-DDThh:mm:ss' , "
 				      "see usage...\n");

test/guest_tests.py

@@ -172,6 +172,23 @@ def setUp(self):
     #             f.write(f"POWER SECVAR LOCATION( {SECVARPATH} ) DOES NOT EXIST SO NO TESTS RAN\n")
     #             f.close()
 
+    def test_malformed_generate(self):
+        cert = cert_files[0]  # arbitrarily use the first cert for testing
+
+        # Generate without a inform:outform should fail
+        cmd = list(filter(lambda x: x, generate_esl("db", "", cert, "foo.esl")))
+        self.assertCmdFalse(cmd)
+
+        # Generate with bad inform:output should fail
+        cmd.append("beans")
+        self.assertCmdFalse(cmd)
+        cmd.pop(-1)
+
+        # Generate with more than one inform:outform should also fail
+        cmd.append("c:e")
+        cmd.append("c:e")
+        self.assertCmdFalse(cmd)
+
     def test_generate_esl_files(self):
         for var_name in variables:
             esl_file = gen_dir + var_name + ".esl"