| Message ID | 20180104103915.23165-1-quentin.schulz@free-electrons.com |
|---|---|
| State | New |
| Headers | show |
| Series | cmd: nvedit: add whitelist option for env import | expand |
Hi all, On Thu, Jan 04, 2018 at 11:39:15AM +0100, Quentin Schulz wrote: > While the `env export` can take as parameters variables to be exported, > `env import` does not have such a mechanism of variable selection. > > Let's add a `-w` option that asks `env import` to look for the > `whitelisted_vars` env variable for a space-separated list of variables > that are whitelisted. > > Every env variable present in env at `addr` and in `whitelisted_vars` > env variable will override the value of the variable in the current env. > All the remaining variables are left untouched. > > One of its use case could be to load a secure environment from the > signed U-Boot binary and load only a handful of variables from an > other, unsecure, environment without completely losing control of > U-Boot. > > Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com> It's been two months since I posted this patch and there hasn't been any review. Is there something to improve? Is there an other approach to take? I'm all ears. Thanks, Quentin > --- > cmd/nvedit.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 64 insertions(+), 7 deletions(-) > > diff --git a/cmd/nvedit.c b/cmd/nvedit.c > index 4e79d03856..9f983b41a1 100644 > --- a/cmd/nvedit.c > +++ b/cmd/nvedit.c > @@ -971,7 +971,7 @@ sep_err: > > #ifdef CONFIG_CMD_IMPORTENV > /* > - * env import [-d] [-t [-r] | -b | -c] addr [size] > + * env import [-d] [-t [-r] | -b | -c] [-w] addr [size] > * -d: delete existing environment before importing; > * otherwise overwrite / append to existing definitions > * -t: assume text format; either "size" must be given or the > @@ -982,6 +982,10 @@ sep_err: > * for line endings. Only effective in addition to -t. > * -b: assume binary format ('\0' separated, "\0\0" terminated) > * -c: assume checksum protected environment format > + * -w: specify that whitelisting of variables should be used when > + * importing environment. The space-separated list of variables > + * that should override the ones in current environment is stored > + * in `whitelisted_vars`. > * addr: memory address to read from > * size: length of input data; if missing, proper '\0' > * termination is mandatory > @@ -991,11 +995,16 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, > { > ulong addr; > char *cmd, *ptr; > + char **array = NULL; > char sep = '\n'; > int chk = 0; > int fmt = 0; > int del = 0; > int crlf_is_lf = 0; > + int wl = 0; > + int wl_count = 0; > + int ret; > + unsigned int i; > size_t size; > > cmd = *argv; > @@ -1026,6 +1035,9 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, > case 'd': > del = 1; > break; > + case 'w': > + wl = 1; > + break; > default: > return CMD_RET_USAGE; > } > @@ -1082,14 +1094,59 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, > ptr = (char *)ep->data; > } > > - if (himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR, > - crlf_is_lf, 0, NULL) == 0) { > + if(wl) { > + char *str, *token, *tmp; > + wl_count = 1; > + > + str = env_get("whitelisted_vars"); > + if (!str) { > + puts("## Error: whitelisted_vars is not set.\n"); > + return CMD_RET_USAGE; > + } > + > + tmp = malloc(sizeof(char) * (strlen(str) + 1)); > + strcpy(tmp, str); > + > + token = strchr(tmp, ' '); > + while (!token) { > + wl_count++; > + token = strchr(token + 1, ' '); > + } > + > + strcpy(tmp, str); > + > + array = malloc(sizeof(char *) * wl_count); > + wl_count = 0; > + > + token = strtok(tmp, " "); > + while (token) { > + array[wl_count] = malloc(sizeof(char) * > + (strlen(token) + 1)); > + strcpy(array[wl_count], token); > + wl_count++; > + token = strtok(NULL, " "); > + } > + > + free(tmp); > + } > + > + ret = himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR, > + crlf_is_lf, wl ? wl_count : 0, wl ? array : NULL); > + if (!ret) { > pr_err("Environment import failed: errno = %d\n", errno); > - return 1; > + ret = 1; > + } else { > + gd->flags |= GD_FLG_ENV_READY; > + ret = 0; > } > - gd->flags |= GD_FLG_ENV_READY; > > - return 0; > + if (wl) { > + for (i = 0; i < wl_count; i++) > + free(array[i]); > + free(array); > + } > + > + return ret; > > sep_err: > printf("## %s: only one of \"-b\", \"-c\" or \"-t\" allowed\n", > @@ -1212,7 +1269,7 @@ static char env_help_text[] = > #endif > #endif > #if defined(CONFIG_CMD_IMPORTENV) > - "env import [-d] [-t [-r] | -b | -c] addr [size] - import environment\n" > + "env import [-d] [-t [-r] | -b | -c] [-w] addr [size] - import environment\n" > #endif > "env print [-a | name ...] - print environment\n" > #if defined(CONFIG_CMD_RUN) > -- > 2.14.1 >
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 4e79d03856..9f983b41a1 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -971,7 +971,7 @@ sep_err: #ifdef CONFIG_CMD_IMPORTENV /* - * env import [-d] [-t [-r] | -b | -c] addr [size] + * env import [-d] [-t [-r] | -b | -c] [-w] addr [size] * -d: delete existing environment before importing; * otherwise overwrite / append to existing definitions * -t: assume text format; either "size" must be given or the @@ -982,6 +982,10 @@ sep_err: * for line endings. Only effective in addition to -t. * -b: assume binary format ('\0' separated, "\0\0" terminated) * -c: assume checksum protected environment format + * -w: specify that whitelisting of variables should be used when + * importing environment. The space-separated list of variables + * that should override the ones in current environment is stored + * in `whitelisted_vars`. * addr: memory address to read from * size: length of input data; if missing, proper '\0' * termination is mandatory @@ -991,11 +995,16 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, { ulong addr; char *cmd, *ptr; + char **array = NULL; char sep = '\n'; int chk = 0; int fmt = 0; int del = 0; int crlf_is_lf = 0; + int wl = 0; + int wl_count = 0; + int ret; + unsigned int i; size_t size; cmd = *argv; @@ -1026,6 +1035,9 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, case 'd': del = 1; break; + case 'w': + wl = 1; + break; default: return CMD_RET_USAGE; } @@ -1082,14 +1094,59 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, ptr = (char *)ep->data; } - if (himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR, - crlf_is_lf, 0, NULL) == 0) { + if(wl) { + char *str, *token, *tmp; + wl_count = 1; + + str = env_get("whitelisted_vars"); + if (!str) { + puts("## Error: whitelisted_vars is not set.\n"); + return CMD_RET_USAGE; + } + + tmp = malloc(sizeof(char) * (strlen(str) + 1)); + strcpy(tmp, str); + + token = strchr(tmp, ' '); + while (!token) { + wl_count++; + token = strchr(token + 1, ' '); + } + + strcpy(tmp, str); + + array = malloc(sizeof(char *) * wl_count); + wl_count = 0; + + token = strtok(tmp, " "); + while (token) { + array[wl_count] = malloc(sizeof(char) * + (strlen(token) + 1)); + strcpy(array[wl_count], token); + wl_count++; + token = strtok(NULL, " "); + } + + free(tmp); + } + + ret = himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR, + crlf_is_lf, wl ? wl_count : 0, wl ? array : NULL); + if (!ret) { pr_err("Environment import failed: errno = %d\n", errno); - return 1; + ret = 1; + } else { + gd->flags |= GD_FLG_ENV_READY; + ret = 0; } - gd->flags |= GD_FLG_ENV_READY; - return 0; + if (wl) { + for (i = 0; i < wl_count; i++) + free(array[i]); + free(array); + } + + return ret; sep_err: printf("## %s: only one of \"-b\", \"-c\" or \"-t\" allowed\n", @@ -1212,7 +1269,7 @@ static char env_help_text[] = #endif #endif #if defined(CONFIG_CMD_IMPORTENV) - "env import [-d] [-t [-r] | -b | -c] addr [size] - import environment\n" + "env import [-d] [-t [-r] | -b | -c] [-w] addr [size] - import environment\n" #endif "env print [-a | name ...] - print environment\n" #if defined(CONFIG_CMD_RUN)
While the `env export` can take as parameters variables to be exported, `env import` does not have such a mechanism of variable selection. Let's add a `-w` option that asks `env import` to look for the `whitelisted_vars` env variable for a space-separated list of variables that are whitelisted. Every env variable present in env at `addr` and in `whitelisted_vars` env variable will override the value of the variable in the current env. All the remaining variables are left untouched. One of its use case could be to load a secure environment from the signed U-Boot binary and load only a handful of variables from an other, unsecure, environment without completely losing control of U-Boot. Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com> --- cmd/nvedit.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 64 insertions(+), 7 deletions(-)