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.sch...@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
> 

Attachment: signature.asc
Description: PGP signature

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to