On Fri, Aug 27, 2021 at 06:34:30AM +0200, Heinrich Schuchardt wrote:
On 8/27/21 5:53 AM, AKASHI Takahiro wrote:
On Thu, Aug 26, 2021 at 03:48:05PM +0200, Heinrich Schuchardt wrote:
Even if we cannot read the variable store from disk we still need to initialize the secure boot state.
Don't continue to boot if the variable preseed is invalid as this indicates that the variable store has been tampered.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
v2: no change
lib/efi_loader/efi_variable.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 80996d0f47..6d92229e2a 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -427,13 +427,17 @@ efi_status_t efi_init_variables(void) if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) { ret = efi_var_restore((struct efi_var_file *) __efi_var_file_begin, true);
if (ret != EFI_SUCCESS)
if (ret != EFI_SUCCESS) { log_err("Invalid EFI variable seed\n");return ret;}
- ret = efi_var_from_file();
- ret = efi_init_secure_state(); if (ret != EFI_SUCCESS) return ret;
- return efi_init_secure_state();
- /* Don't stop booting if variable store is not available */
- efi_var_from_file();
I think we have to think about two different cases:
- there is no "variable store" file available.
- it does exists, but reading from it (efi_var_restore()) failed
For (2), we should return with an error as in the case of CONFIG_EFI_VARIABLES_PRESEED. Otherwise, the behavior is inconsistent.
The preseed store is used for secure boot related variables.
Where does this restriction come from? Kconfig says: Include a file with the initial values for non-volatile UEFI variables into the U-Boot binary. If this configuration option is set, changes to authentication related variables (PK, KEK, db, dbx) are not allowed.
# oops: I didn't notice the last sentence. # but anyhow, it seems too restrictive.
If this store is inconsistent, failing is required to ensure secure boot.
As I said, a file-based variable configuration cannot work as a secure platform any way.
Why do we need this kind of restriction?
-Takahiro Akashi
With my patches applied the file store can not contain any secure boot related variables but it may contain boot options.
Your suggestion could mean that if the file store is corrupted the board is bricked.
If the boot options cannot be read either because the file does not exist or because the file is corrupt, I still want the user to have a chance to load shim, GRUB, or the kernel and boot via the bootefi command.
I cannot see any inconsistency here.
Best regards
Heinrich