diff mbox series

[v2,3/3] efi_loader: add DeployedMode and AuditMode variable measurement

Message ID 20210921071931.3755-4-masahisa.kojima@linaro.org
State Superseded
Delegated to: Heinrich Schuchardt
Headers show
Series Enhance Measured Boot | expand

Commit Message

Masahisa Kojima Sept. 21, 2021, 7:19 a.m. UTC
This commit adds the DeployedMode and AuditMode variable
measurement required in TCG PC Client PFP Spec.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---

(no changes since v1)

 lib/efi_loader/efi_tcg2.c | 47 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

Comments

Ilias Apalodimas Sept. 27, 2021, 1:53 p.m. UTC | #1
On Tue, 21 Sept 2021 at 10:17, Masahisa Kojima
<masahisa.kojima@linaro.org> wrote:
>
> This commit adds the DeployedMode and AuditMode variable
> measurement required in TCG PC Client PFP Spec.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
>
> (no changes since v1)
>
>  lib/efi_loader/efi_tcg2.c | 47 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index ea2c1ead03..68542c7cd3 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -12,6 +12,7 @@
>  #include <dm.h>
>  #include <efi_loader.h>
>  #include <efi_tcg2.h>
> +#include <efi_variable.h>
>  #include <log.h>
>  #include <malloc.h>
>  #include <smbios.h>
> @@ -1828,6 +1829,50 @@ out:
>         return ret;
>  }
>
> +/**
> + * tcg2_measure_deployed_audit_mode() - measure deployedmode and auditmode
> + *
> + * @dev:       TPM device
> + *
> + * Return:     status code
> + */
> +static efi_status_t tcg2_measure_deployed_audit_mode(struct udevice *dev)
> +{
> +       u8 deployed_mode;
> +       u8 audit_mode;
> +       efi_uintn_t size;
> +       efi_status_t ret;
> +       u32 pcr_index;
> +
> +       size = sizeof(deployed_mode);
> +       ret = efi_get_variable_int(L"DeployedMode", &efi_global_variable_guid,
> +                                  NULL, &size, &deployed_mode, NULL);
> +       if (ret != EFI_SUCCESS)
> +               return ret;
> +
> +       pcr_index = (deployed_mode ? 1 : 7);
> +
> +       ret = tcg2_measure_variable(dev, pcr_index,
> +                                   EV_EFI_VARIABLE_DRIVER_CONFIG,
> +                                   L"DeployedMode",
> +                                   &efi_global_variable_guid,
> +                                   size, &deployed_mode);
> +

tcg2_measure_variable() can't fail here?  Do we care if it does?

> +       size = sizeof(audit_mode);
> +       ret = efi_get_variable_int(L"AuditMode", &efi_global_variable_guid,
> +                                  NULL, &size, &audit_mode, NULL);
> +       if (ret != EFI_SUCCESS)
> +               return ret;
> +
> +       ret = tcg2_measure_variable(dev, pcr_index,
> +                                   EV_EFI_VARIABLE_DRIVER_CONFIG,
> +                                   L"AuditMode",
> +                                   &efi_global_variable_guid,
> +                                   size, &audit_mode);
> +

Does it make sense to read both of the variables first and measure
them only if both are present?
IOW is there any connection between AuditMode and DeployedMode measurements?


Regards
/Ilias
> +       return ret;
> +}
> +
>  /**
>   * tcg2_measure_secure_boot_variable() - measure secure boot variables
>   *
> @@ -1891,6 +1936,8 @@ static efi_status_t tcg2_measure_secure_boot_variable(struct udevice *dev)
>                 free(data);
>         }
>
> +       ret = tcg2_measure_deployed_audit_mode(dev);
> +
>  error:
>         return ret;
>  }
> --
> 2.17.1
>
Masahisa Kojima Sept. 28, 2021, 11:45 a.m. UTC | #2
On Mon, 27 Sept 2021 at 22:53, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> On Tue, 21 Sept 2021 at 10:17, Masahisa Kojima
> <masahisa.kojima@linaro.org> wrote:
> >
> > This commit adds the DeployedMode and AuditMode variable
> > measurement required in TCG PC Client PFP Spec.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> >
> > (no changes since v1)
> >
> >  lib/efi_loader/efi_tcg2.c | 47 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > index ea2c1ead03..68542c7cd3 100644
> > --- a/lib/efi_loader/efi_tcg2.c
> > +++ b/lib/efi_loader/efi_tcg2.c
> > @@ -12,6 +12,7 @@
> >  #include <dm.h>
> >  #include <efi_loader.h>
> >  #include <efi_tcg2.h>
> > +#include <efi_variable.h>
> >  #include <log.h>
> >  #include <malloc.h>
> >  #include <smbios.h>
> > @@ -1828,6 +1829,50 @@ out:
> >         return ret;
> >  }
> >
> > +/**
> > + * tcg2_measure_deployed_audit_mode() - measure deployedmode and auditmode
> > + *
> > + * @dev:       TPM device
> > + *
> > + * Return:     status code
> > + */
> > +static efi_status_t tcg2_measure_deployed_audit_mode(struct udevice *dev)
> > +{
> > +       u8 deployed_mode;
> > +       u8 audit_mode;
> > +       efi_uintn_t size;
> > +       efi_status_t ret;
> > +       u32 pcr_index;
> > +
> > +       size = sizeof(deployed_mode);
> > +       ret = efi_get_variable_int(L"DeployedMode", &efi_global_variable_guid,
> > +                                  NULL, &size, &deployed_mode, NULL);
> > +       if (ret != EFI_SUCCESS)
> > +               return ret;
> > +
> > +       pcr_index = (deployed_mode ? 1 : 7);
> > +
> > +       ret = tcg2_measure_variable(dev, pcr_index,
> > +                                   EV_EFI_VARIABLE_DRIVER_CONFIG,
> > +                                   L"DeployedMode",
> > +                                   &efi_global_variable_guid,
> > +                                   size, &deployed_mode);
> > +
>
> tcg2_measure_variable() can't fail here?  Do we care if it does?

I will add appropriate error handling.

>
> > +       size = sizeof(audit_mode);
> > +       ret = efi_get_variable_int(L"AuditMode", &efi_global_variable_guid,
> > +                                  NULL, &size, &audit_mode, NULL);
> > +       if (ret != EFI_SUCCESS)
> > +               return ret;
> > +
> > +       ret = tcg2_measure_variable(dev, pcr_index,
> > +                                   EV_EFI_VARIABLE_DRIVER_CONFIG,
> > +                                   L"AuditMode",
> > +                                   &efi_global_variable_guid,
> > +                                   size, &audit_mode);
> > +
>
> Does it make sense to read both of the variables first and measure
> them only if both are present?

Yes, it is better. If one of the variable is not present, skip both DeployedMode
and AuditMode measurement.

> IOW is there any connection between AuditMode and DeployedMode measurements?

In UEFI spec:
 DeployedMode = 1 -> AuditMode is always 0
 DeployedMode = 0 -> AuditMode can be 0 or 1

Thanks,
Masahisa Kojima

>
>
> Regards
> /Ilias
> > +       return ret;
> > +}
> > +
> >  /**
> >   * tcg2_measure_secure_boot_variable() - measure secure boot variables
> >   *
> > @@ -1891,6 +1936,8 @@ static efi_status_t tcg2_measure_secure_boot_variable(struct udevice *dev)
> >                 free(data);
> >         }
> >
> > +       ret = tcg2_measure_deployed_audit_mode(dev);
> > +
> >  error:
> >         return ret;
> >  }
> > --
> > 2.17.1
> >
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index ea2c1ead03..68542c7cd3 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -12,6 +12,7 @@ 
 #include <dm.h>
 #include <efi_loader.h>
 #include <efi_tcg2.h>
+#include <efi_variable.h>
 #include <log.h>
 #include <malloc.h>
 #include <smbios.h>
@@ -1828,6 +1829,50 @@  out:
 	return ret;
 }
 
+/**
+ * tcg2_measure_deployed_audit_mode() - measure deployedmode and auditmode
+ *
+ * @dev:	TPM device
+ *
+ * Return:	status code
+ */
+static efi_status_t tcg2_measure_deployed_audit_mode(struct udevice *dev)
+{
+	u8 deployed_mode;
+	u8 audit_mode;
+	efi_uintn_t size;
+	efi_status_t ret;
+	u32 pcr_index;
+
+	size = sizeof(deployed_mode);
+	ret = efi_get_variable_int(L"DeployedMode", &efi_global_variable_guid,
+				   NULL, &size, &deployed_mode, NULL);
+	if (ret != EFI_SUCCESS)
+		return ret;
+
+	pcr_index = (deployed_mode ? 1 : 7);
+
+	ret = tcg2_measure_variable(dev, pcr_index,
+				    EV_EFI_VARIABLE_DRIVER_CONFIG,
+				    L"DeployedMode",
+				    &efi_global_variable_guid,
+				    size, &deployed_mode);
+
+	size = sizeof(audit_mode);
+	ret = efi_get_variable_int(L"AuditMode", &efi_global_variable_guid,
+				   NULL, &size, &audit_mode, NULL);
+	if (ret != EFI_SUCCESS)
+		return ret;
+
+	ret = tcg2_measure_variable(dev, pcr_index,
+				    EV_EFI_VARIABLE_DRIVER_CONFIG,
+				    L"AuditMode",
+				    &efi_global_variable_guid,
+				    size, &audit_mode);
+
+	return ret;
+}
+
 /**
  * tcg2_measure_secure_boot_variable() - measure secure boot variables
  *
@@ -1891,6 +1936,8 @@  static efi_status_t tcg2_measure_secure_boot_variable(struct udevice *dev)
 		free(data);
 	}
 
+	ret = tcg2_measure_deployed_audit_mode(dev);
+
 error:
 	return ret;
 }