[U-Boot] efi_loader: implementing non-volatile UEFI variables

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Tue, Jun 25, 2019 at 08:33:30AM +0200, Wolfgang Denk wrote:
> Dear Takahiro,
> 
> In message <20190625011039.GO6610@linaro.org> you wrote:
> >
> > > Think about secure boot. It is a bad idea to expose variables in this way.
> >
> > Actually, we are thinking of disabling U-Boot environment (I mean,
> > ENV_IS_NOWHERE) still allowing for UEFI variables for security reason.
> 
> OK.  I know of systems that have passed security audits with
> U-Boot environment enabled.  In those cases it as sufficient to make
> a certain set of variables immutable.
> 
> Also, adding the new "volatile" attribute to the environment flags
> as I suggested would allow to fine-tune which variables get stored
> in the persistent nvironment copy at all.
> 
> > One of the differences between U-Boot env and UEFI variables
> > is that the former can and should be saved to backing storage
> > only with explicit "saveenv" command, while the latter are
> > expected to be saved immediately.
> 
> OK - but this does not conflict in any way with the U-Boot
> environment concept.  In the same way we can add a "volatile"
> property to the flags, we can add something like an "autosave"
> property which would cause that aany modification of the variable
> value would automatically run "env save".
> 
> Once you think about more generic features of the property flags you
> can implement all kinds of clever/useful/fancy things.  For example,
> one could think of a "protected" property, which would require
> password entry to change the value, etc.
> 
> Of course you could also flag variables with an "UEFI" attribute and
> add all kinds of wanted behaviour.
> 
> > Preserving respective semantics simultaneously would be possible, but
> > it would make the implementation a bit complicated and ugly.
> 
> It does not have to be ugly, and I think it is also not so
> complicatred.  In any case it seems more attractive to me than
> adding a completly separate, new implementation for variable
> storage.

Really? Take an example:
U-Boot (non-volatile) variable a: value: A
U-Boot (non-volatile) variable b: value: B
UEFI non-volatile variable c: value: C

Then,
1) user changed b's value to B', but didn't want to save it
2) user changed c's value to C'

How should U-Boot behave here?
3)
- hold B as well as B' and marks b as "modified, but not saved"
- search the entire U-Boot environment, create a temporary buffer
  of {a=A, b=B, c=C'} and save it to backing storage
If user does "env save" later,
- discard B (and save {a=A, b=B', c=C'})

I said this is ugly and complicated.

> > Instead, I believe that it will be a clever idea that we should have
> > separate contexts for them as I showed in my non-volatile support patch[1].
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2019-June/371601.html
> 
> To be honest: I thinkt his is not really a clever approach.  it
> would be _much_ easier to add a "volatile" property to the U-Boot
> variable flags.  Your patch modifies 12 files, and adds more than
> 600 lines of code.  And since you're modifying   env/fat.c  I have
> the impression that this is not even universally usable - does it
> really work only when storing the environment in a FAT file system?
> Not with ext4? Or any other storage? And it works only for UEFI, but
> "normal" environment variables do not benefit from this?
> 
> Note that my suggested extension of the variable flags would be
> completely agnostic of this...
> 
> > One of possible improvements here is to refactor the env code and
> > parameterize "contexts" at env_save()/env_load().
> 
> Contexts, or flags.  But this does not require much refactoring.  It
> should be a straighforward extension.

Contexts != flags, I mean, by Context, a separate "env_t" data
with interfaces like env_save(&env_efi)/env_load(&env_efi).
I don't think that it is a "different implementation."

I don't know why you stick to a "single" storage, but if you don't
see the implementation above(3) as ugly, that's fine. I can take it
since it's is doable anyway.

Thanks,
-Takahiro Akashi


> > > >And I agree on that.  But even if it was not the case, having four
> > > >different implementations for the same thing is .... sub-optiman.
> > > 
> > > We have a lot of things that can be disabled in U-Boot. Why should we
> > > not be able to disable UEFI variables?
> >
> > To be honest, I'm a bit doubtful about practical meaning of
> > disabling UEFI variables for UEFI execution environment.
> 
> This was Heinrich's argument, and I admit that I didn't understandit
> either.
> 
> > > UEFI variables should be accessible via the UEFI runtime API and not via
> > > some U-Boot specific hack which no other program but U-Boot cares about.
> >
> > Please notice that one of the reasons that prevents UEFI variables
> > from being accessed by OS is a real hardware(device/controller) may be
> > shared between firmware(i.e. UEFI runtime) and OS and mutually exclusive
> > access must be ensured.
> 
> Again, this was Heinrich's argument.
> 
> Best regards,
> 
> Wolfgang Denk
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> There are no data that cannot be plotted on a straight  line  if  the
> axis are chosen correctly.