Skip to content

Commit

Permalink
guest/generate: fix multiple input/output format specifier argument p…
Browse files Browse the repository at this point in the history
…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]>
  • Loading branch information
erichte-ibm committed Jan 3, 2024
1 parent 0e68220 commit f81b4a8
Showing 1 changed file with 21 additions and 3 deletions.
24 changes: 21 additions & 3 deletions backends/guest/guest_svc_generate.c
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -493,14 +506,19 @@ 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");
else if (args->time && validate_time(args->time))
prlog(PR_ERR, "invalid timestamp flag '-t YYYY-MM-DDThh:mm:ss' , "
"see usage...\n");
Expand Down

0 comments on commit f81b4a8

Please sign in to comment.