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

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

[Heinrich Schuchardt]

Hello Ilias,

thanks a lot for the good online meeting this morning together with your
colleague Suggan where we discussed the requirements for the
implementation of non-volatile variables in U-Boot.

Currently UEFI variables are stored in U-Boot variables. Saving the
U-Boot variables will persist all UEFI variables in the environment both
volatile and non-volatile. This does not conform the the UEFI standard.

To provide a secure storage Linaro is considering to implement an OP-TEE
module for variable storage and as alternative to this OP-TEE module a
standalone MM service which will be a BL32 ATF module.

So in future we will have possibly three alternative drivers for UEFI
variables:

- U-Boot only implementation
  (what is now in lib/efi_loader/efi_variable.c)
- OP-TEE module
- standalone MM service

And maybe a fourth dummy one implementing no variable service at all.

To make the OP-TEE module and the standalone MM service usable in EDK2
too they will provide implementations for GetVariable(),
GetNextVariable(), QueryVariable(), and SetVariable() both for volatile
and non-volatile variables.

The U-Boot only implementation currently provides these variables only
at boottime. Takahiro sent patches with a runtime cache enabling
GetVariable(), GetNextVariable() and possibly in future QueryVariable()
but not SetVariable().

Currently when reaching SetVirtualAddressMap() we patch the runtime
table to replace the variable services by dummy functions returning
EFI_UNSUPPORTED (or at least this should be the return value). Alex
delivered a patch that will move this patching to ExitBootServices() to
comply with the requirements of operating systems which do not call
SetVirtualAddressMap().

As in future we will have three different drivers for variables and
these will differ in which runtime services they support it makes sense
to move the obligation of patching the runtime services table to the
variable driver itself. We already have implemented the event group
EFI_EVENT_GROUP_EXIT_BOOT_SERVICES. So if the driver creates an event
for this group it can do the patching in the notification function of
the event.

I will update the current variables implementation to take care of the
runtime services table patching for variable services using
EFI_EVENT_GROUP_EXIT_BOOT_SERVICES and add a null driver. This will
provide the templates for the U-Boot side implementation of the OP-TEE
and standalone MM service solution.

Once we have that API right we can start moving in Takahiros patches for
separating UEFI and U-Boot variables and creating a runtime cache.

Best regards

Heinrich

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

[Ilias Apalodimas]

Hi Heinrich,

(+ Sughosh and Francois)


On Thu, 20 Jun 2019 at 11:06, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Hello Ilias,
>
> thanks a lot for the good online meeting this morning together with your
> colleague Suggan where we discussed the requirements for the
> implementation of non-volatile variables in U-Boot.
>
> Currently UEFI variables are stored in U-Boot variables. Saving the
> U-Boot variables will persist all UEFI variables in the environment both
> volatile and non-volatile. This does not conform the the UEFI standard.
>
> To provide a secure storage Linaro is considering to implement an OP-TEE
> module for variable storage and as alternative to this OP-TEE module a
> standalone MM service which will be a BL32 ATF module.
>
> So in future we will have possibly three alternative drivers for UEFI
> variables:
>
> - U-Boot only implementation
>   (what is now in lib/efi_loader/efi_variable.c)
> - OP-TEE module
> - standalone MM service
StandaloneMM will run as an application (trusted application or in
short TA) in OP-TEE.
So this two options are actually one.
Please note that this approach will offer identical functionality for
Armv7 and Armv8 architectures.
The current StandaloneMM implementation in EDKII run in SPM (and not
OP-TEE) so it's only usable on armv8

>
> And maybe a fourth dummy one implementing no variable service at all.
>
> To make the OP-TEE module and the standalone MM service usable in EDK2
> too they will provide implementations for GetVariable(),
> GetNextVariable(), QueryVariable(), and SetVariable() both for volatile
> and non-volatile variables.
>
> The U-Boot only implementation currently provides these variables only
> at boottime. Takahiro sent patches with a runtime cache enabling
> GetVariable(), GetNextVariable() and possibly in future QueryVariable()
> but not SetVariable().
Correct, we are currently considering not enabling SetVariable in
runtime, but only update UEFI variables on next-boot

>
> Currently when reaching SetVirtualAddressMap() we patch the runtime
> table to replace the variable services by dummy functions returning
> EFI_UNSUPPORTED (or at least this should be the return value). Alex
> delivered a patch that will move this patching to ExitBootServices() to
> comply with the requirements of operating systems which do not call
> SetVirtualAddressMap().
>
> As in future we will have three different drivers for variables and
> these will differ in which runtime services they support it makes sense
> to move the obligation of patching the runtime services table to the
> variable driver itself. We already have implemented the event group
> EFI_EVENT_GROUP_EXIT_BOOT_SERVICES. So if the driver creates an event
> for this group it can do the patching in the notification function of
> the event.
>
> I will update the current variables implementation to take care of the
> runtime services table patching for variable services using
> EFI_EVENT_GROUP_EXIT_BOOT_SERVICES and add a null driver. This will
> provide the templates for the U-Boot side implementation of the OP-TEE
> and standalone MM service solution.
>
> Once we have that API right we can start moving in Takahiros patches for
> separating UEFI and U-Boot variables and creating a runtime cache.
>
> Best regards
>
> Heinrich

Thanks for the meeting. If you need any future sync-ups please let me know

Regards
/Ilias

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

[Wolfgang Denk]

Dear Heinrich,

In message <83a2d8c5-1b06-c502-8a63-dc3ca307deb8@gmx.de> you wrote:
> 
> thanks a lot for the good online meeting this morning together with your
> colleague Suggan where we discussed the requirements for the
> implementation of non-volatile variables in U-Boot.
> 
> Currently UEFI variables are stored in U-Boot variables. Saving the
> U-Boot variables will persist all UEFI variables in the environment both
> volatile and non-volatile. This does not conform the the UEFI standard.

Is this the only problem of using the environment as storage?

> To provide a secure storage Linaro is considering to implement an OP-TEE
> module for variable storage and as alternative to this OP-TEE module a
> standalone MM service which will be a BL32 ATF module.
> 
> So in future we will have possibly three alternative drivers for UEFI
> variables:
> 
> - U-Boot only implementation
>   (what is now in lib/efi_loader/efi_variable.c)
> - OP-TEE module
> - standalone MM service
> 
> And maybe a fourth dummy one implementing no variable service at all.

Is this really a good idea?


If the volatile / non-volatile behaviour is the onlyh problem you
see with using environment variables, would it then not make much
more sense to extend the environment flags concept by adding a
"volatile" flag, with the meaing that such variables don;t get
written by "env save"?

This would also make sense in some other places - for example, the
"filesize" variable is a perfect candidate to be flagged as
"volatile".  There is absolutely no use in saving it persistently.


So such an extension would be useful for others, too, and might
eventually avoid so many different implementations for the same
task?  Also, the implementation should be straightforward...

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
Accident: A condition in which presence of mind is good, but  absence
of body is better.

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

[Heinrich Schuchardt]

On 6/24/19 12:23 PM, Wolfgang Denk wrote:
> Dear Heinrich,
>
> In message <83a2d8c5-1b06-c502-8a63-dc3ca307deb8@gmx.de> you wrote:
>>
>> thanks a lot for the good online meeting this morning together with your
>> colleague Suggan where we discussed the requirements for the
>> implementation of non-volatile variables in U-Boot.
>>
>> Currently UEFI variables are stored in U-Boot variables. Saving the
>> U-Boot variables will persist all UEFI variables in the environment both
>> volatile and non-volatile. This does not conform the the UEFI standard.
>
> Is this the only problem of using the environment as storage?

Dear Wolfgang,

to be really useful UEFI variables should be available to the operating
system. This is not possible using U-Boot variables as storage.

>
>> To provide a secure storage Linaro is considering to implement an OP-TEE
>> module for variable storage and as alternative to this OP-TEE module a
>> standalone MM service which will be a BL32 ATF module.
>>
>> So in future we will have possibly three alternative drivers for UEFI
>> variables:
>>
>> - U-Boot only implementation
>>    (what is now in lib/efi_loader/efi_variable.c)
>> - OP-TEE module
>> - standalone MM service
>>
>> And maybe a fourth dummy one implementing no variable service at all.
>
> Is this really a good idea?

Tom is always complaining that the UEFI subsystem has become too large.
As GRUB and Linux boot without UEFI variables they are one possibility
to size down the image. But I would not choose this config option by
default.

>
> If the volatile / non-volatile behaviour is the only problem you
> see with using environment variables, would it then not make much
> more sense to extend the environment flags concept by adding a
> "volatile" flag, with the meaning that such variables don't get
> written by "env save"?

The best solution for UEFI variables would be to live in the secure part
of the ARM trusted firmware.

UEFI variables have both a name and a GUID as keys. Furthermore both
variable names and values are in UTF-16. So they are quite different to
U-Boot variables.

Look at this output:

=> printenv

efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported={boot,run}(blob)0000000000000000
efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_PlatformLang={nv,boot,run}(blob)656e2d555300
efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_PlatformLangCodes={boot,run}(blob)656e2d555300
efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_RuntimeServicesSupported={boot,run}(blob)8004

=> printenv -e

OsIndicationsSupported: BS|RT, DataSize = 0x8
     00000000: 00 00 00 00 00 00 00 00                          ........
PlatformLang: NV|BS|RT, DataSize = 0x6
     00000000: 65 6e 2d 55 53 00                                en-US.
PlatformLangCodes: BS|RT, DataSize = 0x6
     00000000: 65 6e 2d 55 53 00                                en-US.
RuntimeServicesSupported: BS|RT, DataSize = 0x2
     00000000: 80 04                                            ..

>
> This would also make sense in some other places - for example, the
> "filesize" variable is a perfect candidate to be flagged as
> "volatile".  There is absolutely no use in saving it persistently.
>
> So such an extension would be useful for others, too, and might
> eventually avoid so many different implementations for the same
> task?  Also, the implementation should be straightforward...
>

This is worthwhile but insufficient to solve the UEFI problems.

Best regards

Heinrich

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

[Wolfgang Denk]

Dear Heinrich,

In message <7083d208-4b3c-7261-a03b-9066dc8d2009@gmx.de> you wrote:
>
> to be really useful UEFI variables should be available to the operating
> system. This is not possible using U-Boot variables as storage.

Oops?  Is it possible that you are not aware of the
tools/env/fw_env* code?

Of course U-Boot environment variables can be accessed (read and
written) from Linux.

> >> And maybe a fourth dummy one implementing no variable service at all.
> >
> > Is this really a good idea?
>
> Tom is always complaining that the UEFI subsystem has become too large.

And I agree on that.  But even if it was not the case, having four
different implementations for the same thing is .... sub-optiman.

And if the "volatile" feature is all that is missing when using
environment variables, then this should be the road taken:

- It avoids a complete new implementation
- It can be added witrh very little effort
- It would be just a minimal increase in memory footprinmt
- It is useful for a lot of other pirposes as well.

What are your agruments for _not_ taking this approach?

> The best solution for UEFI variables would be to live in the secure part
> of the ARM trusted firmware.

You think too narrow. Not all the world is ARM, and not everyone
uses ATF.

> UEFI variables have both a name and a GUID as keys.

Internally U-Boot variables are stored in a hash table; you could
easily add GUID support.  the external storage format is also simple
enough.  You could extend it in a compatible way for example from

	<name>=<value>\0

to something like

	<name>;<guid>=<value>\0

of, if you don't want to waste another separator character, even

	<name>=<guid>=<value>\0

Extending the env import parser and the env export code for this
does not look like rocket science either.

> Furthermore both
> variable names and values are in UTF-16. So they are quite different to
> U-Boot variables.

Nothing in U-Boot prevents this.  There is a number of ways to
convert UTF-16 (or even any kind of binary data) to and from plain
ASCII strings, and only your UEFI code needs to understand such
encoding.

> Look at this output:
>
> -> printenv
>
> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported={boot,run}(blob)0000000000000000
> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_PlatformLang={nv,boot,run}(blob)656e2d555300
> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_PlatformLangCodes={boot,run}(blob)656e2d555300
> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_RuntimeServicesSupported={boot,run}(blob)8004
>
> => printenv -e
>
> OsIndicationsSupported: BS|RT, DataSize = 0x8
>      00000000: 00 00 00 00 00 00 00 00                          ........
> PlatformLang: NV|BS|RT, DataSize = 0x6
>      00000000: 65 6e 2d 55 53 00                                en-US.
> PlatformLangCodes: BS|RT, DataSize = 0x6
>      00000000: 65 6e 2d 55 53 00                                en-US.
> RuntimeServicesSupported: BS|RT, DataSize = 0x2
>      00000000: 80 04                                            ..

I'm looking at it, but I don't understand what you want to show me
here?

All I can see is that this looks like something I would not want to
use (read: I consider it ugly and think the design should be
improved).

> This is worthwhile but insufficient to solve the UEFI problems.

Could you please be so kine and clearly state what is lacking?

So far I have found:

- lack of "volatile" vs. "non-volatile"
- lack of support for GUID
- lack of support for UTF-16

None of these should be difficult to add by small (or even minimal)
extensions of the existing code, which would probaly also useful to
others (at least some of them).

So far, I see no killing point not an advantage that would justify
adding a completly new implementation for a problem that has been
solved before.

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
Everyone, in some small sacred sanctuary of the self, is nuts.
                                                         - Leo Rosten

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

[Heinrich Schuchardt]

On 6/24/19 8:50 PM, Wolfgang Denk wrote:
> Dear Heinrich,
>
> In message <7083d208-4b3c-7261-a03b-9066dc8d2009@gmx.de> you wrote:
>>
>> to be really useful UEFI variables should be available to the operating
>> system. This is not possible using U-Boot variables as storage.
>
> Oops?  Is it possible that you are not aware of the
> tools/env/fw_env* code?

I am fully aware of this.

Think about secure boot. It is a bad idea to expose variables in this way.

>
> Of course U-Boot environment variables can be accessed (read and
> written) from Linux.
>
>>>> And maybe a fourth dummy one implementing no variable service at all.
>>>
>>> Is this really a good idea?
>>
>> Tom is always complaining that the UEFI subsystem has become too large.
>
> 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?

What Linaro is doing with OP-TEE makes sense to enable secure boot. But
it will be an ARM64 specific solution.

>
> And if the "volatile" feature is all that is missing when using
> environment variables, then this should be the road taken:
>
> - It avoids a complete new implementation
> - It can be added witrh very little effort
> - It would be just a minimal increase in memory footprinmt
> - It is useful for a lot of other pirposes as well.
>
> What are your agruments for _not_ taking this approach?

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.

Best regards

Heinrich

>
>> The best solution for UEFI variables would be to live in the secure part
>> of the ARM trusted firmware.
>
> You think too narrow. Not all the world is ARM, and not everyone
> uses ATF.
>
>> UEFI variables have both a name and a GUID as keys.
>
> Internally U-Boot variables are stored in a hash table; you could
> easily add GUID support.  the external storage format is also simple
> enough.  You could extend it in a compatible way for example from
>
> 	<name>=<value>\0
>
> to something like
>
> 	<name>;<guid>=<value>\0
>
> of, if you don't want to waste another separator character, even
>
> 	<name>=<guid>=<value>\0
>
> Extending the env import parser and the env export code for this
> does not look like rocket science either.
>
>> Furthermore both
>> variable names and values are in UTF-16. So they are quite different to
>> U-Boot variables.
>
> Nothing in U-Boot prevents this.  There is a number of ways to
> convert UTF-16 (or even any kind of binary data) to and from plain
> ASCII strings, and only your UEFI code needs to understand such
> encoding.
>
>> Look at this output:
>>
>> -> printenv
>>
>> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported={boot,run}(blob)0000000000000000
>> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_PlatformLang={nv,boot,run}(blob)656e2d555300
>> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_PlatformLangCodes={boot,run}(blob)656e2d555300
>> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_RuntimeServicesSupported={boot,run}(blob)8004
>>
>> => printenv -e
>>
>> OsIndicationsSupported: BS|RT, DataSize = 0x8
>>       00000000: 00 00 00 00 00 00 00 00                          ........
>> PlatformLang: NV|BS|RT, DataSize = 0x6
>>       00000000: 65 6e 2d 55 53 00                                en-US.
>> PlatformLangCodes: BS|RT, DataSize = 0x6
>>       00000000: 65 6e 2d 55 53 00                                en-US.
>> RuntimeServicesSupported: BS|RT, DataSize = 0x2
>>       00000000: 80 04                                            ..
>
> I'm looking at it, but I don't understand what you want to show me
> here?
>
> All I can see is that this looks like something I would not want to
> use (read: I consider it ugly and think the design should be
> improved).
>
>> This is worthwhile but insufficient to solve the UEFI problems.
>
> Could you please be so kine and clearly state what is lacking?
>
> So far I have found:
>
> - lack of "volatile" vs. "non-volatile"
> - lack of support for GUID
> - lack of support for UTF-16
>
> None of these should be difficult to add by small (or even minimal)
> extensions of the existing code, which would probaly also useful to
> others (at least some of them).
>
> So far, I see no killing point not an advantage that would justify
> adding a completly new implementation for a problem that has been
> solved before.
>
> Best regards,
>
> Wolfgang Denk
>

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

[AKASHI Takahiro]

On Mon, Jun 24, 2019 at 09:10:07PM +0200, Heinrich Schuchardt wrote:
> On 6/24/19 8:50 PM, Wolfgang Denk wrote:
> >Dear Heinrich,
> >
> >In message <7083d208-4b3c-7261-a03b-9066dc8d2009@gmx.de> you wrote:
> >>
> >>to be really useful UEFI variables should be available to the operating
> >>system. This is not possible using U-Boot variables as storage.
> >
> >Oops?  Is it possible that you are not aware of the
> >tools/env/fw_env* code?
> 
> I am fully aware of this.

Oops, I don't know, but

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

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.

Preserving respective semantics simultaneously would be possible, but
it would make the implementation a bit complicated and ugly.
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

One of possible improvements here is to refactor the env code and
parameterize "contexts" at env_save()/env_load().

> >
> >Of course U-Boot environment variables can be accessed (read and
> >written) from Linux.
> >
> >>>>And maybe a fourth dummy one implementing no variable service at all.
> >>>
> >>>Is this really a good idea?
> >>
> >>Tom is always complaining that the UEFI subsystem has become too large.
> >
> >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.

> What Linaro is doing with OP-TEE makes sense to enable secure boot. But
> it will be an ARM64 specific solution.
> 
> >
> >And if the "volatile" feature is all that is missing when using
> >environment variables, then this should be the road taken:
> >
> >- It avoids a complete new implementation
> >- It can be added witrh very little effort
> >- It would be just a minimal increase in memory footprinmt
> >- It is useful for a lot of other pirposes as well.
> >
> >What are your agruments for _not_ taking this approach?

See my comment above.

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

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> >
> >>The best solution for UEFI variables would be to live in the secure part
> >>of the ARM trusted firmware.
> >
> >You think too narrow. Not all the world is ARM, and not everyone
> >uses ATF.
> >
> >>UEFI variables have both a name and a GUID as keys.
> >
> >Internally U-Boot variables are stored in a hash table; you could
> >easily add GUID support.  the external storage format is also simple
> >enough.  You could extend it in a compatible way for example from
> >
> >	<name>=<value>\0
> >
> >to something like
> >
> >	<name>;<guid>=<value>\0
> >
> >of, if you don't want to waste another separator character, even
> >
> >	<name>=<guid>=<value>\0
> >
> >Extending the env import parser and the env export code for this
> >does not look like rocket science either.
> >
> >>Furthermore both
> >>variable names and values are in UTF-16. So they are quite different to
> >>U-Boot variables.
> >
> >Nothing in U-Boot prevents this.  There is a number of ways to
> >convert UTF-16 (or even any kind of binary data) to and from plain
> >ASCII strings, and only your UEFI code needs to understand such
> >encoding.
> >
> >>Look at this output:
> >>
> >>-> printenv
> >>
> >>efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported={boot,run}(blob)0000000000000000
> >>efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_PlatformLang={nv,boot,run}(blob)656e2d555300
> >>efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_PlatformLangCodes={boot,run}(blob)656e2d555300
> >>efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_RuntimeServicesSupported={boot,run}(blob)8004
> >>
> >>=> printenv -e
> >>
> >>OsIndicationsSupported: BS|RT, DataSize = 0x8
> >>      00000000: 00 00 00 00 00 00 00 00                          ........
> >>PlatformLang: NV|BS|RT, DataSize = 0x6
> >>      00000000: 65 6e 2d 55 53 00                                en-US.
> >>PlatformLangCodes: BS|RT, DataSize = 0x6
> >>      00000000: 65 6e 2d 55 53 00                                en-US.
> >>RuntimeServicesSupported: BS|RT, DataSize = 0x2
> >>      00000000: 80 04                                            ..
> >
> >I'm looking at it, but I don't understand what you want to show me
> >here?
> >
> >All I can see is that this looks like something I would not want to
> >use (read: I consider it ugly and think the design should be
> >improved).
> >
> >>This is worthwhile but insufficient to solve the UEFI problems.
> >
> >Could you please be so kine and clearly state what is lacking?
> >
> >So far I have found:
> >
> >- lack of "volatile" vs. "non-volatile"
> >- lack of support for GUID
> >- lack of support for UTF-16
> >
> >None of these should be difficult to add by small (or even minimal)
> >extensions of the existing code, which would probaly also useful to
> >others (at least some of them).
> >
> >So far, I see no killing point not an advantage that would justify
> >adding a completly new implementation for a problem that has been
> >solved before.
> >
> >Best regards,
> >
> >Wolfgang Denk
> >
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

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

[Heinrich Schuchardt]

On 6/25/19 3:10 AM, AKASHI Takahiro wrote:
> On Mon, Jun 24, 2019 at 09:10:07PM +0200, Heinrich Schuchardt wrote:
>> On 6/24/19 8:50 PM, Wolfgang Denk wrote:
>>> Dear Heinrich,
>>>
>>> In message <7083d208-4b3c-7261-a03b-9066dc8d2009@gmx.de> you wrote:
>>>>
>>>> to be really useful UEFI variables should be available to the operating
>>>> system. This is not possible using U-Boot variables as storage.
>>>
>>> Oops?  Is it possible that you are not aware of the
>>> tools/env/fw_env* code?
>>
>> I am fully aware of this.
>
> Oops, I don't know, but
>
>> 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.
>
> 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.
>
> Preserving respective semantics simultaneously would be possible, but
> it would make the implementation a bit complicated and ugly.
> 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
>
> One of possible improvements here is to refactor the env code and
> parameterize "contexts" at env_save()/env_load().
>
>>>
>>> Of course U-Boot environment variables can be accessed (read and
>>> written) from Linux.
>>>
>>>>>> And maybe a fourth dummy one implementing no variable service at all.
>>>>>
>>>>> Is this really a good idea?
>>>>
>>>> Tom is always complaining that the UEFI subsystem has become too large.
>>>
>>> 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.

I will drop
https://lists.denx.de/pipermail/u-boot/2019-June/373915.html
"efi_debug: make variable support customizable"
to stop that discussion. We still can introduce the customization option
once the OP-TEE option has been worked out.

Concerning your patch
https://lists.denx.de/pipermail/u-boot/2019-June/371602.html
"env: save UEFI non-volatile variables in dedicated storage"
What do you think about Wolfgang's propositions below to adjust
env_save() to provide a volatile/non-volatile flag for normal U-Boot
variables?

Best regards

Heinrich

>
>> What Linaro is doing with OP-TEE makes sense to enable secure boot. But
>> it will be an ARM64 specific solution.
>>
>>>
>>> And if the "volatile" feature is all that is missing when using
>>> environment variables, then this should be the road taken:
>>>
>>> - It avoids a complete new implementation
>>> - It can be added witrh very little effort
>>> - It would be just a minimal increase in memory footprinmt
>>> - It is useful for a lot of other pirposes as well.
>>>
>>> What are your agruments for _not_ taking this approach?
>
> See my comment above.
>
>> 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.
>
> -Takahiro Akashi
>
>> Best regards
>>
>> Heinrich
>>
>>>
>>>> The best solution for UEFI variables would be to live in the secure part
>>>> of the ARM trusted firmware.
>>>
>>> You think too narrow. Not all the world is ARM, and not everyone
>>> uses ATF.
>>>
>>>> UEFI variables have both a name and a GUID as keys.
>>>
>>> Internally U-Boot variables are stored in a hash table; you could
>>> easily add GUID support.  the external storage format is also simple
>>> enough.  You could extend it in a compatible way for example from
>>>
>>> 	<name>=<value>\0
>>>
>>> to something like
>>>
>>> 	<name>;<guid>=<value>\0
>>>
>>> of, if you don't want to waste another separator character, even
>>>
>>> 	<name>=<guid>=<value>\0
>>>
>>> Extending the env import parser and the env export code for this
>>> does not look like rocket science either.
>>>
>>>> Furthermore both
>>>> variable names and values are in UTF-16. So they are quite different to
>>>> U-Boot variables.
>>>
>>> Nothing in U-Boot prevents this.  There is a number of ways to
>>> convert UTF-16 (or even any kind of binary data) to and from plain
>>> ASCII strings, and only your UEFI code needs to understand such
>>> encoding.
>>>
>>>> Look at this output:
>>>>
>>>> -> printenv
>>>>
>>>> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported={boot,run}(blob)0000000000000000
>>>> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_PlatformLang={nv,boot,run}(blob)656e2d555300
>>>> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_PlatformLangCodes={boot,run}(blob)656e2d555300
>>>> efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_RuntimeServicesSupported={boot,run}(blob)8004
>>>>
>>>> => printenv -e
>>>>
>>>> OsIndicationsSupported: BS|RT, DataSize = 0x8
>>>>       00000000: 00 00 00 00 00 00 00 00                          ........
>>>> PlatformLang: NV|BS|RT, DataSize = 0x6
>>>>       00000000: 65 6e 2d 55 53 00                                en-US.
>>>> PlatformLangCodes: BS|RT, DataSize = 0x6
>>>>       00000000: 65 6e 2d 55 53 00                                en-US.
>>>> RuntimeServicesSupported: BS|RT, DataSize = 0x2
>>>>       00000000: 80 04                                            ..
>>>
>>> I'm looking at it, but I don't understand what you want to show me
>>> here?
>>>
>>> All I can see is that this looks like something I would not want to
>>> use (read: I consider it ugly and think the design should be
>>> improved).
>>>
>>>> This is worthwhile but insufficient to solve the UEFI problems.
>>>
>>> Could you please be so kine and clearly state what is lacking?
>>>
>>> So far I have found:
>>>
>>> - lack of "volatile" vs. "non-volatile"
>>> - lack of support for GUID
>>> - lack of support for UTF-16
>>>
>>> None of these should be difficult to add by small (or even minimal)
>>> extensions of the existing code, which would probaly also useful to
>>> others (at least some of them).
>>>
>>> So far, I see no killing point not an advantage that would justify
>>> adding a completly new implementation for a problem that has been
>>> solved before.
>>>
>>> Best regards,
>>>
>>> Wolfgang Denk
>>>
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
>

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

[Wolfgang Denk]

Dear Heinrich,

In message <63fcd992-2d43-d168-64e8-7058c3a244f8@gmx.de> you wrote:
>
> > Oops?  Is it possible that you are not aware of the
> > tools/env/fw_env* code?
>
> I am fully aware of this.

OK, then why do you claim that U-Boot nvironment variables are not
"available to the operating system"?

> Think about secure boot. It is a bad idea to expose variables in this way.

Would you please be so kind to elucidate what the problem is?

> > 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?


Your way of replying is not very constructive.  You don't even try
to comment on my argument, but instead you bring up a completely new
thing, and without explaining why that would be a problem.

See above:

You claim U-Boot environment variables cannot be accessed by Linux.
I tell you they can. You say it's bad for secure boot, but fail to
explain why that would be tha case.

Here I argument that 4 different implementations of the same thing
are a bad idea.  You ignore that argument and jump to a new one,
about disabling UEFI variables.  Why do you need another variable
implementation for that?

Please try and be a bit more constructive and helpful in your
argumentation.  I don't doubt that you have spent a lot of thought
already on this, but then plase explain why you need this thing and
why something already existing cannot be used.  Explain it such that
someone without intimate UEFI knowledge can understand it, too.


> What Linaro is doing with OP-TEE makes sense to enable secure boot. But
> it will be an ARM64 specific solution.

What exactly do you want to tell us with that statement?

> > And if the "volatile" feature is all that is missing when using
> > environment variables, then this should be the road taken:
> >
> > - It avoids a complete new implementation
> > - It can be added witrh very little effort
> > - It would be just a minimal increase in memory footprinmt
> > - It is useful for a lot of other pirposes as well.
> >
> > What are your agruments for _not_ taking this approach?
>
> 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.

I don't care what other programs are doing.  But I do care what we
do in U-Boot.  And you talk about adding a ton of multi-redundant
( 4 times, as you said yourself) code to U-Boot, but the arguments
you bring up why exactly this is needed and why it cannot be done
the U-Boot way are pretty weak.

Sorry, but this is not really convincing.

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
Conscious is when you are aware of something, and conscience is  when
you wish you weren't.

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

[Wolfgang Denk]

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.

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

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

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

[AKASHI Takahiro]

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.

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

[Wolfgang Denk]

Dear Takahiro,

In message <20190625075931.GP6610@linaro.org> you wrote:
>
> > 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?

You did not complete the scenario, but I guess you mean that the
user _wants_ to save 'c' to persistent storage?

Yes, the current implementation does not allow this.

But if you extend the existing environment variable flags by some
new properties you can have all this easily:

a) add a "volatile" flag.

   In the above setup you should then mark 'b' as volatile, so it
   does not get saved to the environment.

b) add a "UEFI" flag (*) and extend "env save" and "env export"
   to respect such flags (ideally both for a "include only these"
   and an "exclude these" operation).

   Then you can do a _lot_ of new things.

   You can for example save _only_ the UEFI variable to persistent
   storage - which would implement the feature you requested before
   where you said you would like to set CONFIG_ENV_IS_NOWHERE.

   Alternatively, you could use "env save" to only save the "normal"
   U-Boot environment, and "env export" to extract only the UEFI
   variables and then use a file write to store these elsewhere.

   You can even go a step further and define separate storage areas
   so "env save" will do this automatically for you (more cosing
   effort, easier to use).

   (*) For a start and just to fulfil your requirement, such a
   simple "UEFI" flag would be sufficient.  If you think further,
   you could as well implement a flag that takes a string value,
   so you can define any number of different variable contexts that
   can all be managed using the existing code, but kept strictly
   separate else.  There is a zillion of potential use cases.

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

What you describe in 3) is indeed ugly, but it is also not
necessary.  There are other, more intelligent ways to implement
that.  See above for a proposal.

> > 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."

See above.  If you think small (and go for minimal coding efforts),
just add a UEFI flag now to provide a UEFI context.  If tomorrow
someone needs a FOO context, add a FOO flag.  We did not really need
such context based variable handlint for the last 2 decades, so we
can probably go on like this for a couple of years.

If you think bit and are willing to pay a little more efforts for a
more generic, runtime extensible solution, than add a context flag
that takes a string as value, and you can define as many contexts as
you have memory for.  And you can use them all through the same
existing tools, and all this is independent of the storage of the
persistent copies.  and yes, of course you can have separate storage
for each of the "context" groups.

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

I do not stick to a single storage, on contrary.  And yes, 3) _is_
ugly.  But there should be better ways to implement this.

My concern is to widen the view and not only think of UEFI needs
here, but to make such feature generally useful for others, too.
And at the same time minimize the amount oif new code we need and
keeping it agnostic of the storage media used.

Please feel free to contact me directly if the outline above is not
clear enough and you want more details how I think this could be
implemented.


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
Viele Probleme erscheinen uns nur deshalb so groß, weil wir sie mit zu
wenig Abstand betrachten.

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

[Ilias Apalodimas]

Dear Wolfgang,
> > 
> > thanks a lot for the good online meeting this morning together with your
> > colleague Suggan where we discussed the requirements for the
> > implementation of non-volatile variables in U-Boot.
> > 
> > Currently UEFI variables are stored in U-Boot variables. Saving the
> > U-Boot variables will persist all UEFI variables in the environment both
> > volatile and non-volatile. This does not conform the the UEFI standard.
> 
> Is this the only problem of using the environment as storage?
> 
No it's not. For UEFI secure variables you need authentication, signature checks
amongst other things. 
> > To provide a secure storage Linaro is considering to implement an OP-TEE
> > module for variable storage and as alternative to this OP-TEE module a
> > standalone MM service which will be a BL32 ATF module.
> > 
> > So in future we will have possibly three alternative drivers for UEFI
> > variables:
> > 
> > - U-Boot only implementation
> >   (what is now in lib/efi_loader/efi_variable.c)
> > - OP-TEE module
> > - standalone MM service
> > 
> > And maybe a fourth dummy one implementing no variable service at all.
> 
> Is this really a good idea?
> 
> 
> If the volatile / non-volatile behaviour is the onlyh problem you
> see with using environment variables, would it then not make much
> more sense to extend the environment flags concept by adding a
> "volatile" flag, with the meaing that such variables don;t get
> written by "env save"?
Yes but the volatile/non-volatile is the least of our problems

> 
> This would also make sense in some other places - for example, the
> "filesize" variable is a perfect candidate to be flagged as
> "volatile".  There is absolutely no use in saving it persistently.
> 
> 
> So such an extension would be useful for others, too, and might
> eventually avoid so many different implementations for the same
> task?  Also, the implementation should be straightforward...
You need Secure variables. That implies they should live in Non-Secure world nor
you should store them is some NOr//NAND flash that you can manipulate under
linux with fw_setenv.


I personally think storing UEFI and U-Boot env variables in the *same* storage
partition a bad idea. I also considering mixing apples and oranges. They are two
completely different things. The fact that it was easy to use existing U-Boot
primitives to make that happen does not make the approach correct. It just makes
it easier.

Rergards
/Ilias

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

[Wolfgang Denk]

Dear Ilias,

In message <20190625091140.GA19606@apalos> you wrote:
>
> > > Currently UEFI variables are stored in U-Boot variables. Saving the
> > > U-Boot variables will persist all UEFI variables in the environment both
> > > volatile and non-volatile. This does not conform the the UEFI standard.
> > 
> > Is this the only problem of using the environment as storage?
> > 
> No it's not. For UEFI secure variables you need authentication, signature checks
> amongst other things. 

There have been thoughts about using signed environment storage
before.  This is manageable as long as your environment is read-only.
But for writing ("env save") you need access to the private key to
sign the new data.  Do you have a good solution for this?

Except for this problem, I see no reason why my proposal of adding a
"UEFI" flag (or context, if you prefer this word) would not be a
solution for this requiremnt?

> > If the volatile / non-volatile behaviour is the onlyh problem you
> > see with using environment variables, would it then not make much
> > more sense to extend the environment flags concept by adding a
> > "volatile" flag, with the meaing that such variables don;t get
> > written by "env save"?
> Yes but the volatile/non-volatile is the least of our problems

Agreed - and it's trivial to solve.

> > So such an extension would be useful for others, too, and might
> > eventually avoid so many different implementations for the same
> > task?  Also, the implementation should be straightforward...
> You need Secure variables. That implies they should live in Non-Secure world nor
> you should store them is some NOr//NAND flash that you can manipulate under
> linux with fw_setenv.

Secure variables is a completely new topic actually.  It has not
been mentioned before.  It would have been nice if someone had
listed all teh requirements before...

And I think I should point out that the suggested patch does not
address this requirement either.

> I personally think storing UEFI and U-Boot env variables in the *same* storage
> partition a bad idea. I also considering mixing apples and oranges. They are two
> completely different things. The fact that it was easy to use existing U-Boot
> primitives to make that happen does not make the approach correct. It just makes
> it easier.

I can understand that you want to have separate storage, and indeed
it makes sense in other use cases as well.  And actually I see this
as one of the advantages of my proposal: you can have this as well
easily, less intrusive and with less effort than the submitted
patches.  And with the additional benefit that it is usefoul for
others as well.

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
To get something done, a committee should consist  of  no  more  than
three men, two of them absent.

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

[AKASHI Takahiro]

Wolfgang,

On Tue, Jun 25, 2019 at 11:11:18AM +0200, Wolfgang Denk wrote:
> Dear Takahiro,
> 
> In message <20190625075931.GP6610@linaro.org> you wrote:
> >
> > > 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?
> 
> You did not complete the scenario, but I guess you mean that the
> user _wants_ to save 'c' to persistent storage?

That is the behavior that UEFI specification requires.

> Yes, the current implementation does not allow this.
> 
> But if you extend the existing environment variable flags by some
> new properties you can have all this easily:
> 
> a) add a "volatile" flag.
> 
>    In the above setup you should then mark 'b' as volatile, so it
>    does not get saved to the environment.
> 
> b) add a "UEFI" flag (*) and extend "env save" and "env export"
>    to respect such flags (ideally both for a "include only these"
>    and an "exclude these" operation).
> 
>    Then you can do a _lot_ of new things.
> 
>    You can for example save _only_ the UEFI variable to persistent
>    storage - which would implement the feature you requested before
>    where you said you would like to set CONFIG_ENV_IS_NOWHERE.
> 
>    Alternatively, you could use "env save" to only save the "normal"
>    U-Boot environment, and "env export" to extract only the UEFI
>    variables and then use a file write to store these elsewhere.
> 
>    You can even go a step further and define separate storage areas
>    so "env save" will do this automatically for you (more cosing
>    effort, easier to use).
> 
>    (*) For a start and just to fulfil your requirement, such a
>    simple "UEFI" flag would be sufficient.  If you think further,
>    you could as well implement a flag that takes a string value,
>    so you can define any number of different variable contexts that
>    can all be managed using the existing code, but kept strictly
>    separate else.  There is a zillion of potential use cases.
> 
> > 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.
> 
> What you describe in 3) is indeed ugly, but it is also not
> necessary.  There are other, more intelligent ways to implement
> that.  See above for a proposal.
> 
> > > 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."
> 
> See above.  If you think small (and go for minimal coding efforts),
> just add a UEFI flag now to provide a UEFI context.  If tomorrow
> someone needs a FOO context, add a FOO flag.  We did not really need
> such context based variable handlint for the last 2 decades, so we
> can probably go on like this for a couple of years.

In the decades, there came out no new FOO context other than UEFI,
so it is unlikely that you will see yet another context for next
couple of years.

> If you think bit and are willing to pay a little more efforts for a
> more generic, runtime extensible solution, than add a context flag
> that takes a string as value, and you can define as many contexts as
> you have memory for.  And you can use them all through the same
> existing tools, and all this is independent of the storage of the
> persistent copies.  and yes, of course you can have separate storage
> for each of the "context" groups.
> 
> > 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.
> 
> I do not stick to a single storage, on contrary.

It is good news to me. I thought that a "single" storage
was the point.

Depending on the nature of "context," we may want to have
different storage devices for different contexts.

> And yes, 3) _is_
> ugly.  But there should be better ways to implement this.
> 
> My concern is to widen the view and not only think of UEFI needs
> here, but to make such feature generally useful for others, too.
> And at the same time minimize the amount oif new code we need and
> keeping it agnostic of the storage media used.

So overall, I basically agree with your proposal, but still need
some more thinkings.

* two new terms
  - interface (I prefer this over context now): uboot or uefi for now
  - variable attribute: volatile(/non-volatile) (and autosave if appropriate)
    some attributes may be interface-specific and expandable in future
* extend existing env-related interfaces
  - accept one or two additional parameters, interface and/or attribute
* (not sure yet) extend hash functions(hsearch_r ...)
  - to allow arbitrary type of key instead of a string of name
  - to allow arbitrary type of value instead of a printable string
  (assuming GUID for UEFI variable)
* add CONFIG_ENV_xxx's to specify dedicated storage device for each interface

Does this meet your requirements?

-Takahiro Akashi

> Please feel free to contact me directly if the outline above is not
> clear enough and you want more details how I think this could be
> implemented.
> 
> 
> 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
> Viele Probleme erscheinen uns nur deshalb so groß, weil wir sie mit zu
> wenig Abstand betrachten.

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

[Ilias Apalodimas]

Dear Wolfgang,

On Tue, 25 Jun 2019 at 12:27, Wolfgang Denk <wd@denx.de> wrote:
>
> Dear Ilias,
>
> In message <20190625091140.GA19606@apalos> you wrote:
> >
> > > > Currently UEFI variables are stored in U-Boot variables. Saving the
> > > > U-Boot variables will persist all UEFI variables in the environment both
> > > > volatile and non-volatile. This does not conform the the UEFI standard.
> > >
> > > Is this the only problem of using the environment as storage?
> > >
> > No it's not. For UEFI secure variables you need authentication, signature checks
> > amongst other things.
>
> There have been thoughts about using signed environment storage
> before.  This is manageable as long as your environment is read-only.
> But for writing ("env save") you need access to the private key to
> sign the new data.  Do you have a good solution for this?
Not really, not anything secure anyway
That's we we suggested letting the secure world deal with it.

>
> Except for this problem, I see no reason why my proposal of adding a
> "UEFI" flag (or context, if you prefer this word) would not be a
> solution for this requiremnt?

I think the argument is orthogonal to what we are discussing.
I understand the need for U-Boot to deal with volatile and
non-volatile variables. I also know use-cases that would benefit from
it (i.e the filesize you mentioned)

Adding that attribute on the the variables is fine. What is not fine
is trying to apply UEFI semantics on the projects environmental
variable subsystem, just because the volatile/non-volatile happens to
be 'ok' for both UEFI/ENV variables.
The attribute will only offer you some kind of UEFI variable "emulation".
So what i'd ideally love to see is something like:
1. UEFI variables get stored *somewhere* if a secure OS does not exist
and can't deal with Secure UEFI variables. We can then emulate the
UEFI behavior with 0 security. In this scenario i can justify your
proposal, which in fact makes sense.
2. If A Secure OS runs (whether it's Arm/Intel/Whatever), we can
replace the read/store callbacks and have proper UEFI variables with
full semantics for which, a secure world application will be
responsible (and will be responsible for the storage as well)

>
> > > If the volatile / non-volatile behaviour is the onlyh problem you
> > > see with using environment variables, would it then not make much
> > > more sense to extend the environment flags concept by adding a
> > > "volatile" flag, with the meaing that such variables don;t get
> > > written by "env save"?
> > Yes but the volatile/non-volatile is the least of our problems
>
> Agreed - and it's trivial to solve.
>
> > > So such an extension would be useful for others, too, and might
> > > eventually avoid so many different implementations for the same
> > > task?  Also, the implementation should be straightforward...
> > You need Secure variables. That implies they should live in Non-Secure world nor
> > you should store them is some NOr//NAND flash that you can manipulate under
> > linux with fw_setenv.
>
> Secure variables is a completely new topic actually.  It has not
> been mentioned before.  It would have been nice if someone had
> listed all teh requirements before...
>
> And I think I should point out that the suggested patch does not
> address this requirement either.
>
> > I personally think storing UEFI and U-Boot env variables in the *same* storage
> > partition a bad idea. I also considering mixing apples and oranges. They are two
> > completely different things. The fact that it was easy to use existing U-Boot
> > primitives to make that happen does not make the approach correct. It just makes
> > it easier.
>
> I can understand that you want to have separate storage, and indeed
> it makes sense in other use cases as well.  And actually I see this
> as one of the advantages of my proposal: you can have this as well
> easily, less intrusive and with less effort than the submitted
> patches.  And with the additional benefit that it is usefoul for
> others as well.
Does my answer above cover this as well?

Regards
/Ilias

>
> 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
> To get something done, a committee should consist  of  no  more  than
> three men, two of them absent.

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

[Wolfgang Denk]

Dear Takahiro,

In message <20190626052624.GR6610@linaro.org> you wrote:
>
> > See above.  If you think small (and go for minimal coding efforts),
> > just add a UEFI flag now to provide a UEFI context.  If tomorrow
> > someone needs a FOO context, add a FOO flag.  We did not really need
> > such context based variable handlint for the last 2 decades, so we
> > can probably go on like this for a couple of years.
>
> In the decades, there came out no new FOO context other than UEFI,
> so it is unlikely that you will see yet another context for next
> couple of years.

I fully agree.

> It is good news to me. I thought that a "single" storage
> was the point.

No.  The current implementation of persistent storage for the
environment has grown historically. In the beginnign we had just a
single copy; later a redundant copy got added. Storage was defined
by static (compile time) selection of the driver and offsets for the
two copies in that device.  And there was/is support only for oine
such copy.

In the mean time, we have DM, and we can / could attach multiple
copies of the environment. We could handle environment blobs from
several devices, for example in a hierarchical way - say, you start
with a standard version of the environment, and when device XXX is
present it is checked if it holds an environment blob (say, a distro
specific set of environment settings), which then gets loaded. Etc.

> Depending on the nature of "context," we may want to have
> different storage devices for different contexts.

This is certainly possible.  It may make sense to make this a
configurable option, whether you want a "classic" envionment share
for all types, or separate storage for each "context".

> So overall, I basically agree with your proposal, but still need
> some more thinkings.
>
> * two new terms
>   - interface (I prefer this over context now): uboot or uefi for now

I'd prefer context here.  Interface my make more sense fromt he UEFi
point of view, but I see other use cases that will use the normal,
U-Boot set of commands / functions to manipulate the variables in
their "context".  Interface suggests that there is another API,
which may be true for UEFI, but not generally.

>   - variable attribute: volatile(/non-volatile) (and autosave if appropriate)
>     some attributes may be interface-specific and expandable in future

Agreed.  For now we can just add to the existing flags (as this
helps to keep the code small).  If there are new ideas / use cases
that require more complex atributes, this can be extended.  Also, I
think we can store all "contexts" in a common hash table, and only
make the import / export / lookup routines respect the new flags /
contexts.

> * extend existing env-related interfaces
>   - accept one or two additional parameters, interface and/or attribute

Agreed.

We also should define a (documented) set of rules, for example for
mixed storage - i. e., if I run a plain "env save", this would only
save the U-Boot environment, but not the UEFI context? 

> * (not sure yet) extend hash functions(hsearch_r ...)
>   - to allow arbitrary type of key instead of a string of name
>   - to allow arbitrary type of value instead of a printable string
>   (assuming GUID for UEFI variable)

I'm not sure if this is worth the effort. Binary data can be
stringified easily.

> * add CONFIG_ENV_xxx's to specify dedicated storage device for each interface

We should be careful not to make this static again.  it makes sense
to be able to configure only the needed drivers / handlers that are
actually supported / used on a board - say, we support "contexts" in
SPI NOR, on SDCard, and on eMMC.  The rest should be flexible - even
if there are default locations it would be nice to be able to
read/write the U-Boot anvionment or the UEFI data to any of these
devices.

> Does this meet your requirements?

Sounds good :-)  Thanks!

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
God made the integers; all else is the work of Man.       - Kronecker

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

[Wolfgang Denk]

Dear Ilias,

In message <CAC_iWjLqouCEb=kcpomhHwDiM9Cdb_mOQDMTUAZ0G_dP9SuDxg@mail.gmail.com> you wrote:
>
> > There have been thoughts about using signed environment storage
> > before.  This is manageable as long as your environment is read-only.
> > But for writing ("env save") you need access to the private key to
> > sign the new data.  Do you have a good solution for this?
> Not really, not anything secure anyway
> That's we we suggested letting the secure world deal with it.

Do you think this will work?  When people who don;t know about
U-Boot features and limitations try to come up with a solution it
may simply not fit.

And the problem is a generic one: if you want to use secure boot
with signed images (including the environment), you must make sure
the device cannot be used to create new valid signed images, i. e.
you must keep your private key strictly protectd and definitely not
on the device.  But then - how do you sign a new environment copy?

> I understand the need for U-Boot to deal with volatile and
> non-volatile variables. I also know use-cases that would benefit from
> it (i.e the filesize you mentioned)
>
> Adding that attribute on the the variables is fine. What is not fine
> is trying to apply UEFI semantics on the projects environmental
> variable subsystem, just because the volatile/non-volatile happens to
> be 'ok' for both UEFI/ENV variables.
> The attribute will only offer you some kind of UEFI variable "emulation".

Ok - but the patches that have been submitted modify (heavily) the
U-Boot environment code.

So I see two options: a) we emulate UEFI variables using the
existing U-Boot environment code to the extend possible; in this
case I would prefer my proposal over the patches that have been
submitted. Or b) UEFI data is handled completely separately, within
it's own code base; in that case the patches should be ignored.

> So what i'd ideally love to see is something like:
> 1. UEFI variables get stored *somewhere* if a secure OS does not exist
> and can't deal with Secure UEFI variables. We can then emulate the
> UEFI behavior with 0 security. In this scenario i can justify your
> proposal, which in fact makes sense.

Thanks - that would be case a).

> 2. If A Secure OS runs (whether it's Arm/Intel/Whatever), we can
> replace the read/store callbacks and have proper UEFI variables with
> full semantics for which, a secure world application will be
> responsible (and will be responsible for the storage as well)

That would be b) - in this case UEFI has to provide it's own code
base, and no UEFI specific patches should impact the U-Boot
environment handling.

> > I can understand that you want to have separate storage, and indeed
> > it makes sense in other use cases as well.  And actually I see this
> > as one of the advantages of my proposal: you can have this as well
> > easily, less intrusive and with less effort than the submitted
> > patches.  And with the additional benefit that it is usefoul for
> > others as well.
> Does my answer above cover this as well?

I think so - if my a) / b) separation above matches your
understanding.

Thanks.

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
NOTE: The  Most  Fundamental  Particles  in  This  Product  Are  Held
Together  by  a  "Gluing" Force About Which Little is Currently Known
and Whose Adhesive Power Can Therefore Not Be Permanently Guaranteed.

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

[AKASHI Takahiro]

Wolfgang,

I think that we are getting much closer than a few days ago,
but first let me explain one point that I'm afraid that you
might misunderstand somehow:

On Wed, Jun 26, 2019 at 11:44:03AM +0200, Wolfgang Denk wrote:
> Dear Ilias,
> 
> In message <CAC_iWjLqouCEb=kcpomhHwDiM9Cdb_mOQDMTUAZ0G_dP9SuDxg@mail.gmail.com> you wrote:
> >
> > > There have been thoughts about using signed environment storage
> > > before.  This is manageable as long as your environment is read-only.
> > > But for writing ("env save") you need access to the private key to
> > > sign the new data.  Do you have a good solution for this?

In UEFI environment, *not* all the variables are to be authenticated,
but just a few. The signature verification for such "authenticated"
variables should be done *per* variable.

More specifically to say,
* each authenticated variable's data is composed of signature + value,
  where the data format is well defined in UEFI specification.
* signing algorithm is RSA2048, so there should be a pair of
  private key and public key.
  (I hope that you are familiar with those crypto concepts.)
* Certificates or keys (public keys) to be used for authentication
  are also stored as authenticated variables:
  PK: Platform Key to be used for verifying signature of KEK 
  KEK: Key Enabling Key to be used for verifying signature of db
  db: Signature/Certificate database to be used for verifying signature
      of PE image
  (there are couples more, but we can ignore them for now.)

On the other hand,
* authenticated variables can only be updated via UEFI API,
	SetVariable(u16 *variable_name,
		    const efi_guid_t *vendor, u32 attributes,
		    efi_uintn_t data_size, const void *data);
  where data must have been already formatted as I explained above.
* All UEFI does here are:
  - decode data and retrieve a signature
  - verify this signature with one of certificates in "db"

Do you follow me?

So preparing data for authenticated variables are totally up to
users, and U-Boot doesn't have to know about any private keys for signing.
Once the system is properly constructed and installed with right
cerficates, we will be able to store keys offline.

With the secure storage solution (for instance, dedicated OP-TEE app),
all those verification stuff would be done securely (in secure world)
and U-Boot doesn't have to know of public key, neither.

One big advantage of this solution is that, even if the system (in
non-secure world) would be compromised, malware could not modify or
destroy these signature database (PK, KEK or db).
In another word, we can provide tamper-resistant storage.

Therefore, you claim above doesn't apply to our case (UEFI).

> > Not really, not anything secure anyway
> > That's we we suggested letting the secure world deal with it.
> 
> Do you think this will work?  When people who don;t know about
> U-Boot features and limitations try to come up with a solution it
> may simply not fit.
> 
> And the problem is a generic one: if you want to use secure boot
> with signed images (including the environment), you must make sure
> the device cannot be used to create new valid signed images, i. e.
> you must keep your private key strictly protectd and definitely not
> on the device.  But then - how do you sign a new environment copy?
> 
> > I understand the need for U-Boot to deal with volatile and
> > non-volatile variables. I also know use-cases that would benefit from
> > it (i.e the filesize you mentioned)
> >
> > Adding that attribute on the the variables is fine. What is not fine
> > is trying to apply UEFI semantics on the projects environmental
> > variable subsystem, just because the volatile/non-volatile happens to
> > be 'ok' for both UEFI/ENV variables.
> > The attribute will only offer you some kind of UEFI variable "emulation".
> 
> Ok - but the patches that have been submitted modify (heavily) the
> U-Boot environment code.

I have a different opinion here, but don't care now.
I have not mentioned "secure" aspect of UEFI variables in my patch,
but I didn't do so intentionally because I believe that it is an orthogonal
issue to be discussed separately.

> So I see two options: a) we emulate UEFI variables using the
> existing U-Boot environment code to the extend possible; in this
> case I would prefer my proposal over the patches that have been
> submitted. Or b) UEFI data is handled completely separately, within
> it's own code base; in that case the patches should be ignored.
> 
> > So what i'd ideally love to see is something like:
> > 1. UEFI variables get stored *somewhere* if a secure OS does not exist
> > and can't deal with Secure UEFI variables. We can then emulate the
> > UEFI behavior with 0 security. In this scenario i can justify your
> > proposal, which in fact makes sense.
> 
> Thanks - that would be case a).
> 
> > 2. If A Secure OS runs (whether it's Arm/Intel/Whatever), we can
> > replace the read/store callbacks and have proper UEFI variables with
> > full semantics for which, a secure world application will be
> > responsible (and will be responsible for the storage as well)
> 
> That would be b) - in this case UEFI has to provide it's own code
> base, and no UEFI specific patches should impact the U-Boot
> environment handling.
> 
> > > I can understand that you want to have separate storage, and indeed
> > > it makes sense in other use cases as well.  And actually I see this
> > > as one of the advantages of my proposal: you can have this as well
> > > easily, less intrusive and with less effort than the submitted
> > > patches.  And with the additional benefit that it is usefoul for
> > > others as well.
> > Does my answer above cover this as well?
> 
> I think so - if my a) / b) separation above matches your
> understanding.

That is the reason why I think b) is an orthogonal issue.
Anyhow, there expected to be lots of platforms where users want to use
UEFI U-Boot without requiring secure boot. Even on such platforms,
my patch will provide more UEFI-compliant semantics of UEFI variables.

@Ilias, please don't call my patch *emulation*. I'm always doing best
to keep the implementation fully UEFI-compliant :)

Thanks,
-Takahiro Akashi


> Thanks.
> 
> 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
> NOTE: The  Most  Fundamental  Particles  in  This  Product  Are  Held
> Together  by  a  "Gluing" Force About Which Little is Currently Known
> and Whose Adhesive Power Can Therefore Not Be Permanently Guaranteed.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

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

[Ilias Apalodimas]

Dear Wolfgang, Akashi-san,
On Thu, Jun 27, 2019 at 02:15:19PM +0900, AKASHI Takahiro wrote:
> Wolfgang,
> 
> I think that we are getting much closer than a few days ago,
> but first let me explain one point that I'm afraid that you
> might misunderstand somehow:
> 
> On Wed, Jun 26, 2019 at 11:44:03AM +0200, Wolfgang Denk wrote:
> > Dear Ilias,
> > 
> > In message <CAC_iWjLqouCEb=kcpomhHwDiM9Cdb_mOQDMTUAZ0G_dP9SuDxg@mail.gmail.com> you wrote:
> > >
> > > > There have been thoughts about using signed environment storage
> > > > before.  This is manageable as long as your environment is read-only.
> > > > But for writing ("env save") you need access to the private key to
> > > > sign the new data.  Do you have a good solution for this?
> 
I think you are are trying to suggest a common way for U-Boot to
support that, we are not.
The plan for us was to split UEFI and U-Boot variables and let StMM
deal will *all* UEFI variables (since that's what the application
does). As Takahiro nicely explained the vast majority of UEFI variables are not
Authenticated variables.

> In UEFI environment, *not* all the variables are to be authenticated,
> but just a few. The signature verification for such "authenticated"
> variables should be done *per* variable.
> 
> More specifically to say,
> * each authenticated variable's data is composed of signature + value,
>   where the data format is well defined in UEFI specification.
> * signing algorithm is RSA2048, so there should be a pair of
>   private key and public key.
>   (I hope that you are familiar with those crypto concepts.)
> * Certificates or keys (public keys) to be used for authentication
>   are also stored as authenticated variables:
>   PK: Platform Key to be used for verifying signature of KEK 
>   KEK: Key Enabling Key to be used for verifying signature of db
>   db: Signature/Certificate database to be used for verifying signature
>       of PE image
>   (there are couples more, but we can ignore them for now.)
> 
> On the other hand,
> * authenticated variables can only be updated via UEFI API,
> 	SetVariable(u16 *variable_name,
> 		    const efi_guid_t *vendor, u32 attributes,
> 		    efi_uintn_t data_size, const void *data);
>   where data must have been already formatted as I explained above.
> * All UEFI does here are:
>   - decode data and retrieve a signature
>   - verify this signature with one of certificates in "db"
> 
> Do you follow me?
> 
> So preparing data for authenticated variables are totally up to
> users, and U-Boot doesn't have to know about any private keys for signing.
> Once the system is properly constructed and installed with right
> cerficates, we will be able to store keys offline.
> 
> With the secure storage solution (for instance, dedicated OP-TEE app),
> all those verification stuff would be done securely (in secure world)
> and U-Boot doesn't have to know of public key, neither.
> 
> One big advantage of this solution is that, even if the system (in
> non-secure world) would be compromised, malware could not modify or
> destroy these signature database (PK, KEK or db).
> In another word, we can provide tamper-resistant storage.
> 
> Therefore, you claim above doesn't apply to our case (UEFI).
> 
> > > Not really, not anything secure anyway
> > > That's we we suggested letting the secure world deal with it.
> > 
> > Do you think this will work?  When people who don;t know about
> > U-Boot features and limitations try to come up with a solution it
> > may simply not fit.
> > 
> > And the problem is a generic one: if you want to use secure boot
> > with signed images (including the environment), you must make sure
> > the device cannot be used to create new valid signed images, i. e.
> > you must keep your private key strictly protectd and definitely not
> > on the device.  But then - how do you sign a new environment copy?
> > 
> > > I understand the need for U-Boot to deal with volatile and
> > > non-volatile variables. I also know use-cases that would benefit from
> > > it (i.e the filesize you mentioned)
> > >
> > > Adding that attribute on the the variables is fine. What is not fine
> > > is trying to apply UEFI semantics on the projects environmental
> > > variable subsystem, just because the volatile/non-volatile happens to
> > > be 'ok' for both UEFI/ENV variables.
> > > The attribute will only offer you some kind of UEFI variable "emulation".
> > 
> > Ok - but the patches that have been submitted modify (heavily) the
> > U-Boot environment code.
> 
> I have a different opinion here, but don't care now.
> I have not mentioned "secure" aspect of UEFI variables in my patch,
> but I didn't do so intentionally because I believe that it is an orthogonal
> issue to be discussed separately.
> 
> > So I see two options: a) we emulate UEFI variables using the
> > existing U-Boot environment code to the extend possible; in this
> > case I would prefer my proposal over the patches that have been
> > submitted. Or b) UEFI data is handled completely separately, within
> > it's own code base; in that case the patches should be ignored.
> > 
> > > So what i'd ideally love to see is something like:
> > > 1. UEFI variables get stored *somewhere* if a secure OS does not exist
> > > and can't deal with Secure UEFI variables. We can then emulate the
> > > UEFI behavior with 0 security. In this scenario i can justify your
> > > proposal, which in fact makes sense.
> > 
> > Thanks - that would be case a).
> > 
> > > 2. If A Secure OS runs (whether it's Arm/Intel/Whatever), we can
> > > replace the read/store callbacks and have proper UEFI variables with
> > > full semantics for which, a secure world application will be
> > > responsible (and will be responsible for the storage as well)
> > 
> > That would be b) - in this case UEFI has to provide it's own code
> > base, and no UEFI specific patches should impact the U-Boot
> > environment handling.
> > 
> > > > I can understand that you want to have separate storage, and indeed
> > > > it makes sense in other use cases as well.  And actually I see this
> > > > as one of the advantages of my proposal: you can have this as well
> > > > easily, less intrusive and with less effort than the submitted
> > > > patches.  And with the additional benefit that it is usefoul for
> > > > others as well.
> > > Does my answer above cover this as well?
> > 
> > I think so - if my a) / b) separation above matches your
> > understanding.
> 
> That is the reason why I think b) is an orthogonal issue.
> Anyhow, there expected to be lots of platforms where users want to use
> UEFI U-Boot without requiring secure boot. Even on such platforms,
> my patch will provide more UEFI-compliant semantics of UEFI variables.
> 
Same here b) is orthogonal to the rest imho
> @Ilias, please don't call my patch *emulation*. I'm always doing best
> to keep the implementation fully UEFI-compliant :)
Fair enough. The 'emulation' was not referring to the quality of the patch, 
but explain it will offer offer limited security compared to what running all 
these in secure world.


Thanks
/Ilias

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

[Wolfgang Denk]

Dear Takahiro,

In message <20190627051518.GB19527@linaro.org> you wrote:
>
> In UEFI environment, *not* all the variables are to be authenticated,
> but just a few. The signature verification for such "authenticated"
> variables should be done *per* variable.

I see.

> * authenticated variables can only be updated via UEFI API,
> 	SetVariable(u16 *variable_name,
> 		    const efi_guid_t *vendor, u32 attributes,
> 		    efi_uintn_t data_size, const void *data);
>   where data must have been already formatted as I explained above.
> * All UEFI does here are:
>   - decode data and retrieve a signature
>   - verify this signature with one of certificates in "db"

Do I understand correctly that the user who wants to change the
value of such a authenticated variable, has to provide both the
value plus the signature through the user interface?

> So preparing data for authenticated variables are totally up to
> users, and U-Boot doesn't have to know about any private keys for signing.
> Once the system is properly constructed and installed with right
> cerficates, we will be able to store keys offline.

Understood.  Thanks for the explanation.

> That is the reason why I think b) is an orthogonal issue.
> Anyhow, there expected to be lots of platforms where users want to use
> UEFI U-Boot without requiring secure boot. Even on such platforms,
> my patch will provide more UEFI-compliant semantics of UEFI variables.

This is ok with me.  But if this is really orthogonal, then it
should be so also code-wise.  I don't like such intrusive
modifications of the U-Boot environment code for purposes that have
nothing to do with U-Boot environment variables.

Either we store in (in the simple, non-secure) case UEFI variables a
spart of the environment (see my proposal), then we don;t need your
patches.  Or you provide a separate UEFI data storage solution
(which may be configred with or without secure boot support), but
then this should be completely separate from the environment code.

Thanks!

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
God made machine language; all the rest is the work of man.

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

[Wolfgang Denk]

Dear Ilias,

In message <20190627070821.GA10271@apalos> you wrote:
>
> > > > > There have been thoughts about using signed environment storage
> > > > > before.  This is manageable as long as your environment is read-only.
> > > > > But for writing ("env save") you need access to the private key to
> > > > > sign the new data.  Do you have a good solution for this?
> > 
> I think you are are trying to suggest a common way for U-Boot to
> support that, we are not.

Well, if there is a chance to use a common code base, then such an
approach is always preferrable over using multiple separate
implementations for the same thing.

But it's not up to me to decide if you really can or want to
utilize the exiting environment code.  You decide.

But then please make up your mind:

_Either_ use the environment code - if so, then please in a way that
is ideally useful to others, too, or at least does not hurt others
(for example in terms of code size or complexity /maintainability).
_Or_ use your own, UEFI specific implementation - but then please
don;t meddle with the environment code - instead, leave this
unchanged. Feel free to use it as is where it fits your need, or
write new, UEFI specific code otherwise.

I don't want to see patches that are meddling with the environment
code for purposes that have nothing to do with the environment
handling in U-Boot.

> The plan for us was to split UEFI and U-Boot variables and let StMM
> deal will *all* UEFI variables (since that's what the application
> does). As Takahiro nicely explained the vast majority of UEFI variables are not
> Authenticated variables.

That's perfectly fine with me.  But please keep the code base clean.
Either use common tools for storage (existing environment code), or
use something else (completely new UEFI specific code).

Thanks.

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
You cannot propel yourself forward by patting yourself on the back.