Not crazy about the patch subject message, but
Reviewed-by: Jordan Justen <[email protected]>

Best I could manage was:
OvmfPkg/SerializeVariablesLib: ignore secure variable restore errors

On Sun, May 19, 2013 at 3:35 PM, Laszlo Ersek <[email protected]> wrote:
> OvmfPkg's file-based NvVar storage is read back as follows at boot (all
> paths under OvmfPkg/Library/):
>
> PlatformBdsPolicyBehavior() [PlatformBdsLib/BdsPlatform.c]
>   PlatformBdsRestoreNvVarsFromHardDisk()
>     VisitAllInstancesOfProtocol
>       for each simple file system:
>         VisitingFileSystemInstance()
>           ConnectNvVarsToFileSystem() [NvVarsFileLib/NvVarsFileLib.c]
>             LoadNvVarsFromFs() [NvVarsFileLib/FsAccess.c]
>               ReadNvVarsFile()
> +-------------> SerializeVariablesSetSerializedVariables() 
> [SerializeVariablesLib/SerializeVariablesLib.c]
> |                 SerializeVariablesIterateInstanceVariables()
> |   +-------------> IterateVariablesInBuffer()
> |   |                 for each loaded / deserialized variable:
> | +-|-----------------> IterateVariablesCallbackSetSystemVariable()
> | | |                     gRT->SetVariable()
> | | |
> | | IterateVariablesInBuffer() stops processing variables as soon as the
> | | first error is encountered from the callback function.
> | |
> | | In this case the callback function is
> | IterateVariablesCallbackSetSystemVariable(), selected by
> SerializeVariablesSetSerializedVariables().
>
> The result is that no NvVar is restored from the file after the first
> gRT->SetVariable() failure.
>
> On my system such a failure
> - never happens in an OVMF build with secure boot disabled,
> - happens *immediately* with SECURE_BOOT_ENABLE, because the first
>   variable to restore is "AuthVarKeyDatabase".
>
> "AuthVarKeyDatabase" has the EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS
> attribute set. Since the loop tries to restore it before any keys (PK, KEK
> etc) are enrolled, gRT->SetVariable() rejects it with
> EFI_SECURITY_VIOLATION. Consequently the NvVar restore loop terminates
> immediately, and we never reach non-authenticated variables such as
> Boot#### and BootOrder.
>
> Until work on KVM-compatible flash emulation converges between qemu and
> OvmfPkg, improve the SECURE_BOOT_ENABLE boot experience by masking
> EFI_SECURITY_VIOLATION in the callback:
> - authenticated variables continue to be rejected same as before, but
> - at least we allow the loop to progress and restore non-authenticated
>   variables, for example boot options.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
>
> Signed-off-by: Laszlo Ersek <[email protected]>
> ---
>  If someone would like to test this patch on top of current master, both
>  of following two dependencies are needed:
>
>  (1) either my patch *or* Jordan's recommendation from the thread
>      [edk2] Default PcdFlashNvStorageVariableSize crashed OVMF
>      <http://thread.gmane.org/gmane.comp.bios.tianocore.devel/2781>,
>
>  (2) as a temporary workaround for the problems introduced by SVN
>      r14370, reverting SVN r14370:
>      [edk2] SetupBrowserDxe: r14370 breaks PK enrolment / secure boot in 
> OvmfPkg
>      <http://thread.gmane.org/gmane.comp.bios.tianocore.devel/2855>.
>
>  .../SerializeVariablesLib/SerializeVariablesLib.c  |   27 ++++++++++++++-----
>  1 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c 
> b/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c
> index 112f20e..11bd3c8 100644
> --- a/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c
> +++ b/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c
> @@ -284,13 +284,26 @@ IterateVariablesCallbackSetSystemVariable (
>    IN  VOID                         *Data
>    )
>  {
> -  return gRT->SetVariable (
> -           VariableName,
> -           VendorGuid,
> -           Attributes,
> -           DataSize,
> -           Data
> -           );
> +  EFI_STATUS          Status;
> +  STATIC CONST UINT32 AuthMask =
> +                        EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS |
> +                        EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
> +
> +  Status = gRT->SetVariable (
> +             VariableName,
> +             VendorGuid,
> +             Attributes,
> +             DataSize,
> +             Data
> +             );
> +
> +  if (Status == EFI_SECURITY_VIOLATION && (Attributes & AuthMask) != 0) {
> +    DEBUG ((DEBUG_WARN, "%a: setting authenticated variable \"%s\" "
> +            "failed with EFI_SECURITY_VIOLATION, ignoring\n", __FUNCTION__,
> +            VariableName));
> +    Status = EFI_SUCCESS;
> +  }
> +  return Status;
>  }
>
>
> --
> 1.7.1
>
>
> ------------------------------------------------------------------------------
> AlienVault Unified Security Management (USM) platform delivers complete
> security visibility with the essential security capabilities. Easily and
> efficiently configure, manage, and operate all of your security controls
> from a single console and one unified framework. Download a free trial.
> http://p.sf.net/sfu/alienvault_d2d
> _______________________________________________
> edk2-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
AlienVault Unified Security Management (USM) platform delivers complete
security visibility with the essential security capabilities. Easily and
efficiently configure, manage, and operate all of your security controls
from a single console and one unified framework. Download a free trial.
http://p.sf.net/sfu/alienvault_d2d
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to