Re: [PATCH v2 3/6] efi_loader: don't load signature database from file
On 8/27/21 6:49 AM, AKASHI Takahiro wrote:
On Fri, Aug 27, 2021 at 06:42:39AM +0200, Heinrich Schuchardt wrote:
On 8/27/21 6:12 AM, AKASHI Takahiro wrote:
On Thu, Aug 26, 2021 at 03:48:02PM +0200, Heinrich Schuchardt wrote:
The UEFI specification requires that the signature database may only be
stored in tamper-resistant storage. So these variable may not be read
from an unsigned file.
I don't have a strong opinion here, but it seems to be too restrictive.
Nobody expects that file-based variable implementation is *safe*.
Leave it as it is so that people can easily experiment secure boot.
If the prior boot stage checks the integrity of the U-Boot binary, the
file based implementation becomes 'safe' with this patch.
How safe (or secure) is it? That is a question.
What is your thread model?
The preseed store is as safe as the capsule updates that Linaro is
working on where the certificate for verifying the capsule is baked into
U-Boot or the StMM based variables.
They all require that an attacker can neither load a manipulated U-Boot
nor that he can alter the memory containing U-Boot at runtime.
Best regards
Heinrich
-Takahiro Akashi
Users can still experiment with secure boot by setting the secure boot
variables via the efidebug command.
I cannot see a use case for having the secure boot data base on an
insecure medium.
Best regards
Heinrich
-Takahiro Akashi
Signed-off-by: Heinrich Schuchardt
---
v2:
no change
---
include/efi_variable.h | 5 +++-
lib/efi_loader/efi_var_common.c | 2 --
lib/efi_loader/efi_var_file.c | 41 -
lib/efi_loader/efi_variable.c | 2 +-
4 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h
index 4623a64142..2d97655e1f 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -161,10 +161,13 @@ efi_status_t __maybe_unused efi_var_collect(struct
efi_var_file **bufp, loff_t *
/**
* efi_var_restore() - restore EFI variables from buffer
*
+ * Only if @safe is set secure boot related variables will be restored.
+ *
* @buf: buffer
+ * @safe: restoring from tamper-resistant storage
* Return: status code
*/
-efi_status_t efi_var_restore(struct efi_var_file *buf);
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
/**
* efi_var_from_file() - read variables from file
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index cf7afecd60..b0c5b672c5 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type name_type[] = {
{u"KEK", &efi_global_variable_guid, EFI_AUTH_VAR_KEK},
{u"db", &efi_guid_image_security_database, EFI_AUTH_VAR_DB},
{u"dbx", &efi_guid_image_security_database, EFI_AUTH_VAR_DBX},
- /* not used yet
{u"dbt", &efi_guid_image_security_database, EFI_AUTH_VAR_DBT},
{u"dbr", &efi_guid_image_security_database, EFI_AUTH_VAR_DBR},
- */
};
static bool efi_secure_boot;
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
index de076b8cbc..c7c6805ed0 100644
--- a/lib/efi_loader/efi_var_file.c
+++ b/lib/efi_loader/efi_var_file.c
@@ -148,9 +148,10 @@ error:
#endif
}
-efi_status_t efi_var_restore(struct efi_var_file *buf)
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
{
struct efi_var_entry *var, *last_var;
+ u16 *data;
efi_status_t ret;
if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
@@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file *buf)
return EFI_INVALID_PARAMETER;
}
- var = buf->var;
last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
- while (var < last_var) {
- u16 *data = var->name + u16_strlen(var->name) + 1;
-
- if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) {
- ret = efi_var_mem_ins(var->name, &var->guid, var->attr,
- var->length, data, 0, NULL,
- var->time);
- if (ret != EFI_SUCCESS)
- log_err("Failed to set EFI variable %ls\n",
- var->name);
- }
- var = (struct efi_var_entry *)
- ALIGN((uintptr_t)data + var->length, 8);
+ for (var = buf->var; var < last_var;
+var = (struct efi_var_entry *)
+ ALIGN((uintptr_t)data + var->length, 8)) {
+
+ data = var->name + u16_strlen(var->name) + 1;
+
+ /*
+* Secure boot related and non-volatile variables shall only be
+* restored from U-Boot's preseed.
+*/
+ if (!safe &&
+
Re: [PATCH v2 3/6] efi_loader: don't load signature database from file
On Fri, Aug 27, 2021 at 01:49:41PM +0900, AKASHI Takahiro wrote:
> On Fri, Aug 27, 2021 at 06:42:39AM +0200, Heinrich Schuchardt wrote:
> > On 8/27/21 6:12 AM, AKASHI Takahiro wrote:
> > > On Thu, Aug 26, 2021 at 03:48:02PM +0200, Heinrich Schuchardt wrote:
> > > > The UEFI specification requires that the signature database may only be
> > > > stored in tamper-resistant storage. So these variable may not be read
> > > > from an unsigned file.
> > >
> > > I don't have a strong opinion here, but it seems to be too restrictive.
> > > Nobody expects that file-based variable implementation is *safe*.
> > > Leave it as it is so that people can easily experiment secure boot.
> >
> > If the prior boot stage checks the integrity of the U-Boot binary, the
> > file based implementation becomes 'safe' with this patch.
>
> How safe (or secure) is it? That is a question.
> What is your thread model?
Obviously, thread -> threat
>
> -Takahiro Akashi
>
>
> > Users can still experiment with secure boot by setting the secure boot
> > variables via the efidebug command.
> >
> > I cannot see a use case for having the secure boot data base on an
> > insecure medium.
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > > -Takahiro Akashi
> > >
> > >
> > > > Signed-off-by: Heinrich Schuchardt
> > > > ---
> > > > v2:
> > > > no change
> > > > ---
> > > > include/efi_variable.h | 5 +++-
> > > > lib/efi_loader/efi_var_common.c | 2 --
> > > > lib/efi_loader/efi_var_file.c | 41 -
> > > > lib/efi_loader/efi_variable.c | 2 +-
> > > > 4 files changed, 30 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/include/efi_variable.h b/include/efi_variable.h
> > > > index 4623a64142..2d97655e1f 100644
> > > > --- a/include/efi_variable.h
> > > > +++ b/include/efi_variable.h
> > > > @@ -161,10 +161,13 @@ efi_status_t __maybe_unused
> > > > efi_var_collect(struct efi_var_file **bufp, loff_t *
> > > > /**
> > > >* efi_var_restore() - restore EFI variables from buffer
> > > >*
> > > > + * Only if @safe is set secure boot related variables will be restored.
> > > > + *
> > > >* @buf: buffer
> > > > + * @safe: restoring from tamper-resistant storage
> > > >* Return:status code
> > > >*/
> > > > -efi_status_t efi_var_restore(struct efi_var_file *buf);
> > > > +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
> > > >
> > > > /**
> > > >* efi_var_from_file() - read variables from file
> > > > diff --git a/lib/efi_loader/efi_var_common.c
> > > > b/lib/efi_loader/efi_var_common.c
> > > > index cf7afecd60..b0c5b672c5 100644
> > > > --- a/lib/efi_loader/efi_var_common.c
> > > > +++ b/lib/efi_loader/efi_var_common.c
> > > > @@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type
> > > > name_type[] = {
> > > > {u"KEK", &efi_global_variable_guid, EFI_AUTH_VAR_KEK},
> > > > {u"db", &efi_guid_image_security_database, EFI_AUTH_VAR_DB},
> > > > {u"dbx", &efi_guid_image_security_database, EFI_AUTH_VAR_DBX},
> > > > - /* not used yet
> > > > {u"dbt", &efi_guid_image_security_database, EFI_AUTH_VAR_DBT},
> > > > {u"dbr", &efi_guid_image_security_database, EFI_AUTH_VAR_DBR},
> > > > - */
> > > > };
> > > >
> > > > static bool efi_secure_boot;
> > > > diff --git a/lib/efi_loader/efi_var_file.c
> > > > b/lib/efi_loader/efi_var_file.c
> > > > index de076b8cbc..c7c6805ed0 100644
> > > > --- a/lib/efi_loader/efi_var_file.c
> > > > +++ b/lib/efi_loader/efi_var_file.c
> > > > @@ -148,9 +148,10 @@ error:
> > > > #endif
> > > > }
> > > >
> > > > -efi_status_t efi_var_restore(struct efi_var_file *buf)
> > > > +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
> > > > {
> > > > struct efi_var_entry *var, *last_var;
> > > > + u16 *data;
> > > > efi_status_t ret;
> > > >
> > > > if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
> > > > @@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file
> > > > *buf)
> > > > return EFI_INVALID_PARAMETER;
> > > > }
> > > >
> > > > - var = buf->var;
> > > > last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
> > > > - while (var < last_var) {
> > > > - u16 *data = var->name + u16_strlen(var->name) + 1;
> > > > -
> > > > - if (var->attr & EFI_VARIABLE_NON_VOLATILE &&
> > > > var->length) {
> > > > - ret = efi_var_mem_ins(var->name, &var->guid,
> > > > var->attr,
> > > > - var->length, data, 0,
> > > > NULL,
> > > > - var->time);
> > > > - if (ret != EFI_SUCCESS)
> > > > - log_err("Failed to set EFI variable
> > > > %ls\n",
> > > > - var->name);
> > > > - }
Re: [PATCH v2 3/6] efi_loader: don't load signature database from file
On Fri, Aug 27, 2021 at 06:42:39AM +0200, Heinrich Schuchardt wrote:
> On 8/27/21 6:12 AM, AKASHI Takahiro wrote:
> > On Thu, Aug 26, 2021 at 03:48:02PM +0200, Heinrich Schuchardt wrote:
> > > The UEFI specification requires that the signature database may only be
> > > stored in tamper-resistant storage. So these variable may not be read
> > > from an unsigned file.
> >
> > I don't have a strong opinion here, but it seems to be too restrictive.
> > Nobody expects that file-based variable implementation is *safe*.
> > Leave it as it is so that people can easily experiment secure boot.
>
> If the prior boot stage checks the integrity of the U-Boot binary, the
> file based implementation becomes 'safe' with this patch.
How safe (or secure) is it? That is a question.
What is your thread model?
-Takahiro Akashi
> Users can still experiment with secure boot by setting the secure boot
> variables via the efidebug command.
>
> I cannot see a use case for having the secure boot data base on an
> insecure medium.
>
> Best regards
>
> Heinrich
>
> >
> > -Takahiro Akashi
> >
> >
> > > Signed-off-by: Heinrich Schuchardt
> > > ---
> > > v2:
> > > no change
> > > ---
> > > include/efi_variable.h | 5 +++-
> > > lib/efi_loader/efi_var_common.c | 2 --
> > > lib/efi_loader/efi_var_file.c | 41 -
> > > lib/efi_loader/efi_variable.c | 2 +-
> > > 4 files changed, 30 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/include/efi_variable.h b/include/efi_variable.h
> > > index 4623a64142..2d97655e1f 100644
> > > --- a/include/efi_variable.h
> > > +++ b/include/efi_variable.h
> > > @@ -161,10 +161,13 @@ efi_status_t __maybe_unused efi_var_collect(struct
> > > efi_var_file **bufp, loff_t *
> > > /**
> > >* efi_var_restore() - restore EFI variables from buffer
> > >*
> > > + * Only if @safe is set secure boot related variables will be restored.
> > > + *
> > >* @buf:buffer
> > > + * @safe:restoring from tamper-resistant storage
> > >* Return: status code
> > >*/
> > > -efi_status_t efi_var_restore(struct efi_var_file *buf);
> > > +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
> > >
> > > /**
> > >* efi_var_from_file() - read variables from file
> > > diff --git a/lib/efi_loader/efi_var_common.c
> > > b/lib/efi_loader/efi_var_common.c
> > > index cf7afecd60..b0c5b672c5 100644
> > > --- a/lib/efi_loader/efi_var_common.c
> > > +++ b/lib/efi_loader/efi_var_common.c
> > > @@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type name_type[]
> > > = {
> > > {u"KEK", &efi_global_variable_guid, EFI_AUTH_VAR_KEK},
> > > {u"db", &efi_guid_image_security_database, EFI_AUTH_VAR_DB},
> > > {u"dbx", &efi_guid_image_security_database, EFI_AUTH_VAR_DBX},
> > > - /* not used yet
> > > {u"dbt", &efi_guid_image_security_database, EFI_AUTH_VAR_DBT},
> > > {u"dbr", &efi_guid_image_security_database, EFI_AUTH_VAR_DBR},
> > > - */
> > > };
> > >
> > > static bool efi_secure_boot;
> > > diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> > > index de076b8cbc..c7c6805ed0 100644
> > > --- a/lib/efi_loader/efi_var_file.c
> > > +++ b/lib/efi_loader/efi_var_file.c
> > > @@ -148,9 +148,10 @@ error:
> > > #endif
> > > }
> > >
> > > -efi_status_t efi_var_restore(struct efi_var_file *buf)
> > > +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
> > > {
> > > struct efi_var_entry *var, *last_var;
> > > + u16 *data;
> > > efi_status_t ret;
> > >
> > > if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
> > > @@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file
> > > *buf)
> > > return EFI_INVALID_PARAMETER;
> > > }
> > >
> > > - var = buf->var;
> > > last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
> > > - while (var < last_var) {
> > > - u16 *data = var->name + u16_strlen(var->name) + 1;
> > > -
> > > - if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) {
> > > - ret = efi_var_mem_ins(var->name, &var->guid, var->attr,
> > > - var->length, data, 0, NULL,
> > > - var->time);
> > > - if (ret != EFI_SUCCESS)
> > > - log_err("Failed to set EFI variable %ls\n",
> > > - var->name);
> > > - }
> > > - var = (struct efi_var_entry *)
> > > - ALIGN((uintptr_t)data + var->length, 8);
> > > + for (var = buf->var; var < last_var;
> > > + var = (struct efi_var_entry *)
> > > +ALIGN((uintptr_t)data + var->length, 8)) {
> > > +
> > > + data = var->name + u16_strlen(var->name) + 1;
> > > +
> > > + /*
> > > + * Secure boot related and non-volatile variables shall onl
Re: [PATCH v2 3/6] efi_loader: don't load signature database from file
On 8/27/21 6:12 AM, AKASHI Takahiro wrote:
On Thu, Aug 26, 2021 at 03:48:02PM +0200, Heinrich Schuchardt wrote:
The UEFI specification requires that the signature database may only be
stored in tamper-resistant storage. So these variable may not be read
from an unsigned file.
I don't have a strong opinion here, but it seems to be too restrictive.
Nobody expects that file-based variable implementation is *safe*.
Leave it as it is so that people can easily experiment secure boot.
If the prior boot stage checks the integrity of the U-Boot binary, the
file based implementation becomes 'safe' with this patch.
Users can still experiment with secure boot by setting the secure boot
variables via the efidebug command.
I cannot see a use case for having the secure boot data base on an
insecure medium.
Best regards
Heinrich
-Takahiro Akashi
Signed-off-by: Heinrich Schuchardt
---
v2:
no change
---
include/efi_variable.h | 5 +++-
lib/efi_loader/efi_var_common.c | 2 --
lib/efi_loader/efi_var_file.c | 41 -
lib/efi_loader/efi_variable.c | 2 +-
4 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h
index 4623a64142..2d97655e1f 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -161,10 +161,13 @@ efi_status_t __maybe_unused efi_var_collect(struct
efi_var_file **bufp, loff_t *
/**
* efi_var_restore() - restore EFI variables from buffer
*
+ * Only if @safe is set secure boot related variables will be restored.
+ *
* @buf: buffer
+ * @safe: restoring from tamper-resistant storage
* Return:status code
*/
-efi_status_t efi_var_restore(struct efi_var_file *buf);
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
/**
* efi_var_from_file() - read variables from file
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index cf7afecd60..b0c5b672c5 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type name_type[] = {
{u"KEK", &efi_global_variable_guid, EFI_AUTH_VAR_KEK},
{u"db", &efi_guid_image_security_database, EFI_AUTH_VAR_DB},
{u"dbx", &efi_guid_image_security_database, EFI_AUTH_VAR_DBX},
- /* not used yet
{u"dbt", &efi_guid_image_security_database, EFI_AUTH_VAR_DBT},
{u"dbr", &efi_guid_image_security_database, EFI_AUTH_VAR_DBR},
- */
};
static bool efi_secure_boot;
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
index de076b8cbc..c7c6805ed0 100644
--- a/lib/efi_loader/efi_var_file.c
+++ b/lib/efi_loader/efi_var_file.c
@@ -148,9 +148,10 @@ error:
#endif
}
-efi_status_t efi_var_restore(struct efi_var_file *buf)
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
{
struct efi_var_entry *var, *last_var;
+ u16 *data;
efi_status_t ret;
if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
@@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file *buf)
return EFI_INVALID_PARAMETER;
}
- var = buf->var;
last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
- while (var < last_var) {
- u16 *data = var->name + u16_strlen(var->name) + 1;
-
- if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) {
- ret = efi_var_mem_ins(var->name, &var->guid, var->attr,
- var->length, data, 0, NULL,
- var->time);
- if (ret != EFI_SUCCESS)
- log_err("Failed to set EFI variable %ls\n",
- var->name);
- }
- var = (struct efi_var_entry *)
- ALIGN((uintptr_t)data + var->length, 8);
+ for (var = buf->var; var < last_var;
+var = (struct efi_var_entry *)
+ ALIGN((uintptr_t)data + var->length, 8)) {
+
+ data = var->name + u16_strlen(var->name) + 1;
+
+ /*
+* Secure boot related and non-volatile variables shall only be
+* restored from U-Boot's preseed.
+*/
+ if (!safe &&
+ (efi_auth_var_get_type(var->name, &var->guid) !=
+EFI_AUTH_VAR_NONE ||
+!(var->attr & EFI_VARIABLE_NON_VOLATILE)))
+ continue;
+ if (!var->length)
+ continue;
+ ret = efi_var_mem_ins(var->name, &var->guid, var->attr,
+ var->length, data, 0, NULL,
+ var->time);
+ if (ret != EFI_SUCCESS)
+ log_err("Failed to set EFI variable %ls\n", var->n
Re: [PATCH v2 3/6] efi_loader: don't load signature database from file
On Thu, Aug 26, 2021 at 03:48:02PM +0200, Heinrich Schuchardt wrote:
> The UEFI specification requires that the signature database may only be
> stored in tamper-resistant storage. So these variable may not be read
> from an unsigned file.
I don't have a strong opinion here, but it seems to be too restrictive.
Nobody expects that file-based variable implementation is *safe*.
Leave it as it is so that people can easily experiment secure boot.
-Takahiro Akashi
> Signed-off-by: Heinrich Schuchardt
> ---
> v2:
> no change
> ---
> include/efi_variable.h | 5 +++-
> lib/efi_loader/efi_var_common.c | 2 --
> lib/efi_loader/efi_var_file.c | 41 -
> lib/efi_loader/efi_variable.c | 2 +-
> 4 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/include/efi_variable.h b/include/efi_variable.h
> index 4623a64142..2d97655e1f 100644
> --- a/include/efi_variable.h
> +++ b/include/efi_variable.h
> @@ -161,10 +161,13 @@ efi_status_t __maybe_unused efi_var_collect(struct
> efi_var_file **bufp, loff_t *
> /**
> * efi_var_restore() - restore EFI variables from buffer
> *
> + * Only if @safe is set secure boot related variables will be restored.
> + *
> * @buf: buffer
> + * @safe:restoring from tamper-resistant storage
> * Return: status code
> */
> -efi_status_t efi_var_restore(struct efi_var_file *buf);
> +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
>
> /**
> * efi_var_from_file() - read variables from file
> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> index cf7afecd60..b0c5b672c5 100644
> --- a/lib/efi_loader/efi_var_common.c
> +++ b/lib/efi_loader/efi_var_common.c
> @@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type name_type[] = {
> {u"KEK", &efi_global_variable_guid, EFI_AUTH_VAR_KEK},
> {u"db", &efi_guid_image_security_database, EFI_AUTH_VAR_DB},
> {u"dbx", &efi_guid_image_security_database, EFI_AUTH_VAR_DBX},
> - /* not used yet
> {u"dbt", &efi_guid_image_security_database, EFI_AUTH_VAR_DBT},
> {u"dbr", &efi_guid_image_security_database, EFI_AUTH_VAR_DBR},
> - */
> };
>
> static bool efi_secure_boot;
> diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> index de076b8cbc..c7c6805ed0 100644
> --- a/lib/efi_loader/efi_var_file.c
> +++ b/lib/efi_loader/efi_var_file.c
> @@ -148,9 +148,10 @@ error:
> #endif
> }
>
> -efi_status_t efi_var_restore(struct efi_var_file *buf)
> +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
> {
> struct efi_var_entry *var, *last_var;
> + u16 *data;
> efi_status_t ret;
>
> if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
> @@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file *buf)
> return EFI_INVALID_PARAMETER;
> }
>
> - var = buf->var;
> last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
> - while (var < last_var) {
> - u16 *data = var->name + u16_strlen(var->name) + 1;
> -
> - if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) {
> - ret = efi_var_mem_ins(var->name, &var->guid, var->attr,
> - var->length, data, 0, NULL,
> - var->time);
> - if (ret != EFI_SUCCESS)
> - log_err("Failed to set EFI variable %ls\n",
> - var->name);
> - }
> - var = (struct efi_var_entry *)
> - ALIGN((uintptr_t)data + var->length, 8);
> + for (var = buf->var; var < last_var;
> + var = (struct efi_var_entry *)
> +ALIGN((uintptr_t)data + var->length, 8)) {
> +
> + data = var->name + u16_strlen(var->name) + 1;
> +
> + /*
> + * Secure boot related and non-volatile variables shall only be
> + * restored from U-Boot's preseed.
> + */
> + if (!safe &&
> + (efi_auth_var_get_type(var->name, &var->guid) !=
> + EFI_AUTH_VAR_NONE ||
> + !(var->attr & EFI_VARIABLE_NON_VOLATILE)))
> + continue;
> + if (!var->length)
> + continue;
> + ret = efi_var_mem_ins(var->name, &var->guid, var->attr,
> + var->length, data, 0, NULL,
> + var->time);
> + if (ret != EFI_SUCCESS)
> + log_err("Failed to set EFI variable %ls\n", var->name);
> }
> return EFI_SUCCESS;
> }
> @@ -213,7 +222,7 @@ efi_status_t efi_var_from_file(void)
> log_err("Failed to load EFI variables\n");
> goto error;
> }
> - if (buf->length != len || efi_var_restore(buf) != EFI_SUCCESS)
> + if (buf->length
[PATCH v2 3/6] efi_loader: don't load signature database from file
The UEFI specification requires that the signature database may only be
stored in tamper-resistant storage. So these variable may not be read
from an unsigned file.
Signed-off-by: Heinrich Schuchardt
---
v2:
no change
---
include/efi_variable.h | 5 +++-
lib/efi_loader/efi_var_common.c | 2 --
lib/efi_loader/efi_var_file.c | 41 -
lib/efi_loader/efi_variable.c | 2 +-
4 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h
index 4623a64142..2d97655e1f 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -161,10 +161,13 @@ efi_status_t __maybe_unused efi_var_collect(struct
efi_var_file **bufp, loff_t *
/**
* efi_var_restore() - restore EFI variables from buffer
*
+ * Only if @safe is set secure boot related variables will be restored.
+ *
* @buf: buffer
+ * @safe: restoring from tamper-resistant storage
* Return: status code
*/
-efi_status_t efi_var_restore(struct efi_var_file *buf);
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
/**
* efi_var_from_file() - read variables from file
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index cf7afecd60..b0c5b672c5 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type name_type[] = {
{u"KEK", &efi_global_variable_guid, EFI_AUTH_VAR_KEK},
{u"db", &efi_guid_image_security_database, EFI_AUTH_VAR_DB},
{u"dbx", &efi_guid_image_security_database, EFI_AUTH_VAR_DBX},
- /* not used yet
{u"dbt", &efi_guid_image_security_database, EFI_AUTH_VAR_DBT},
{u"dbr", &efi_guid_image_security_database, EFI_AUTH_VAR_DBR},
- */
};
static bool efi_secure_boot;
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
index de076b8cbc..c7c6805ed0 100644
--- a/lib/efi_loader/efi_var_file.c
+++ b/lib/efi_loader/efi_var_file.c
@@ -148,9 +148,10 @@ error:
#endif
}
-efi_status_t efi_var_restore(struct efi_var_file *buf)
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
{
struct efi_var_entry *var, *last_var;
+ u16 *data;
efi_status_t ret;
if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
@@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file *buf)
return EFI_INVALID_PARAMETER;
}
- var = buf->var;
last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
- while (var < last_var) {
- u16 *data = var->name + u16_strlen(var->name) + 1;
-
- if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) {
- ret = efi_var_mem_ins(var->name, &var->guid, var->attr,
- var->length, data, 0, NULL,
- var->time);
- if (ret != EFI_SUCCESS)
- log_err("Failed to set EFI variable %ls\n",
- var->name);
- }
- var = (struct efi_var_entry *)
- ALIGN((uintptr_t)data + var->length, 8);
+ for (var = buf->var; var < last_var;
+var = (struct efi_var_entry *)
+ ALIGN((uintptr_t)data + var->length, 8)) {
+
+ data = var->name + u16_strlen(var->name) + 1;
+
+ /*
+* Secure boot related and non-volatile variables shall only be
+* restored from U-Boot's preseed.
+*/
+ if (!safe &&
+ (efi_auth_var_get_type(var->name, &var->guid) !=
+EFI_AUTH_VAR_NONE ||
+!(var->attr & EFI_VARIABLE_NON_VOLATILE)))
+ continue;
+ if (!var->length)
+ continue;
+ ret = efi_var_mem_ins(var->name, &var->guid, var->attr,
+ var->length, data, 0, NULL,
+ var->time);
+ if (ret != EFI_SUCCESS)
+ log_err("Failed to set EFI variable %ls\n", var->name);
}
return EFI_SUCCESS;
}
@@ -213,7 +222,7 @@ efi_status_t efi_var_from_file(void)
log_err("Failed to load EFI variables\n");
goto error;
}
- if (buf->length != len || efi_var_restore(buf) != EFI_SUCCESS)
+ if (buf->length != len || efi_var_restore(buf, false) != EFI_SUCCESS)
log_err("Invalid EFI variables file\n");
error:
free(buf);
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index ba0874e9e7..a7d305ffbc 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -426,7 +426,7 @@ efi_status_t efi_init_variables(void)
if (IS_ENABLED(CONF
