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@denx.de There are no data that cannot be plotted on a straight line if the axis are chosen correctly.