Re: [PATCH v2 2/5] load_env support for whitelisting which variables are read from an env file, even if check_signatures=enforce

[Andrey Borzenkov]

В Fri, 6 Sep 2013 14:10:01 -0700
Jonathan McCune <address@hidden> пишет:

> Thanks for the feedback, inline:
> 
> On Fri, Sep 6, 2013 at 12:48 PM, Andrey Borzenkov <address@hidden>wrote:
> 
> > В Fri,  6 Sep 2013 09:18:50 -0700
> > Jon McCune <address@hidden> пишет:
> >
> > > 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.
> >
> > What is the use case? load_env is called exactly once at the beginning
> > of configfile processing. At this point file still has valid signature
> > assuming grub-editenv (or some other tool) computed one. When do you
> > need to load environment more than once?
> >
> 
> I agree that the default grub.cfg behaves such as you describe, but
> consider a configuration where the signing key is not available during
> every boot cycle.  E.g., it is password-protected by a password that I
> know, but that other users of the machine do not know.  Let's assume it's a
> server in a physically secure location so that, e.g., booting from a CD or
> USB drive is not a viable attack.  Let us also assume that the attacker may
> gain root privileges in the OS at some point after the bootloader
> configuration is completed and the signing key is secured.
> 
> Now suppose I want to enable "savedefault" functionality, so that users can
> control which of several installed OSes, kernels, kernel configurations,
> ... to boot.  I don't care which configuration boots, or which one is the
> default, but I want to make sure the machine only boots known
> configurations.
> 

So just use another environment block for untrusted variables, that's
all. I do not see why any change in sources is required.

Now if you could come up with solution that maintains compatibility
with existing grub.cfg, that would be valid reason. But right now
grub.cfg must be changed anyway at which point just save untrusted
variables separately from trusted. 

> If I attempt such a configuration today, with check_signatures=enforce, it
> becomes impossible to save_env any kind of default (or next_entry, or
> boot_once) variable.  I might try to cope with this by writing my (signed)
> grub.cfg to temporarily disable check_signatures:
> 
> set check_signatures=no
> save_env next_entry boot_once prev_next_entry
> set check_signatures=yes
> 
> Unfortunately, the grubenv file has now changed.  If it was previously
> signed, that signature is no longer valid.  Thus, in order to load in any
> environment variables (on the next reboot), I again have to disable
> check_signatures.
> 
> set check_signatures=no
> load_env
> set check_signatures=yes
> 
> In the default commodity grub.cfg, where load_env is called exactly once at
> the top, I agree that this is not an especially serious limitation
> (although changing, e.g., root or prefix could lead to availability
> problems, but an attacker who can modify grubenv (i.e., they're root) can
> already cause availability problems).
> 
> However, this opens up an attack for any more complex configurations:  I
> have no idea what variables an attacker may have written into the grubenv
> file during the previous boot.  Thus, after an untrusted load_env, I must
> write the remainder of my grub.cfg not to depend on any pre-existing
> variables that may now be attacker-controlled.  An untrusted load_env is
> essentially a barrier across which any already-set environment variables
> have to be discarded.  There's no way to securely save and restore them
> across the untrusted load_env (as an attacker might guess the "saved_foo"
> name or simply read it out of the .cfg file).
> 
> This "barrier" unnecessarily constrains the author of grub.cfg, and
> negatively impacts more complex configurations.  Consider, e.g., using
> multiple different signing keys to cover different configuration files that
> are sourced by a primary configuration file, each of which may have their
> own associated grubenv file.  Such a configuration cannot be created
> securely with today's load_env and save_env.  My patch is intended to
> remedy situations such as this.
> 
> Perhaps it is desirable to eliminate the changes to load_env and save_env
> that control whether the PUBKEY filter is active, and simply add the
> ability to specify that only certain variables will be set based on the
> arguments to a load_env command?  This corresponds to the existing
> functionality of save_env.  This would require an idiom in grub.cfg that
> calls to save_env and load_env (if a previous save_env could have possibly
> modified the relevant grubenv file) are wrapped in explicit disabling of
> signature checking:
> 
> check_signatures=no
> save_env foo bar baz
> check_signatures=enforce
> 
> ...
> 
> set check_signatures=no
> load_env foo bar baz
> set check_signatures=enforce
> 
> However, without my modifications (or something else offering equivalent
> functionality) to load_env, I don't see a way for it to be possible to
> securely load only selected configuration settings from more than one
> untrusted environment file.
> 
> I hope this is clear, and I'm certainly open to alternatives if there's a
> simpler approach available.
> 
> Thanks,
> -Jon