Re: [PATCH v3 1/5] efi_loader: add secure boot variable measurement

2021-08-10 Thread Masahisa Kojima
Hi Akashi-san,

Thank you for your comment.

On Tue, 10 Aug 2021 at 10:44, AKASHI Takahiro
 wrote:
>
> Kojima-san,
>
> On Fri, Aug 06, 2021 at 04:02:11PM +0900, Masahisa Kojima wrote:
> > TCG PC Client PFP spec requires to measure the secure
> > boot policy before validating the UEFI image.
> > This commit adds the secure boot variable measurement
> > of "SecureBoot", "PK", "KEK", "db", "dbx", "dbt", and "dbr".
> >
> > Note that this implementation assumes that secure boot
> > variables are pre-configured and not be set/updated in runtime.
> >
> > Signed-off-by: Masahisa Kojima 
> > ---
> > Changes in v3:
> > - add "dbt" and "dbr" measurement
> > - accept empty variable measurement for "SecureBoot", "PK",
> >   "KEK", "db" and "dbx" as TCG2 spec requires
> > - fix comment format
> >
> > Changes in v2:
> > - missing null check for getting variable data
> > - some minor fix for readability
> >
> >  include/efi_tcg2.h|  20 +
> >  lib/efi_loader/efi_tcg2.c | 165 ++
> >  2 files changed, 185 insertions(+)
> >
> > diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
> > index bcfb98168a..497ba3ce94 100644
> > --- a/include/efi_tcg2.h
> > +++ b/include/efi_tcg2.h
> > @@ -142,6 +142,26 @@ struct efi_tcg2_final_events_table {
> >   struct tcg_pcr_event2 event[];
> >  };
> >
> > +/**
> > + * struct tdUEFI_VARIABLE_DATA - event log structure of UEFI variable
> > + * @variable_name:   The vendorGUID parameter in the
> > + *   GetVariable() API.
> > + * @unicode_name_length: The length in CHAR16 of the Unicode name of
> > + *   the variable.
> > + * @variable_data_length:The size of the variable data.
> > + * @unicode_name:The CHAR16 unicode name of the variable
> > + *   without NULL-terminator.
> > + * @variable_data:   The data parameter of the efi variable
> > + *   in the GetVariable() API.
> > + */
> > +struct efi_tcg2_uefi_variable_data {
> > + efi_guid_t variable_name;
> > + u64 unicode_name_length;
> > + u64 variable_data_length;
> > + u16 unicode_name[1];
> > + u8 variable_data[1];
> > +};
> > +
> >  struct efi_tcg2_protocol {
> >   efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this,
> >  struct 
> > efi_tcg2_boot_service_capability *capability);
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > index 1319a8b378..a2e9587cd0 100644
> > --- a/lib/efi_loader/efi_tcg2.c
> > +++ b/lib/efi_loader/efi_tcg2.c
> > @@ -78,6 +78,19 @@ static const struct digest_info hash_algo_list[] = {
> >   },
> >  };
> >
> > +struct variable_info {
> > + u16 *name;
> > + const efi_guid_t*guid;
> > +};
> > +
> > +static struct variable_info secure_variables[] = {
> > + {L"SecureBoot", _global_variable_guid},
> > + {L"PK", _global_variable_guid},
> > + {L"KEK", _global_variable_guid},
> > + {L"db", _guid_image_security_database},
> > + {L"dbx", _guid_image_security_database},
> > +};
> > +
> >  #define MAX_HASH_COUNT ARRAY_SIZE(hash_algo_list)
> >
> >  /**
> > @@ -1264,6 +1277,39 @@ free_pool:
> >   return ret;
> >  }
> >
> > +/**
> > + * tcg2_measure_event() - common function to add event log and extend PCR
> > + *
> > + * @dev: TPM device
> > + * @pcr_index:   PCR index
> > + * @event_type:  type of event added
> > + * @size:event size
> > + * @event:   event data
> > + *
> > + * Return:   status code
> > + */
> > +static efi_status_t EFIAPI
> > +tcg2_measure_event(struct udevice *dev, u32 pcr_index, u32 event_type,
> > +u32 size, u8 event[])
> > +{
> > + struct tpml_digest_values digest_list;
> > + efi_status_t ret;
> > +
> > + ret = tcg2_create_digest(event, size, _list);
> > + if (ret != EFI_SUCCESS)
> > + goto out;
> > +
> > + ret = tcg2_pcr_extend(dev, pcr_index, _list);
> > + if (ret != EFI_SUCCESS)
> > + goto out;
> > +
> > + ret = tcg2_agile_log_append(pcr_index, event_type, _list,
> > + size, event);
> > +
> > +out:
> > + return ret;
> > +}
> > +
> >  /**
> >   * efi_append_scrtm_version - Append an S-CRTM EV_S_CRTM_VERSION event on 
> > the
> >   * eventlog and extend the PCRs
> > @@ -1294,6 +1340,118 @@ out:
> >   return ret;
> >  }
> >
> > +/**
> > + * tcg2_measure_variable() - add variable event log and extend PCR
> > + *
> > + * @dev: TPM device
> > + * @pcr_index:   PCR index
> > + * @event_type:  type of event added
> > + * @var_name:variable name
> > + * @guid:guid
> > + * @data_size:   variable data size
> > + * @data:variable data
> > + *
> > + * Return:   status code
> > + */
> > +static 

Re: [PATCH v3 1/5] efi_loader: add secure boot variable measurement

2021-08-09 Thread AKASHI Takahiro
Kojima-san,

On Fri, Aug 06, 2021 at 04:02:11PM +0900, Masahisa Kojima wrote:
> TCG PC Client PFP spec requires to measure the secure
> boot policy before validating the UEFI image.
> This commit adds the secure boot variable measurement
> of "SecureBoot", "PK", "KEK", "db", "dbx", "dbt", and "dbr".
> 
> Note that this implementation assumes that secure boot
> variables are pre-configured and not be set/updated in runtime.
> 
> Signed-off-by: Masahisa Kojima 
> ---
> Changes in v3:
> - add "dbt" and "dbr" measurement
> - accept empty variable measurement for "SecureBoot", "PK",
>   "KEK", "db" and "dbx" as TCG2 spec requires
> - fix comment format
> 
> Changes in v2:
> - missing null check for getting variable data
> - some minor fix for readability
> 
>  include/efi_tcg2.h|  20 +
>  lib/efi_loader/efi_tcg2.c | 165 ++
>  2 files changed, 185 insertions(+)
> 
> diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
> index bcfb98168a..497ba3ce94 100644
> --- a/include/efi_tcg2.h
> +++ b/include/efi_tcg2.h
> @@ -142,6 +142,26 @@ struct efi_tcg2_final_events_table {
>   struct tcg_pcr_event2 event[];
>  };
>  
> +/**
> + * struct tdUEFI_VARIABLE_DATA - event log structure of UEFI variable
> + * @variable_name:   The vendorGUID parameter in the
> + *   GetVariable() API.
> + * @unicode_name_length: The length in CHAR16 of the Unicode name of
> + *   the variable.
> + * @variable_data_length:The size of the variable data.
> + * @unicode_name:The CHAR16 unicode name of the variable
> + *   without NULL-terminator.
> + * @variable_data:   The data parameter of the efi variable
> + *   in the GetVariable() API.
> + */
> +struct efi_tcg2_uefi_variable_data {
> + efi_guid_t variable_name;
> + u64 unicode_name_length;
> + u64 variable_data_length;
> + u16 unicode_name[1];
> + u8 variable_data[1];
> +};
> +
>  struct efi_tcg2_protocol {
>   efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this,
>  struct 
> efi_tcg2_boot_service_capability *capability);
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 1319a8b378..a2e9587cd0 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -78,6 +78,19 @@ static const struct digest_info hash_algo_list[] = {
>   },
>  };
>  
> +struct variable_info {
> + u16 *name;
> + const efi_guid_t*guid;
> +};
> +
> +static struct variable_info secure_variables[] = {
> + {L"SecureBoot", _global_variable_guid},
> + {L"PK", _global_variable_guid},
> + {L"KEK", _global_variable_guid},
> + {L"db", _guid_image_security_database},
> + {L"dbx", _guid_image_security_database},
> +};
> +
>  #define MAX_HASH_COUNT ARRAY_SIZE(hash_algo_list)
>  
>  /**
> @@ -1264,6 +1277,39 @@ free_pool:
>   return ret;
>  }
>  
> +/**
> + * tcg2_measure_event() - common function to add event log and extend PCR
> + *
> + * @dev: TPM device
> + * @pcr_index:   PCR index
> + * @event_type:  type of event added
> + * @size:event size
> + * @event:   event data
> + *
> + * Return:   status code
> + */
> +static efi_status_t EFIAPI
> +tcg2_measure_event(struct udevice *dev, u32 pcr_index, u32 event_type,
> +u32 size, u8 event[])
> +{
> + struct tpml_digest_values digest_list;
> + efi_status_t ret;
> +
> + ret = tcg2_create_digest(event, size, _list);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + ret = tcg2_pcr_extend(dev, pcr_index, _list);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + ret = tcg2_agile_log_append(pcr_index, event_type, _list,
> + size, event);
> +
> +out:
> + return ret;
> +}
> +
>  /**
>   * efi_append_scrtm_version - Append an S-CRTM EV_S_CRTM_VERSION event on the
>   * eventlog and extend the PCRs
> @@ -1294,6 +1340,118 @@ out:
>   return ret;
>  }
>  
> +/**
> + * tcg2_measure_variable() - add variable event log and extend PCR
> + *
> + * @dev: TPM device
> + * @pcr_index:   PCR index
> + * @event_type:  type of event added
> + * @var_name:variable name
> + * @guid:guid
> + * @data_size:   variable data size
> + * @data:variable data
> + *
> + * Return:   status code
> + */
> +static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index,
> +   u32 event_type, u16 *var_name,
> +   const efi_guid_t *guid,
> +   efi_uintn_t data_size, u8 *data)
> +{
> + u32 event_size;
> + efi_status_t ret;
> + struct 

[PATCH v3 1/5] efi_loader: add secure boot variable measurement

2021-08-06 Thread Masahisa Kojima
TCG PC Client PFP spec requires to measure the secure
boot policy before validating the UEFI image.
This commit adds the secure boot variable measurement
of "SecureBoot", "PK", "KEK", "db", "dbx", "dbt", and "dbr".

Note that this implementation assumes that secure boot
variables are pre-configured and not be set/updated in runtime.

Signed-off-by: Masahisa Kojima 
---
Changes in v3:
- add "dbt" and "dbr" measurement
- accept empty variable measurement for "SecureBoot", "PK",
  "KEK", "db" and "dbx" as TCG2 spec requires
- fix comment format

Changes in v2:
- missing null check for getting variable data
- some minor fix for readability

 include/efi_tcg2.h|  20 +
 lib/efi_loader/efi_tcg2.c | 165 ++
 2 files changed, 185 insertions(+)

diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index bcfb98168a..497ba3ce94 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -142,6 +142,26 @@ struct efi_tcg2_final_events_table {
struct tcg_pcr_event2 event[];
 };
 
+/**
+ * struct tdUEFI_VARIABLE_DATA - event log structure of UEFI variable
+ * @variable_name: The vendorGUID parameter in the
+ * GetVariable() API.
+ * @unicode_name_length:   The length in CHAR16 of the Unicode name of
+ * the variable.
+ * @variable_data_length:  The size of the variable data.
+ * @unicode_name:  The CHAR16 unicode name of the variable
+ * without NULL-terminator.
+ * @variable_data: The data parameter of the efi variable
+ * in the GetVariable() API.
+ */
+struct efi_tcg2_uefi_variable_data {
+   efi_guid_t variable_name;
+   u64 unicode_name_length;
+   u64 variable_data_length;
+   u16 unicode_name[1];
+   u8 variable_data[1];
+};
+
 struct efi_tcg2_protocol {
efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this,
   struct 
efi_tcg2_boot_service_capability *capability);
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 1319a8b378..a2e9587cd0 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -78,6 +78,19 @@ static const struct digest_info hash_algo_list[] = {
},
 };
 
+struct variable_info {
+   u16 *name;
+   const efi_guid_t*guid;
+};
+
+static struct variable_info secure_variables[] = {
+   {L"SecureBoot", _global_variable_guid},
+   {L"PK", _global_variable_guid},
+   {L"KEK", _global_variable_guid},
+   {L"db", _guid_image_security_database},
+   {L"dbx", _guid_image_security_database},
+};
+
 #define MAX_HASH_COUNT ARRAY_SIZE(hash_algo_list)
 
 /**
@@ -1264,6 +1277,39 @@ free_pool:
return ret;
 }
 
+/**
+ * tcg2_measure_event() - common function to add event log and extend PCR
+ *
+ * @dev:   TPM device
+ * @pcr_index: PCR index
+ * @event_type:type of event added
+ * @size:  event size
+ * @event: event data
+ *
+ * Return: status code
+ */
+static efi_status_t EFIAPI
+tcg2_measure_event(struct udevice *dev, u32 pcr_index, u32 event_type,
+  u32 size, u8 event[])
+{
+   struct tpml_digest_values digest_list;
+   efi_status_t ret;
+
+   ret = tcg2_create_digest(event, size, _list);
+   if (ret != EFI_SUCCESS)
+   goto out;
+
+   ret = tcg2_pcr_extend(dev, pcr_index, _list);
+   if (ret != EFI_SUCCESS)
+   goto out;
+
+   ret = tcg2_agile_log_append(pcr_index, event_type, _list,
+   size, event);
+
+out:
+   return ret;
+}
+
 /**
  * efi_append_scrtm_version - Append an S-CRTM EV_S_CRTM_VERSION event on the
  *   eventlog and extend the PCRs
@@ -1294,6 +1340,118 @@ out:
return ret;
 }
 
+/**
+ * tcg2_measure_variable() - add variable event log and extend PCR
+ *
+ * @dev:   TPM device
+ * @pcr_index: PCR index
+ * @event_type:type of event added
+ * @var_name:  variable name
+ * @guid:  guid
+ * @data_size: variable data size
+ * @data:  variable data
+ *
+ * Return: status code
+ */
+static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index,
+ u32 event_type, u16 *var_name,
+ const efi_guid_t *guid,
+ efi_uintn_t data_size, u8 *data)
+{
+   u32 event_size;
+   efi_status_t ret;
+   struct efi_tcg2_uefi_variable_data *event;
+
+   event_size = sizeof(event->variable_name) +
+sizeof(event->unicode_name_length) +
+sizeof(event->variable_data_length) +
+(u16_strlen(var_name) * sizeof(u16)) + data_size;
+   event =