|
From: | Jonathan McCune |
Subject: | Re: [PATCH v4 2/4] load_env support for whitelisting which variables are read from an env file, even if check_signatures=enforce |
Date: | Wed, 25 Sep 2013 08:01:25 -0700 |
В Tue, 24 Sep 2013 19:00:30 -0700
Jon McCune <address@hidden> пишет:
I do not really like such silent assumptions. Being able to load only
> This version of the patch implements whitelisting in the envblk subsystem,
> instead of in loadenv.c. It seems to be cleaner than the previous patches.
>
> This works by adding an open_envblk_file_untrusted() method that bypasses
> signature checking, but only if the invocation of load_env includes a
> whitelist of one or more environment variables that are to be read from the
> file.
selected environment variables is useful by itself, but I still may
want to ensure file was not tampered with.
I suggest you simply add flag "--skip-signature-check" to load_env and
add support for explicit variable list. Then it is up to user how and
when to use it.
And please update also documentation for command changes.
Why do you need extra function? Is not flag to open_envblk_file enough?
> +static grub_file_t
> +open_envblk_file_untrusted (char *filename)
Why do you need to disable *all* filters? Assuming disabling
> +{
> + grub_file_filter_t curfilt[GRUB_FILE_FILTER_MAX];
> + grub_file_t file;
> +
> + /* Temporarily disable all filters so as to read the untrusted file */
> + grub_memcpy (curfilt, grub_file_filters_enabled, sizeof (curfilt));
> + grub_file_filter_disable_all ();
compression was good enough, you just need to disable signature
checking, right?
Again, that's really too restrictive. Like with any other iterators,
> void
> grub_envblk_iterate (grub_envblk_t envblk,
> + const grub_env_whitelist_t* whitelist,
> int hook (const char *name, const char *value))
I'd make hook accept extra pointer for hook-specific data and pass this
data to grub_envblk_iterate. This will let caller decide the policy.
[Prev in Thread] | Current Thread | [Next in Thread] |