diff mbox series

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

Message ID 20211001111844.7422-4-masahisa.kojima@linaro.org
State New
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(+)

-- 
2.17.1

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;
 }