I'll attempt not to add noise into an otherwise detailed and technical
discussion, but I like to go back to first principles:

[1] Does Peter's suggested mechanism extend the use model that exists
between a kernel and UEFI?
If so, I would suggest that that is dangerous ground.  As a side note, is
there a how to covering UEFI from a functional use point of view?

[2] Is it reasonable to think about who 'owns' these variables?
Usual database rules apply, one owner, no duplicates

[3] I think that I understand the need to work around a lack of file system
storage in firmware by using the OS (however indirectly), but see question
[1]

[4] Bending over backwards to help deal with legacy implementations is a
good goal, but how far is too far?

Apologies if this adds nothing to the discussion, if so, please ignore...

David

On Thu, 1 Nov 2018 at 14:35 Grant Likely <[email protected]> wrote:

>
>
> On 12/10/2018 19:15, Peter Jones wrote:
> > Alright, so I've taken a shot at this - here's my first attempt, let me
> > know what's unclear/wrong/insane/etc :)
> >
> > From: Peter Jones <[email protected]>
> > Date: Thu, 11 Oct 2018 14:44:30 -0400
> > Subject: [PATCH] Update variable storage language
> >
> > This patch updates the UEFI chapter of the spec, specifically the parts
> > about UEFI variable storage.  This language utilizes UEFI's existing
> > Capsule Update on Disk and EFI Configuration table mechanisms, providing
> > independent mechanisms for exporting variables to the OS and importing
> > updates from the OS.
> >
> > The mechanism for exporting variables from the firmware uses a UEFI
> > Configuration Table rather than requiring the firmware to maintain the
> > Capsule on Disk, so that a platform with no permanent storage can
> > implement exporting variables to the OS without having to write to
> > storage, and a system without permanent storage can implement exporting
> > to the OS without having to implement importing from Capsule on Disk.
> > To keep things simple, we do not specify Capsule on Disk as an export
> > mechanism to the OS >
> > This text also specifies a de facto coherency protocol.  As a result, in
> > most cases requirements on the firmware producing the configuration
> > table are "must" requirements, but they only apply if the export
> > procedure is implemented.  Likewise, the requirements on the OS to
> > create a Variable Updates Capsule are mostly "should" requirements, but
> > the requirements on the contents are "must" requirements.
> > ---
> >   source/chapter2-uefi.rst | 237 ++++++++++++++++++++++++++++-----------
> >   1 file changed, 174 insertions(+), 63 deletions(-)
> >
> > diff --git a/source/chapter2-uefi.rst b/source/chapter2-uefi.rst
> > index 0cbddff6b46..320d71d7456 100644
> > --- a/source/chapter2-uefi.rst
> > +++ b/source/chapter2-uefi.rst
> > @@ -196,66 +196,177 @@ command:
> >   Runtime Variable Access
> >   -----------------------
> >
> > -.. todo::
> > -
> > -   There are many platforms where it is difficult to support
> SetVariable() for
> > -   non-volatile variables because the firmware cannot access storage
> after
> > -   ExitBootServices() is called.
> > -   e.g., If firmware accesses an eMMC device directly at runtime, it
> will
> > -   collide with transactions initiated by the OS.
> > -   Neither U-Boot nor Tianocore have a solution for accessing shared
> media for
> > -   variable updates. [#OPTEESupplicant]_
> > -
> > -   In these platforms SetVariable() calls with the
> EFI_VARIABLE_NON_VOLATILE
> > -   attribute set will work in boot services, but will fail in runtime
> services.
> > -   The [UEFI]_ specification doesn't address what to do in this
> situation.
> > -   We need feedback on options before writing this section of EBBR, or
> making a
> > -   proposal to modify UEFI.
> > -
> > -   We need a solution that communicates to the OS that non-volatile
> variable
> > -   updates are not supported at runtime, and that defines the behaviour
> when
> > -   SetVariable() is called with the EFI_VARIABLE_NON_VOLATILE attribute.
> > -
> > -   Presumably, the solution will require SetVariable() to return
> > -   EFI_INVALID_PARAMETER if called with the EFI_VARIABLE_NON_VOLATILE
> > -   attribute, but beyond that there are a number of options:
> > -
> > -   #. Clear EFI_VARIABLE_NON_VOLATILE from all variables at
> ExitBootServices()
> > -
> > -      If the platform is incapable of updating non-volatile variables
> from Runtime
> > -      Services then it must clear the EFI_VARIABLE_NON_VOLATILE
> attribute from all
> > -      non-volatile variables when ExitBootServices() is called.
> > -
> > -      An OS can discover that non-volatile variables cannot be updated
> at
> > -      runtime by noticing that the NON_VOLATILE attribute is not set.
> > -
> > -   #. Clear all variables at ExitBootServices()
> > -
> > -      If the platform is incapable of updating non-volatile variables
> from Runtime
> > -      Services then it will clear all variables and return
> EFI_INVALID_PARAMETER
> > -      on all calls to SetVariable().
> > -
> > -      SUSE in particular currently uses this behaviour to decide
> whether or not
> > -      to treat the ESP as removable media.
> > -
> > -   #. Advertise that SetVariable() doesn't work at runtime with another
> variable
> > -
> > -      Platforms can check another variable to determine if they have
> this quirk,
> > -      perhaps by adding a new BootOptionSupport flag.
> > -
> > -   This is not a complete list, and other options can still be
> proposed. We're
> > -   looking for feedback on what would be most faithful to the UEFI
> spec, and
> > -   would work for the OS distributions before filling out this section
> of the
> > -   specification.
> > -
> > -   Comments can be sent to the [email protected]
> mailing list.
> > -
> > -.. [#OPTEESupplicant] It is worth noting that OP-TEE has a similar
> problem
> > -   regarding secure storage.
> > -   OP-TEE's chosen solution is to rely on an OS supplicant agent to
> perform
> > -   storage operations on behalf of OP-TEE.
> > -   The same solution may be applicable to solving the UEFI non-volatile
> > -   variable problem, but that approach is also not entirely UEFI
> compliant
> > -   because it requires additional OS support to work.
> > -
> > -
> https://github.com/OP-TEE/optee_os/blob/master/documentation/secure_storage.md
> > +There are many platforms where it is difficult to implement
> SetVariable() for
> > +non-volatile variables during runtime services because the firmware
> cannot
> > +access storage after ExitBootServices() is called.
> > +
> > +e.g., If firmware accesses an eMMC device directly at runtime, it will
> > +collide with transactions initiated by the OS.
> > +Neither U-Boot nor Tianocore have a generic solution for accessing or
> updating
> > +variables stored on shared media. [#OPTEESupplicant]_
> > +
> > +If a platform does not implement modifying non-volatile variables with
> > +SetVariable() after ExitBootServices(), then it must implement support
> for
> > +discovering this during Boot Services via the "RuntimeServicesSupported"
> > +variable (see UEFI Mantis 1961).
> > +
> > +Such a system may also support exporting the variable storage to the
> > +kernel via a UEFI configuration table and re-loading it from a Capsule
> on
> > +Disk as described in [UEFI]_ 8.5.5.  If this is supported, the platform
> > +must implement the following: > +
> > +- The firmware must provide BS variable named "CapsuleVariableSupport"
> > +  under the VARIABLE_STORAGE_GUID, with the following bits defined:
> > +
> > +    #define EBBR_CAPSULE_VARIABLE_EXPORT        0x01
> > +    #define EBBR_CAPSULE_VARIABLE_IMPORT        0x02
> > +    #define EBBR_CAPSULE_VARIABLE_IMPORT_AUTH           0x04
> > +    #define EBBR_CAPSULE_VARIABLE_IMPORT_AUTH_2    0x08
> > +    #define EBBR_CAPSULE_VARIABLE_IMPORT_AUTH_3    0x10 > +
> > +- If the firmware supports exporting variables via a configuration
> table,
> > +  the EBBR_CAPSULE_VARIABLE_EXPORT bit must be set, and the firmware
> must
> > +  create a configuration table identified by the VARIABLE_STORAGE_GUID
> > +  (defined below) before any EFI applications are started, and must
> update it
> > +  any time a variable is altered via SetVariable(), until
> ExitBootServices()
> > +  has successfully returned.
> Hmmm. What I thought you were describing when we talked doesn't match up
> with what you're recommending here. I guess I misunderstood at first.
> Originally I though you were suggesting for the OS loader to extract the
> full set of UEFI variables, cache them in a configuration table, and
> then pick them up again after the kernel has booted. This is a solution
> that would work now without any changes to firmware (though it could
> still be argued unnecessary because we don't have any technical barriers
> to implementing GetVariable() during Runtime Services. Only
> GetVariable() is the problem).
>
> However, it looks like you're suggesting the firmware be responsible for
>   exporting variables into a configuration table. I'm not fond of this
> approach because it create a new firmware interface that does exactly
> the same thing as GetVariable() does right now.
>
> > +- If the firmware supports importing non-volatile variables via a
> Capsule
> > +  on Disk, the EBBR_CAPSULE_VARIABLE_IMPORT bit must be set, and the
> > +  firmware must load its initial variable storage during boot services
> > +  from a capsule with the EFI_CAPSULE_HEADER.CapsuleGuid set to
> > +  VARIABLE_STORAGE_GUID.
>
> It sounds like the model here is the firmware stores the entire set of
> variables in a capsule, which is rewritten by the kernel if the variable
> change. Do I have this correct? If so, I think this is a fragile model.
> There are all kinds of things that can go wrong if the new capsule gets
> corrupted, or potentially security attack vectors by editing/deleting
> the entire variable storage.
>
> Could we instead make the import capsule contain a log of variable
> changes that modify the 'committed' variable set. Firmware can perform
> appropriate checks on each variable change before committing the change.
> This too doesn't require firmware behaviour changes other than parsing
> the variable storage capsule presuming the boot services SetVariable()
> call can be used to perform the updates.
>
> > +- If the firmware supports authenticated variables, the bits
> > +  EBBR_CAPSULE_VARIABLE_IMPORT_AUTH,
> EBBR_CAPSULE_VARIABLE_IMPORT_AUTH_2,
> > +  and EBBR_CAPSULE_VARIABLE_IMPORT_AUTH_3 must be set to indicate
> support for
> > +  EFI_VARIABLE_AUTHENTICATION, EFI_VARIABLE_AUTHENTICATION_2, and
> > +  EFI_VARIABLE_AUTHENTICATION_3 descriptors defined in [UEFI]_ 8.2.
> > +- If the CapsuleVariableSupport variable is not set, the OS must behave
> as if
> > +  all bits are 0.
> > +- Any bits which are not present in the CapsuleVariableSupport variable
> must
> > +  be treated as 0.
> > +
> > +Variable storage data format
> > +----------------------------
> > +
> > +The Variable Configuration Table and the Variable Update Capsule
> structure
> > +share the same data format, and are structured as a capsule update
> containing
> > +a packed array of update records:
> > +
> > +#define VARIABLE_STORAGE_GUID \
> > +        {0x1a3fb419, 0x2171, 0x458d,\
> > +         {0xb8, 0xb4, 0xbe, 0xa3, 0x0c, 0x9f, 0x6b, 0xab }}
> > +
> > +typedef struct {
> > +  CHAR16[64]   VariableName;
> > +  EFI_GUID     VendorGuid;
> > +  UINT32       Attributes;
> > +  UINT32       DataSize;
> > +  UINT8[]      Data;
> > +} EBBR_VARIABLE;
> > +
> > +typedef struct {
> > +  EFI_CAPSULE_HEADER                                     Header;
> > +  UINT8[Header.HeaderSize - sizeof(EFI_CAPSULE_HEADER)]  Reserved;
> > +  EBBR_VARIABLE[]                                        Variables;
> > +} EBBR_VARIABLE_BUNDLE __attribute__((__packed__));
> > +
> > +- EBBR_VARIABLE_BUNDLE.Header.CapsuleGuid must be VARIABLE_STORAGE_GUID
> > +- EBBR_VARIABLE_BUNDLE.Reserved may be 0 or more bytes.
> > +- EBBR_VARIABLE_BUNDLE.Header.HeaderSize is equal to the starting
> offset of
> > +  the EBBR_VARIABLE_BUNDLE.Variables array.
> > +- EBBR_VARIABLE_BUNDLE.Variables may be 0 or more bytes.
> > +- EBBR_VARIABLE_BUNDLE.Header.CapsuleImageSize is the full size of the
> capsule
> > +  including the Header, Reserved, and all Variable array members.
> > +- EBBR_VARIABLE_BUNDLE.Header.Flags must not have any of the following
> set:
> > +  CAPSULE_FLAGS_INITIATE_RESET
> > +- EBBR_VARIABLE_BUNDLE.Header.Flags should have all of the following
> set:
> > +  CAPSULE_FLAGS_PERSIST_ACROSS_RESET
> > +  CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE
> > +
> > +Variable Configuration Table creation
> > +-------------------------------------
> > +
> > +Any platform firmware which supports EBBR_CAPSULE_VARIABLE_EXPORT must
> > +install a UEFI Configuration Table with all appropriate variables
> specified
> > +in [UEFI]_ 3.3, as well as any variables which have been imported from a
> > +Variable Update Capsule or set through SetVariable().
> > +
> > +- The platform firmware must not include multiple entries for the same
> > +  variable.
> > +- The platform should avoid storing any secrets in variables, including
> > +  variables without EFI_VARIABLE_RUNTIME_SERVICES set.
> > +
> > +Variable Configuration Table processing
> > +---------------------------------------
> > +
> > +When processing the Variable Configuration Table, the OS must treat each
> > +EBBR_VARIABLE_BUNDLE.Variable entry as if it were a call to
> SetVariable()
> > +before ExitBootServices() has been called: > +
> > +- The variables are processed according to the requirements in [UEFI]_
> 8.2
> > +- Any update which would result in SetVariable() returning an error must
> > +  be ignored.
> > +- The OS should preserve entries with EFI_VARIABLE_NON_VOLATILE set but
> > +  EFI_VARIABLE_RUNTIME_SERVICES unset, and save them to a Variable
> Updates
> > +  Capsule before system reset.
> > +- Any entry without EFI_VARIABLE_RUNTIME_SERVICES set must not be
> exposed to
> > +  consumers of GetVariable().
> > +- Any entry authenticated with an Authentication Descriptor the OS does
> not
> > +  support should be preserved, but must not be exposed to consumers of
> > +  GetVariable().  Any following entry for any such variable must be
> treated
> > +  the same.
> > +- All authenticated variables should have their Authentication
> Descriptors
> > +  preserved, but only the encapsulated data should be presented through
> > +  GetVariable()-like interfaces.
> > +- The OS must take measures to prevent data in variables without
> > +  EFI_VARIABLE_RUNTIME_SERVICES set from being exposed to unprivileged
> tasks.
> > +
> > +Runtime Processing
> > +------------------
> > +
> > +The OS must take certain measures during runtime operation to insure
> > +consistency:
> > +
> > +- The OS must keep a log of any updates to variables, including delete,
> > +  append, and create operations.
> > +- The OS should attempt to simulate each operation as it would be
> applied
> > +  during the Variable Updates Capsule processing, in order to maintain a
> > +  view which is coherent across a reset.
> > +
> > +Variable Updates Capsule creation
> > +---------------------------------
> > +
> > +Before system reset, the OS should create a Variable Updates Capsule as
> a
> > +Capsule on Disk defined in [UEFI]_ 8.5.5 .  If a platform supports both
> > +EBBR_CAPSULE_VARIABLE_EXPORT and EBBR_CAPSULE_VARIABLE_IMPORT, the
> capsule
> > +should preserve the following from the Variable Configuration Table, if
> it
> > +was present:
> > +
> > +- EBBR_VARIABLE_BUNDLE.Header.HeaderSize,
> EBBR_VARIABLE_BUNDLE.Header.Flags,
> > +  and EBBR_VARIABLE_BUNDLE.Reserved fields
> > +- Any variables with EFI_VARIABLE_NON_VOLATILE set, including those
> without
> > +  EFI_VARIABLE_RUNTIME_SERVICES set
> > +- Any variables authenticated with Authentication Descriptors not
> supported
> > +  by the OS.
> > +- Runtime updates to authenticated variables must be included
> individually,
> > +  including any authenticated deletion.
> > +- Runtime operations on newly created variables which are not
> authenticated
> > +  may be coalesced to a single entry.
> > +
> > +Variable Updates Capsule processing
> > +-----------------------------------
> > +
> > +During boot, the system firmware must create the variables specified in
> [UEFI]_
> > +3.3 before processing the capsule update, and it must ensure that the
> variables
> > +implemented in the UEFI spec are treated as specified at all times.
> When
> > +processing a Variable Updates Capsule, the firmware must process each
> record in
> > +the order they appear in the Variables array as if each were a call to
> > +SetVariable() after ExitBootServices():
> > +
> > +- The variables are processed according to the requirements in [UEFI]_
> 8.2
> > +- Any update without the EFI_VARIABLE_NON_VOLATILE attribute set must be
> > +  ignored.
> > +- Any update which would result in SetVariable() returning an error must
> > +  be ignored.
> > +- Any variable authenticated with an unsupported Authentication
> Descriptor
> > +  not supported by the platform must be ignored.
>
> Okay, now I'm confused because this matches what I thought you were
> talking about with the OS providing an update capsule, not a capsule
> containing the entire storage. This I agree with. What is the point of
> providing variable export in a configuration table then? And the
> language above seems to suggest firmware must store its entire variable
> pool in a capsule, which doesn't make sense to me.
>
> Let's chat at the meeting today.
> g.
>
> _______________________________________________
> Arm.ebbr-discuss mailing list
> [email protected]
>
-- 
David A Rusling
CTO, Linaro
https://linaro.org
_______________________________________________
boot-architecture mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/boot-architecture

Reply via email to