diff mbox series

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

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

Commit Message

Masahisa Kojima Oct. 1, 2021, 11:18 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>
---

Changes in v3:
- read variable first, then mesure the variable

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

Comments

Heinrich Schuchardt Oct. 1, 2021, 4:43 p.m. UTC | #1 
On 10/1/21 13:18, Masahisa Kojima 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>
> ---
>
> Changes in v3:
> - read variable first, then mesure the variable
>
>   lib/efi_loader/efi_tcg2.c | 50 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 50 insertions(+)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 28e0362bf2..7fba4bc458 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>
> @@ -1822,6 +1823,53 @@ 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;
> +
> +	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;
> +
> +	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);
> +	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
>    *
> @@ -1885,6 +1933,8 @@ static efi_status_t tcg2_measure_secure_boot_variable(struct udevice *dev)
>   		free(data);
>   	}
>
> +	ret = tcg2_measure_deployed_audit_mode(dev);

You do the same thing four times. A loop is preferable.

Best regards

Heinrich

> +
>   error:
>   	return ret;
>   }
>
Masahisa Kojima Oct. 4, 2021, 2:30 a.m. UTC | #2 
On Sat, 2 Oct 2021 at 01:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> On 10/1/21 13:18, Masahisa Kojima 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>
> > ---
> >
> > Changes in v3:
> > - read variable first, then mesure the variable
> >
> >   lib/efi_loader/efi_tcg2.c | 50 +++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 50 insertions(+)
> >
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > index 28e0362bf2..7fba4bc458 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>
> > @@ -1822,6 +1823,53 @@ 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;
> > +
> > +     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;
> > +
> > +     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);
> > +     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
> >    *
> > @@ -1885,6 +1933,8 @@ static efi_status_t tcg2_measure_secure_boot_variable(struct udevice *dev)
> >               free(data);
> >       }
> >
> > +     ret = tcg2_measure_deployed_audit_mode(dev);
>
> You do the same thing four times. A loop is preferable.

After your series(efi_loader: centralize known vendor GUIDs) is merged,
I will update this patch to use loop for the secure variables.

Thanks,
Masahisa Kojima

>
> Best regards
>
> Heinrich
>
> > +
> >   error:
> >       return ret;
> >   }
> >
Masahisa Kojima Oct. 22, 2021, 8:04 a.m. UTC | #3 
Hi Heinrich,

On Mon, 4 Oct 2021 at 11:30, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:
>
> On Sat, 2 Oct 2021 at 01:43, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> >
> >
> > On 10/1/21 13:18, Masahisa Kojima 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>
> > > ---
> > >
> > > Changes in v3:
> > > - read variable first, then mesure the variable
> > >
> > >   lib/efi_loader/efi_tcg2.c | 50 +++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 50 insertions(+)
> > >
> > > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > > index 28e0362bf2..7fba4bc458 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>
> > > @@ -1822,6 +1823,53 @@ 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;
> > > +
> > > +     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;
> > > +
> > > +     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);
> > > +     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
> > >    *
> > > @@ -1885,6 +1933,8 @@ static efi_status_t tcg2_measure_secure_boot_variable(struct udevice *dev)
> > >               free(data);
> > >       }
> > >
> > > +     ret = tcg2_measure_deployed_audit_mode(dev);
> >
> > You do the same thing four times. A loop is preferable.
>
> After your series(efi_loader: centralize known vendor GUIDs) is merged,
> I will update this patch to use loop for the secure variables.

I was just about to send v4 patch, I noticed that the following patches
are removed from efi-2022-01 branch.

5024ec8ea7 efi_loader: simplify tcg2_measure_secure_boot_variable()
1622f0234b efi_loader: simplify efi_sigstore_parse_sigdb()
57d1f4a29e efi_loader: function to get GUID for variable name
d326a99318 efi_loader: treat UEFI variable name as const

These patches were there at least until last week I think.
Do you plan not to merge these patches?

Thanks,
Masahisa Kojima


>
> Thanks,
> Masahisa Kojima
>
> >
> > Best regards
> >
> > Heinrich
> >
> > > +
> > >   error:
> > >       return ret;
> > >   }
> > >
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 28e0362bf2..7fba4bc458 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>
@@ -1822,6 +1823,53 @@  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;
+
+	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;
+
+	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);
+	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
  *
@@ -1885,6 +1933,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;
 }