Re: [PATCH v3 2/6] tpm: Support boot measurements

2023-01-24 Thread Ilias Apalodimas
Hi Eddie,

On Mon, Jan 23, 2023 at 02:15:18PM -0600, Eddie James wrote:
>
> On 1/16/23 06:00, Ilias Apalodimas wrote:
> > Hi Eddie
> >
> >
> > > +static inline u16 tpm2_algorithm_to_len(enum tpm2_algorithms a)
> > > +{
> > > + switch (a) {
> > > + case TPM2_ALG_SHA1:
> > > + return TPM2_SHA1_DIGEST_SIZE;
> > > + case TPM2_ALG_SHA256:
> > > + return TPM2_SHA256_DIGEST_SIZE;
> > > + case TPM2_ALG_SHA384:
> > > + return TPM2_SHA384_DIGEST_SIZE;
> > > + case TPM2_ALG_SHA512:
> > > + return TPM2_SHA512_DIGEST_SIZE;
> > > + default:
> > > + return 0;
> > > + }
> > > +}
> > Any reason we can't move the static 'const struct digest_info
> > hash_algo_list' from the efi_tcg.c here?  We can then move the
> > functions defined in there alg_to_mask and alg_to_len.
> >
> > And since alg_to_mask is really just a bitshift maybe replace that?
>
>
> Hi,
>
> It seems more efficient to keep the switch statement rather than iterate
> through the structure array looking for the matching hash algorithm? I could
> remove the 'static const struct digest_info hash_algo_list' in efi_tcg2.c
> and instead use the tpm2_algorithm_to_len and tpm2_algorithm_to_mask in
> efi_tcg2.c. What do you think?

Sure that's fine. I just prefer a single implementation of getting sizes and 
masks

>
>
> > > +
> > > +#define tpm2_algorithm_to_mask(a)(1 << (a))
> > > +
> > >   /* NV index attributes */
> > > +

[...]

> > > + return 0;
> > > +}
> > > +
> > > +static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log 
> > > *elog)
> > > +{
> > I think we need to re-use efi_init_event_log here.  The reason is that on
> > Arm devices TF-A is capable of constructing an eventlog and passing it
> > along in memory.  That code takes that into account and tries to reuse the
> > existing EventLog passed from previous boot stages.
> >
> > The main difference between the EFI function and this one
> > is the allocated memory of the EventLog itself.  But even in this case, it
> > would be better to tweak the EFI code and do
> > create log -> Allocate EFI memory -> copy log and then use that for EFI
>
>
> OK... I'll try and get that to work. I see some potential issues like the
> fact that EFI finds the platform event log differently.
>

It's been a while since I wrote this but from memory you should
1. move the code from efi_init_event_log() and replace the memory
allocation calls from efi -> calloc
2. get rid of that create_final_event() since it's part of the EFI TCG
protocol.
3. Pass the eventlog to the EFI code eventually, allocate EFI memory and
copy it and create the final events table.

The main problem I can see is handling of the EFI Final Events
table but I think it's doable.

>
> >
> > > + struct tcg_efi_spec_id_event *ev;
> > > + struct tcg_pcr_event *log;
> > > + u32 event_size;
> > > + u32 count = 0;
> > > + u32 log_size;
> > > + u32 active;
> > > + u32 mask;
> > > + size_t i;
> > > + u16 len;
> > > + int rc;
> > > +
> > > + rc = tcg2_get_active_pcr_banks(dev, );
> > > + if (rc)
> > > + return rc;
> > > +
> > > + event_size = offsetof(struct tcg_efi_spec_id_event, digest_sizes);
> > > + for (i = 0; i < ARRAY_SIZE(tcg2algos); ++i) {
> > > + mask = tpm2_algorithm_to_mask(tcg2algos[i]);
> > > +
> > > + if (!(active & mask))
> > > + continue;
> > > +
> > > + switch (tcg2algos[i]) {
> > > + case TPM2_ALG_SHA1:
> > > + case TPM2_ALG_SHA256:
> > > + case TPM2_ALG_SHA384:
> > > + case TPM2_ALG_SHA512:
> > > + count++;
> > > + break;
> > > + default:
> > > + continue;
> > > + }
> > > + }
> > > +
> > > + event_size += 1 +
> > > + (sizeof(struct tcg_efi_spec_id_event_algorithm_size) * count);
> > > + log_size = offsetof(struct tcg_pcr_event, event) + event_size;
> > > +
> > > + if (log_size > elog->log_size) {
> > > + printf("%s: log too large: %u > %u\n", __func__, log_size,
> > > +elog->log_size);
> > > + return -ENOBUFS;
> > > + }
> > > +
> 
> > > +
> > > +int tcg2_measure_event(struct udevice *dev, struct tcg2_event_log *elog,
> > > +u32 pcr_index, u32 event_type, u32 size,
> > > +const u8 *event)
> > > +{
> > > + struct tpml_digest_values digest_list;
> > > + int rc;
> > > +
> > > + rc = tcg2_create_digest(dev, event, size, _list);
> > > + if (rc)
> > > + return rc;
> > > +
> > > + rc = tcg2_pcr_extend(dev, pcr_index, _list);
> > > + if (rc)
> > > + return rc;
> > > +
> > > + return tcg2_log_append_check(elog, pcr_index, event_type, _list,
> > > +  size, event);
> > > +}
> > There's a static efi_status_t tcg2_measure_event(...) left in efi_tcg.c
> > which breaks compilation.  WE should just use the one you added in tpm-v2.c
>
>
> Hmm. The EFI version does a slightly different way of event logging, so I'm
> not sure it's that simple. There is EFI 

Re: [PATCH v3 2/6] tpm: Support boot measurements

2023-01-23 Thread Simon Glass
Hi Ilias,

On Mon, 16 Jan 2023 at 03:51, Ilias Apalodimas
 wrote:
>
> Hi Simon,
>
> [...]
>
> > > >>
> > > > [..]
> > > >
> > > >> +static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log 
> > > >> *elog)
> > > >> +{
> > > >> +   struct tcg_efi_spec_id_event *ev;
> > > > We cannot add EFI things to generic TPM code.
> > >
> > >
> > > Ah, this is NOT an EFI thing even though it is named as such. Please see
> > > https://trustedcomputinggroup.org/wp-content/uploads/TCG_ServerManagDomainFWProfile_r1p00_pub.pdf
> > > section 9 and
> > > https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_Specific_Platform_Profile_for_TPM_2p0_1p04_PUBLIC.pdf
> > > section 9
> > >
> > >
> > > Neither of these documents are specific to EFI.
> >
> > OK, then please drop efi_ from the identifiers. It is terribly confusing.
>
> Why?  The terrible confusing part would be trying to read code not
> named after the spec because you don't like EFI.  When looking at code
> that's documented by a spec it's far easier to keep the naming as
> close as possible, so I'd prefer leaving it as is.

Because this is not an EFI spec, is it? What am I missing?

Regards,
Simon


>
> Regards
> /Ilias
> >
> > >
> > >
> > > >
> > > >> +   struct tcg_pcr_event *log;
> > > >> +   u32 event_size;
> > > >> +   u32 count = 0;
> > > >> +   u32 log_size;
> > > >> +   u32 active;
> > > >> +   u32 mask;
> > > >> +   size_t i;
> > > >> +   u16 len;
> > > >> +   int rc;
> > > >> +
> > > >> +   rc = tcg2_get_active_pcr_banks(dev, );
> > > >> +   if (rc)
> > > >> +   return rc;
> > > >> +
> > > >> +   event_size = offsetof(struct tcg_efi_spec_id_event, 
> > > >> digest_sizes);
> > > >> +   for (i = 0; i < ARRAY_SIZE(tcg2algos); ++i) {
> > > >> +   mask = tpm2_algorithm_to_mask(tcg2algos[i]);
> > > >> +
> > > >> +   if (!(active & mask))
> > > >> +   continue;
> > > >> +
> > > >> +   switch (tcg2algos[i]) {
> > > >> +   case TPM2_ALG_SHA1:
> > > >> +   case TPM2_ALG_SHA256:
> > > >> +   case TPM2_ALG_SHA384:
> > > >> +   case TPM2_ALG_SHA512:
> > > >> +   count++;
> > > >> +   break;
> > > >> +   default:
> > > >> +   continue;
> > > >> +   }
> > > >> +   }
> > > >> +
> > > >> +   event_size += 1 +
> > > >> +   (sizeof(struct tcg_efi_spec_id_event_algorithm_size) * 
> > > >> count);
> > > >> +   log_size = offsetof(struct tcg_pcr_event, event) + event_size;
> > > >> +
> > > >> +   if (log_size > elog->log_size) {
> > > >> +   printf("%s: log too large: %u > %u\n", __func__, 
> > > >> log_size,
> > > >> +  elog->log_size);
> > > >> +   return -ENOBUFS;
> > > >> +   }
> > > >> +
> > > >> +   log = (struct tcg_pcr_event *)elog->log;
> > > >> +   put_unaligned_le32(0, >pcr_index);
> > > >> +   put_unaligned_le32(EV_NO_ACTION, >event_type);
> > > >> +   memset(>digest, 0, sizeof(log->digest));
> > > >> +   put_unaligned_le32(event_size, >event_size);
> > > >> +
> > > >> +   ev = (struct tcg_efi_spec_id_event *)log->event;
> > > >> +   strlcpy((char *)ev->signature, 
> > > >> TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03,
> > > > Same with all of this.
> > > >
> > > >> +   sizeof(ev->signature));
> > > >> +   put_unaligned_le32(0, >platform_class);
> > > >> +   ev->spec_version_minor = 
> > > >> TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MINOR_TPM2;
> > > >> +   ev->spec_version_major = 
> > > >> TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MAJOR_TPM2;
> > > >> +   ev->spec_errata = 
> > > >> TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2;
> > > > I'm not quite sure what is going on here...is this log in a format
> > > > defined by the EFI spec? What if we are not using EFI? How would a
> > > > different format be used?
> > > >
> > > > Put another way, people using a TPM should not pull in EFI things just
> > > > to do that.
> > >
> > >
> > > Agreed, however the EFI spec is not involved. These specifications and
> > > structures are general to any boot measurement. I would guess EFI was
> > > the first to do this and therefore defined some structures that the TCG
> > > re-used when writing the specs.
> >
> > Regards,
> > Simon


Re: [PATCH v3 2/6] tpm: Support boot measurements

2023-01-23 Thread Eddie James



On 1/16/23 06:00, Ilias Apalodimas wrote:

Hi Eddie



+static inline u16 tpm2_algorithm_to_len(enum tpm2_algorithms a)
+{
+   switch (a) {
+   case TPM2_ALG_SHA1:
+   return TPM2_SHA1_DIGEST_SIZE;
+   case TPM2_ALG_SHA256:
+   return TPM2_SHA256_DIGEST_SIZE;
+   case TPM2_ALG_SHA384:
+   return TPM2_SHA384_DIGEST_SIZE;
+   case TPM2_ALG_SHA512:
+   return TPM2_SHA512_DIGEST_SIZE;
+   default:
+   return 0;
+   }
+}

Any reason we can't move the static 'const struct digest_info
hash_algo_list' from the efi_tcg.c here?  We can then move the
functions defined in there alg_to_mask and alg_to_len.

And since alg_to_mask is really just a bitshift maybe replace that?



Hi,

It seems more efficient to keep the switch statement rather than iterate 
through the structure array looking for the matching hash algorithm? I 
could remove the 'static const struct digest_info hash_algo_list' in 
efi_tcg2.c and instead use the tpm2_algorithm_to_len and 
tpm2_algorithm_to_mask in efi_tcg2.c. What do you think?




+
+#define tpm2_algorithm_to_mask(a)  (1 << (a))
+
  /* NV index attributes */
  enum tpm_index_attrs {
TPMA_NV_PPWRITE = 1UL << 0,
@@ -419,6 +481,142 @@ enum {
HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT,
  };

+/**
+ * struct tcg2_event_log - Container for managing the platform event log
+ *
+ * @log:   Address of the log
+ * @log_position:  Current entry position
+ * @log_size:  Log space available
+ */
+struct tcg2_event_log {
+   u8 *log;
+   u32 log_position;



-   }
-   }
-
-   *pcr_banks = pcrs.count;
-
-   return 0;
-out:
-   return -1;
-}
-
  /**
   * __get_active_pcr_banks() - returns the currently active PCR banks
   *
@@ -638,7 +378,7 @@ static efi_status_t __get_active_pcr_banks(u32 
*active_pcr_banks)
efi_status_t ret;
int err;

-   ret = platform_get_tpm2_device();
+   ret = tcg2_platform_get_tpm2();
if (ret != EFI_SUCCESS)
goto out;


We can get rid of this entirely and just define the
efi_tcg2_get_active_pcr_banks in the efi_tcg.c now.
__get_active_pcr_banks == tcg2_get_active_pcr_banks with the only
diffence being the udevice which is now an argument



Ack.





@@ -654,70 +394,6 @@ out:
return ret;
  }

   * efi_tcg2_get_capability() - protocol capability information and state 
information
   *
@@ -759,7 +435,7 @@ efi_tcg2_get_capability(struct efi_tcg2_protocol *this,
capability->protocol_version.major = 1;
capability->protocol_version.minor = 1;



+
+static int tcg2_log_append_check(struct tcg2_event_log *elog, u32 pcr_index,
+u32 event_type,
+struct tpml_digest_values *digest_list,
+u32 size, const u8 *event)
+{
+   u32 event_size;
+   u8 *log;
+
+   event_size = size + tcg2_event_get_size(digest_list);
+   if (elog->log_position + event_size > elog->log_size) {
+   printf("%s: log too large: %u + %u > %u\n", __func__,
+  elog->log_position, event_size, elog->log_size);
+   return -ENOBUFS;
+   }
+
+   log = elog->log + elog->log_position;
+   elog->log_position += event_size;
+
+   tcg2_log_append(pcr_index, event_type, digest_list, size, event, log);
+
+   return 0;
+}
+
+static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log *elog)
+{

I think we need to re-use efi_init_event_log here.  The reason is that on
Arm devices TF-A is capable of constructing an eventlog and passing it
along in memory.  That code takes that into account and tries to reuse the
existing EventLog passed from previous boot stages.

The main difference between the EFI function and this one
is the allocated memory of the EventLog itself.  But even in this case, it
would be better to tweak the EFI code and do
create log -> Allocate EFI memory -> copy log and then use that for EFI



OK... I'll try and get that to work. I see some potential issues like 
the fact that EFI finds the platform event log differently.






+   struct tcg_efi_spec_id_event *ev;
+   struct tcg_pcr_event *log;
+   u32 event_size;
+   u32 count = 0;
+   u32 log_size;
+   u32 active;
+   u32 mask;
+   size_t i;
+   u16 len;
+   int rc;
+
+   rc = tcg2_get_active_pcr_banks(dev, );
+   if (rc)
+   return rc;
+
+   event_size = offsetof(struct tcg_efi_spec_id_event, digest_sizes);
+   for (i = 0; i < ARRAY_SIZE(tcg2algos); ++i) {
+   mask = tpm2_algorithm_to_mask(tcg2algos[i]);
+
+   if (!(active & mask))
+   continue;
+
+   switch (tcg2algos[i]) {
+   case TPM2_ALG_SHA1:
+   case TPM2_ALG_SHA256:
+   case TPM2_ALG_SHA384:
+   case 

Re: [PATCH v3 2/6] tpm: Support boot measurements

2023-01-16 Thread Ilias Apalodimas
Hi Eddie


> +static inline u16 tpm2_algorithm_to_len(enum tpm2_algorithms a)
> +{
> + switch (a) {
> + case TPM2_ALG_SHA1:
> + return TPM2_SHA1_DIGEST_SIZE;
> + case TPM2_ALG_SHA256:
> + return TPM2_SHA256_DIGEST_SIZE;
> + case TPM2_ALG_SHA384:
> + return TPM2_SHA384_DIGEST_SIZE;
> + case TPM2_ALG_SHA512:
> + return TPM2_SHA512_DIGEST_SIZE;
> + default:
> + return 0;
> + }
> +}

Any reason we can't move the static 'const struct digest_info
hash_algo_list' from the efi_tcg.c here?  We can then move the
functions defined in there alg_to_mask and alg_to_len.

And since alg_to_mask is really just a bitshift maybe replace that?

> +
> +#define tpm2_algorithm_to_mask(a)(1 << (a))
> +
>  /* NV index attributes */
>  enum tpm_index_attrs {
>   TPMA_NV_PPWRITE = 1UL << 0,
> @@ -419,6 +481,142 @@ enum {
>   HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT,
>  };
>
> +/**
> + * struct tcg2_event_log - Container for managing the platform event log
> + *
> + * @log: Address of the log
> + * @log_position:Current entry position
> + * @log_size:Log space available
> + */
> +struct tcg2_event_log {
> + u8 *log;
> + u32 log_position;
> + u32 log_size;
> +};
> +
> +/**
> + * Create a list of digests of the supported PCR banks for a given input data
> + *
> + * @dev  TPM device
> + * @inputData
> + * @length   Length of the data to calculate the digest
> + * @digest_list  List of digests to fill in
> + *
> + * Return: zero on success, negative errno otherwise
> + */
> +int tcg2_create_digest(struct udevice *dev, const u8 *input, u32 length,
> +struct tpml_digest_values *digest_list);
> +
> +/**
> + * Get the event size of the specified digests
> + *
> + * @digest_list  List of digests for the event
> + *
> + * Return: Size in bytes of the event
> + */
> +u32 tcg2_event_get_size(struct tpml_digest_values *digest_list);
> +
> +/**
> + * tcg2_log_append - Append an event to an event log
> + *
> + * @pcr_indexIndex of the PCR
> + * @event_type   Type of event
> + * @digest_list List of digests to add
> + * @size Size of event
> + * @eventEvent data
> + * @log  Log buffer to append the event to
> + */
> +void tcg2_log_append(u32 pcr_index, u32 event_type,
> +  struct tpml_digest_values *digest_list, u32 size,
> +  const u8 *event, u8 *log);
> +
> +/**
> + * Extend the PCR with specified digests
> + *
> + * @dev  TPM device
> + * @pcr_indexIndex of the PCR
> + * @digest_list  List of digests to extend
> + *
> + * Return: zero on success, negative errno otherwise
> + */
> +int tcg2_pcr_extend(struct udevice *dev, u32 pcr_index,
> + struct tpml_digest_values *digest_list);
> +
> +/**
> + * Measure data into the TPM PCRs and the platform event log.
> + *
> + * @dev  TPM device
> + * @log  Platform event log
> + * @pcr_indexIndex of the PCR
> + * @size Size of the data
> + * @data Pointer to the data
> + * @event_type   Event log type
> + * @event_size   Size of the event
> + * @eventPointer to the event
> + *
> + * Return: zero on success, negative errno otherwise
> + */
> +int tcg2_measure_data(struct udevice *dev, struct tcg2_event_log *elog,
> +   u32 pcr_index, u32 size, const u8 *data, u32 event_type,
> +   u32 event_size, const u8 *event);
> +
> +/**
> + * Measure an event into the platform event log.
> + *
> + * @dev  TPM device
> + * @log  Platform event log
> + * @pcr_indexIndex of the PCR
> + * @event_type   Event log type
> + * @size Size of the event
> + * @eventPointer to the event
> + *
> + * Return: zero on success, negative errno otherwise
> + */
> +int tcg2_measure_event(struct udevice *dev, struct tcg2_event_log *elog,
> +u32 pcr_index, u32 event_type, u32 size,
> +const u8 *event);
> +
> +/**
> + * Begin measurements.
> + *
> + * Return: zero on success, negative errno otherwise
> + */
> +int tcg2_measurement_init(struct udevice **dev, struct tcg2_event_log *elog);
> +
> +/**
> + * Stop measurements and record separator events.
> + */
> +void tcg2_measurement_term(struct udevice *dev, struct tcg2_event_log *elog,
> +bool error);
> +
> +/**
> + * Get the platform event log address and size.
> + *
> + * @dev  TPM device
> + * @addr Address of the log
> + * @size Size of the log
> + *
> + * Return: zero on success, negative errno otherwise
> + */
> +int tcg2_platform_get_log(struct udevice *dev, void **addr, u32 *size);
> +
> +/**
> + * Get the first TPM2 device found.
> + *
> + * @dev  TPM device
> + *
> + * Return: zero on success, negative errno otherwise
> + */
> +int 

Re: [PATCH v3 2/6] tpm: Support boot measurements

2023-01-16 Thread Ilias Apalodimas
Hi Simon,

[...]

> > >>
> > > [..]
> > >
> > >> +static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log 
> > >> *elog)
> > >> +{
> > >> +   struct tcg_efi_spec_id_event *ev;
> > > We cannot add EFI things to generic TPM code.
> >
> >
> > Ah, this is NOT an EFI thing even though it is named as such. Please see
> > https://trustedcomputinggroup.org/wp-content/uploads/TCG_ServerManagDomainFWProfile_r1p00_pub.pdf
> > section 9 and
> > https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_Specific_Platform_Profile_for_TPM_2p0_1p04_PUBLIC.pdf
> > section 9
> >
> >
> > Neither of these documents are specific to EFI.
>
> OK, then please drop efi_ from the identifiers. It is terribly confusing.

Why?  The terrible confusing part would be trying to read code not
named after the spec because you don't like EFI.  When looking at code
that's documented by a spec it's far easier to keep the naming as
close as possible, so I'd prefer leaving it as is.

Regards
/Ilias
>
> >
> >
> > >
> > >> +   struct tcg_pcr_event *log;
> > >> +   u32 event_size;
> > >> +   u32 count = 0;
> > >> +   u32 log_size;
> > >> +   u32 active;
> > >> +   u32 mask;
> > >> +   size_t i;
> > >> +   u16 len;
> > >> +   int rc;
> > >> +
> > >> +   rc = tcg2_get_active_pcr_banks(dev, );
> > >> +   if (rc)
> > >> +   return rc;
> > >> +
> > >> +   event_size = offsetof(struct tcg_efi_spec_id_event, 
> > >> digest_sizes);
> > >> +   for (i = 0; i < ARRAY_SIZE(tcg2algos); ++i) {
> > >> +   mask = tpm2_algorithm_to_mask(tcg2algos[i]);
> > >> +
> > >> +   if (!(active & mask))
> > >> +   continue;
> > >> +
> > >> +   switch (tcg2algos[i]) {
> > >> +   case TPM2_ALG_SHA1:
> > >> +   case TPM2_ALG_SHA256:
> > >> +   case TPM2_ALG_SHA384:
> > >> +   case TPM2_ALG_SHA512:
> > >> +   count++;
> > >> +   break;
> > >> +   default:
> > >> +   continue;
> > >> +   }
> > >> +   }
> > >> +
> > >> +   event_size += 1 +
> > >> +   (sizeof(struct tcg_efi_spec_id_event_algorithm_size) * 
> > >> count);
> > >> +   log_size = offsetof(struct tcg_pcr_event, event) + event_size;
> > >> +
> > >> +   if (log_size > elog->log_size) {
> > >> +   printf("%s: log too large: %u > %u\n", __func__, 
> > >> log_size,
> > >> +  elog->log_size);
> > >> +   return -ENOBUFS;
> > >> +   }
> > >> +
> > >> +   log = (struct tcg_pcr_event *)elog->log;
> > >> +   put_unaligned_le32(0, >pcr_index);
> > >> +   put_unaligned_le32(EV_NO_ACTION, >event_type);
> > >> +   memset(>digest, 0, sizeof(log->digest));
> > >> +   put_unaligned_le32(event_size, >event_size);
> > >> +
> > >> +   ev = (struct tcg_efi_spec_id_event *)log->event;
> > >> +   strlcpy((char *)ev->signature, 
> > >> TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03,
> > > Same with all of this.
> > >
> > >> +   sizeof(ev->signature));
> > >> +   put_unaligned_le32(0, >platform_class);
> > >> +   ev->spec_version_minor = 
> > >> TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MINOR_TPM2;
> > >> +   ev->spec_version_major = 
> > >> TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MAJOR_TPM2;
> > >> +   ev->spec_errata = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2;
> > > I'm not quite sure what is going on here...is this log in a format
> > > defined by the EFI spec? What if we are not using EFI? How would a
> > > different format be used?
> > >
> > > Put another way, people using a TPM should not pull in EFI things just
> > > to do that.
> >
> >
> > Agreed, however the EFI spec is not involved. These specifications and
> > structures are general to any boot measurement. I would guess EFI was
> > the first to do this and therefore defined some structures that the TCG
> > re-used when writing the specs.
>
> Regards,
> Simon


Re: [PATCH v3 2/6] tpm: Support boot measurements

2023-01-13 Thread Simon Glass
Hi Eddie,

On Fri, 13 Jan 2023 at 07:46, Eddie James  wrote:
>
>
> On 1/12/23 17:43, Simon Glass wrote:
> > Hi Eddie,
> >
> > On Thu, 12 Jan 2023 at 09:16, Eddie James  wrote:
> >> Add TPM2 functions to support boot measurement. This includes
> >> starting up the TPM, initializing/appending the event log, and
> >> measuring the U-Boot version. Much of the code was used in the
> >> EFI subsystem, so remove it there and use the common functions.
> >>
> >> Signed-off-by: Eddie James 
> >> ---
> >>   include/efi_tcg2.h|  44 ---
> >>   include/tpm-v2.h  | 211 
> >>   lib/efi_loader/efi_tcg2.c | 362 +--
> >>   lib/tpm-v2.c  | 708 ++
> >>   4 files changed, 938 insertions(+), 387 deletions(-)
> > [..]
> >
> >> diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> >> index 697b982e07..00e1b04d74 100644
> >> --- a/lib/tpm-v2.c
> >> +++ b/lib/tpm-v2.c
> >> @@ -4,13 +4,597 @@
> >>* Author: Miquel Raynal 
> >>*/
> >>
> >> +#include 
> >> +#include 
> > Please check header order:
> >
> > https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files
>
>
> Sure, however I did have a compile error with sandbox build due to
> missing phys_addr_t definition in asm/io.h...

Oh, perhaps that needs to be added to that header? But if it cannot be
fixed, you may need to split some part of an existing header into a
separate file, as was done with ofnode.h into ofnode_decl.h

>
>
> >
> >>   #include 
> >>   #include 
> >> +#include 
> >>   #include 
> >>   #include 
> >>   #include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >>   #include "tpm-utils.h"
> >>
> > [..]
> >
> >> +static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log *elog)
> >> +{
> >> +   struct tcg_efi_spec_id_event *ev;
> > We cannot add EFI things to generic TPM code.
>
>
> Ah, this is NOT an EFI thing even though it is named as such. Please see
> https://trustedcomputinggroup.org/wp-content/uploads/TCG_ServerManagDomainFWProfile_r1p00_pub.pdf
> section 9 and
> https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_Specific_Platform_Profile_for_TPM_2p0_1p04_PUBLIC.pdf
> section 9
>
>
> Neither of these documents are specific to EFI.

OK, then please drop efi_ from the identifiers. It is terribly confusing.

>
>
> >
> >> +   struct tcg_pcr_event *log;
> >> +   u32 event_size;
> >> +   u32 count = 0;
> >> +   u32 log_size;
> >> +   u32 active;
> >> +   u32 mask;
> >> +   size_t i;
> >> +   u16 len;
> >> +   int rc;
> >> +
> >> +   rc = tcg2_get_active_pcr_banks(dev, );
> >> +   if (rc)
> >> +   return rc;
> >> +
> >> +   event_size = offsetof(struct tcg_efi_spec_id_event, digest_sizes);
> >> +   for (i = 0; i < ARRAY_SIZE(tcg2algos); ++i) {
> >> +   mask = tpm2_algorithm_to_mask(tcg2algos[i]);
> >> +
> >> +   if (!(active & mask))
> >> +   continue;
> >> +
> >> +   switch (tcg2algos[i]) {
> >> +   case TPM2_ALG_SHA1:
> >> +   case TPM2_ALG_SHA256:
> >> +   case TPM2_ALG_SHA384:
> >> +   case TPM2_ALG_SHA512:
> >> +   count++;
> >> +   break;
> >> +   default:
> >> +   continue;
> >> +   }
> >> +   }
> >> +
> >> +   event_size += 1 +
> >> +   (sizeof(struct tcg_efi_spec_id_event_algorithm_size) * 
> >> count);
> >> +   log_size = offsetof(struct tcg_pcr_event, event) + event_size;
> >> +
> >> +   if (log_size > elog->log_size) {
> >> +   printf("%s: log too large: %u > %u\n", __func__, log_size,
> >> +  elog->log_size);
> >> +   return -ENOBUFS;
> >> +   }
> >> +
> >> +   log = (struct tcg_pcr_event *)elog->log;
> >> +   put_unaligned_le32(0, >pcr_index);
> >> +   put_unaligned_le32(EV_NO_ACTION, >event_type);
> >> +   memset(>digest, 0, sizeof(log->digest));
> >> +   put_unaligned_le32(event_size, >event_size);
> >> +
> >> +   ev = (struct tcg_efi_spec_id_event *)log->event;
> >> +   strlcpy((char *)ev->signature, TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03,
> > Same with all of this.
> >
> >> +   sizeof(ev->signature));
> >> +   put_unaligned_le32(0, >platform_class);
> >> +   ev->spec_version_minor = 
> >> TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MINOR_TPM2;
> >> +   ev->spec_version_major = 
> >> TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MAJOR_TPM2;
> >> +   ev->spec_errata = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2;
> > I'm not quite sure what is going on here...is this log in a format
> > defined by the EFI spec? What if we are not using EFI? How would a
> > different format be used?
> >
> > Put another way, people using a TPM should not pull in EFI things just
> > to do that.
>
>
> Agreed, 

Re: [PATCH v3 2/6] tpm: Support boot measurements

2023-01-13 Thread Eddie James



On 1/12/23 17:43, Simon Glass wrote:

Hi Eddie,

On Thu, 12 Jan 2023 at 09:16, Eddie James  wrote:

Add TPM2 functions to support boot measurement. This includes
starting up the TPM, initializing/appending the event log, and
measuring the U-Boot version. Much of the code was used in the
EFI subsystem, so remove it there and use the common functions.

Signed-off-by: Eddie James 
---
  include/efi_tcg2.h|  44 ---
  include/tpm-v2.h  | 211 
  lib/efi_loader/efi_tcg2.c | 362 +--
  lib/tpm-v2.c  | 708 ++
  4 files changed, 938 insertions(+), 387 deletions(-)

[..]


diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index 697b982e07..00e1b04d74 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -4,13 +4,597 @@
   * Author: Miquel Raynal 
   */

+#include 
+#include 

Please check header order:

https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files



Sure, however I did have a compile error with sandbox build due to 
missing phys_addr_t definition in asm/io.h...






  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
  #include "tpm-utils.h"


[..]


+static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log *elog)
+{
+   struct tcg_efi_spec_id_event *ev;

We cannot add EFI things to generic TPM code.



Ah, this is NOT an EFI thing even though it is named as such. Please see 
https://trustedcomputinggroup.org/wp-content/uploads/TCG_ServerManagDomainFWProfile_r1p00_pub.pdf 
section 9 and 
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_Specific_Platform_Profile_for_TPM_2p0_1p04_PUBLIC.pdf 
section 9



Neither of these documents are specific to EFI.





+   struct tcg_pcr_event *log;
+   u32 event_size;
+   u32 count = 0;
+   u32 log_size;
+   u32 active;
+   u32 mask;
+   size_t i;
+   u16 len;
+   int rc;
+
+   rc = tcg2_get_active_pcr_banks(dev, );
+   if (rc)
+   return rc;
+
+   event_size = offsetof(struct tcg_efi_spec_id_event, digest_sizes);
+   for (i = 0; i < ARRAY_SIZE(tcg2algos); ++i) {
+   mask = tpm2_algorithm_to_mask(tcg2algos[i]);
+
+   if (!(active & mask))
+   continue;
+
+   switch (tcg2algos[i]) {
+   case TPM2_ALG_SHA1:
+   case TPM2_ALG_SHA256:
+   case TPM2_ALG_SHA384:
+   case TPM2_ALG_SHA512:
+   count++;
+   break;
+   default:
+   continue;
+   }
+   }
+
+   event_size += 1 +
+   (sizeof(struct tcg_efi_spec_id_event_algorithm_size) * count);
+   log_size = offsetof(struct tcg_pcr_event, event) + event_size;
+
+   if (log_size > elog->log_size) {
+   printf("%s: log too large: %u > %u\n", __func__, log_size,
+  elog->log_size);
+   return -ENOBUFS;
+   }
+
+   log = (struct tcg_pcr_event *)elog->log;
+   put_unaligned_le32(0, >pcr_index);
+   put_unaligned_le32(EV_NO_ACTION, >event_type);
+   memset(>digest, 0, sizeof(log->digest));
+   put_unaligned_le32(event_size, >event_size);
+
+   ev = (struct tcg_efi_spec_id_event *)log->event;
+   strlcpy((char *)ev->signature, TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03,

Same with all of this.


+   sizeof(ev->signature));
+   put_unaligned_le32(0, >platform_class);
+   ev->spec_version_minor = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MINOR_TPM2;
+   ev->spec_version_major = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MAJOR_TPM2;
+   ev->spec_errata = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2;

I'm not quite sure what is going on here...is this log in a format
defined by the EFI spec? What if we are not using EFI? How would a
different format be used?

Put another way, people using a TPM should not pull in EFI things just
to do that.



Agreed, however the EFI spec is not involved. These specifications and 
structures are general to any boot measurement. I would guess EFI was 
the first to do this and therefore defined some structures that the TCG 
re-used when writing the specs.





I'm just not quite sure of the best approach here...

Regards,
Simon


Re: [PATCH v3 2/6] tpm: Support boot measurements

2023-01-13 Thread Heinrich Schuchardt

On 1/13/23 08:07, Ilias Apalodimas wrote:

Hi Simon

On Fri, 13 Jan 2023 at 01:43, Simon Glass  wrote:


Hi Eddie,

On Thu, 12 Jan 2023 at 09:16, Eddie James  wrote:


Add TPM2 functions to support boot measurement. This includes
starting up the TPM, initializing/appending the event log, and
measuring the U-Boot version. Much of the code was used in the
EFI subsystem, so remove it there and use the common functions.

Signed-off-by: Eddie James 
---
  include/efi_tcg2.h|  44 ---
  include/tpm-v2.h  | 211 
  lib/efi_loader/efi_tcg2.c | 362 +--
  lib/tpm-v2.c  | 708 ++
  4 files changed, 938 insertions(+), 387 deletions(-)


[..]


diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index 697b982e07..00e1b04d74 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -4,13 +4,597 @@
   * Author: Miquel Raynal 
   */



[...]




+static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log *elog)
+{
+   struct tcg_efi_spec_id_event *ev;


We cannot add EFI things to generic TPM code.


+   struct tcg_pcr_event *log;
+   u32 event_size;
+   u32 count = 0;
+   u32 log_size;
+   u32 active;
+   u32 mask;
+   size_t i;
+   u16 len;
+   int rc;
+
+   rc = tcg2_get_active_pcr_banks(dev, );
+   if (rc)
+   return rc;
+
+   event_size = offsetof(struct tcg_efi_spec_id_event, digest_sizes);
+   for (i = 0; i < ARRAY_SIZE(tcg2algos); ++i) {
+   mask = tpm2_algorithm_to_mask(tcg2algos[i]);
+
+   if (!(active & mask))
+   continue;
+
+   switch (tcg2algos[i]) {
+   case TPM2_ALG_SHA1:
+   case TPM2_ALG_SHA256:
+   case TPM2_ALG_SHA384:
+   case TPM2_ALG_SHA512:
+   count++;
+   break;
+   default:
+   continue;
+   }
+   }
+
+   event_size += 1 +
+   (sizeof(struct tcg_efi_spec_id_event_algorithm_size) * count);
+   log_size = offsetof(struct tcg_pcr_event, event) + event_size;
+
+   if (log_size > elog->log_size) {
+   printf("%s: log too large: %u > %u\n", __func__, log_size,
+  elog->log_size);
+   return -ENOBUFS;
+   }
+
+   log = (struct tcg_pcr_event *)elog->log;
+   put_unaligned_le32(0, >pcr_index);
+   put_unaligned_le32(EV_NO_ACTION, >event_type);
+   memset(>digest, 0, sizeof(log->digest));
+   put_unaligned_le32(event_size, >event_size);
+
+   ev = (struct tcg_efi_spec_id_event *)log->event;
+   strlcpy((char *)ev->signature, TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03,


Same with all of this.


+   sizeof(ev->signature));
+   put_unaligned_le32(0, >platform_class);
+   ev->spec_version_minor = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MINOR_TPM2;
+   ev->spec_version_major = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MAJOR_TPM2;
+   ev->spec_errata = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2;


I'm not quite sure what is going on here...is this log in a format
defined by the EFI spec? What if we are not using EFI? How would a
different format be used?


Yes.  The TCG eventlog and everything Eddie is trying to add are
defined by an extension to the EFI spec.  I am unaware of any forms of
measurement (with a TPM).  The eventlong is purely a software
construct.  The TPM PCR extension involves taking measurements and
talking to the hardware.  Nothing prevents you from doing this outside
EFI.   What I am curious about is how these measurements are used by
the OS in Eddie's case.

When booting with EFI, the kernel calls the GetEventlog callback and
stores the event log in memory.  What happens to bootm?


The eventlog will be in reserved memory. The following patch is needed
to access it in Linux:

[PATCH] tpm: Add reserved memory event log
https://lore.kernel.org/lkml/20230103162010.381214-1-eaja...@linux.ibm.com/

The concept looks ok to me.

Best regards

Heinrich





Put another way, people using a TPM should not pull in EFI things just
to do that.

I'm just not quite sure of the best approach here...


The extensions to the EFI spec defines
1.  What the eventlog format looks like
2.  Functions to configure the TPM pcr banks, retrieve the eventlog
from memory etc.

Regards
/Ilias


Regards,
Simon




Re: [PATCH v3 2/6] tpm: Support boot measurements

2023-01-12 Thread Ilias Apalodimas
Hi Simon

On Fri, 13 Jan 2023 at 01:43, Simon Glass  wrote:
>
> Hi Eddie,
>
> On Thu, 12 Jan 2023 at 09:16, Eddie James  wrote:
> >
> > Add TPM2 functions to support boot measurement. This includes
> > starting up the TPM, initializing/appending the event log, and
> > measuring the U-Boot version. Much of the code was used in the
> > EFI subsystem, so remove it there and use the common functions.
> >
> > Signed-off-by: Eddie James 
> > ---
> >  include/efi_tcg2.h|  44 ---
> >  include/tpm-v2.h  | 211 
> >  lib/efi_loader/efi_tcg2.c | 362 +--
> >  lib/tpm-v2.c  | 708 ++
> >  4 files changed, 938 insertions(+), 387 deletions(-)
>
> [..]
>
> > diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> > index 697b982e07..00e1b04d74 100644
> > --- a/lib/tpm-v2.c
> > +++ b/lib/tpm-v2.c
> > @@ -4,13 +4,597 @@
> >   * Author: Miquel Raynal 
> >   */
> >

[...]

>
> > +static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log *elog)
> > +{
> > +   struct tcg_efi_spec_id_event *ev;
>
> We cannot add EFI things to generic TPM code.
>
> > +   struct tcg_pcr_event *log;
> > +   u32 event_size;
> > +   u32 count = 0;
> > +   u32 log_size;
> > +   u32 active;
> > +   u32 mask;
> > +   size_t i;
> > +   u16 len;
> > +   int rc;
> > +
> > +   rc = tcg2_get_active_pcr_banks(dev, );
> > +   if (rc)
> > +   return rc;
> > +
> > +   event_size = offsetof(struct tcg_efi_spec_id_event, digest_sizes);
> > +   for (i = 0; i < ARRAY_SIZE(tcg2algos); ++i) {
> > +   mask = tpm2_algorithm_to_mask(tcg2algos[i]);
> > +
> > +   if (!(active & mask))
> > +   continue;
> > +
> > +   switch (tcg2algos[i]) {
> > +   case TPM2_ALG_SHA1:
> > +   case TPM2_ALG_SHA256:
> > +   case TPM2_ALG_SHA384:
> > +   case TPM2_ALG_SHA512:
> > +   count++;
> > +   break;
> > +   default:
> > +   continue;
> > +   }
> > +   }
> > +
> > +   event_size += 1 +
> > +   (sizeof(struct tcg_efi_spec_id_event_algorithm_size) * 
> > count);
> > +   log_size = offsetof(struct tcg_pcr_event, event) + event_size;
> > +
> > +   if (log_size > elog->log_size) {
> > +   printf("%s: log too large: %u > %u\n", __func__, log_size,
> > +  elog->log_size);
> > +   return -ENOBUFS;
> > +   }
> > +
> > +   log = (struct tcg_pcr_event *)elog->log;
> > +   put_unaligned_le32(0, >pcr_index);
> > +   put_unaligned_le32(EV_NO_ACTION, >event_type);
> > +   memset(>digest, 0, sizeof(log->digest));
> > +   put_unaligned_le32(event_size, >event_size);
> > +
> > +   ev = (struct tcg_efi_spec_id_event *)log->event;
> > +   strlcpy((char *)ev->signature, TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03,
>
> Same with all of this.
>
> > +   sizeof(ev->signature));
> > +   put_unaligned_le32(0, >platform_class);
> > +   ev->spec_version_minor = 
> > TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MINOR_TPM2;
> > +   ev->spec_version_major = 
> > TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MAJOR_TPM2;
> > +   ev->spec_errata = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2;
>
> I'm not quite sure what is going on here...is this log in a format
> defined by the EFI spec? What if we are not using EFI? How would a
> different format be used?

Yes.  The TCG eventlog and everything Eddie is trying to add are
defined by an extension to the EFI spec.  I am unaware of any forms of
measurement (with a TPM).  The eventlong is purely a software
construct.  The TPM PCR extension involves taking measurements and
talking to the hardware.  Nothing prevents you from doing this outside
EFI.   What I am curious about is how these measurements are used by
the OS in Eddie's case.

When booting with EFI, the kernel calls the GetEventlog callback and
stores the event log in memory.  What happens to bootm?

>
> Put another way, people using a TPM should not pull in EFI things just
> to do that.
>
> I'm just not quite sure of the best approach here...

The extensions to the EFI spec defines
1.  What the eventlog format looks like
2.  Functions to configure the TPM pcr banks, retrieve the eventlog
from memory etc.

Regards
/Ilias
>
> Regards,
> Simon


Re: [PATCH v3 2/6] tpm: Support boot measurements

2023-01-12 Thread Simon Glass
Hi Eddie,

On Thu, 12 Jan 2023 at 09:16, Eddie James  wrote:
>
> Add TPM2 functions to support boot measurement. This includes
> starting up the TPM, initializing/appending the event log, and
> measuring the U-Boot version. Much of the code was used in the
> EFI subsystem, so remove it there and use the common functions.
>
> Signed-off-by: Eddie James 
> ---
>  include/efi_tcg2.h|  44 ---
>  include/tpm-v2.h  | 211 
>  lib/efi_loader/efi_tcg2.c | 362 +--
>  lib/tpm-v2.c  | 708 ++
>  4 files changed, 938 insertions(+), 387 deletions(-)

[..]

> diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> index 697b982e07..00e1b04d74 100644
> --- a/lib/tpm-v2.c
> +++ b/lib/tpm-v2.c
> @@ -4,13 +4,597 @@
>   * Author: Miquel Raynal 
>   */
>
> +#include 
> +#include 

Please check header order:

https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files

>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
>  #include "tpm-utils.h"
>

[..]

> +static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log *elog)
> +{
> +   struct tcg_efi_spec_id_event *ev;

We cannot add EFI things to generic TPM code.

> +   struct tcg_pcr_event *log;
> +   u32 event_size;
> +   u32 count = 0;
> +   u32 log_size;
> +   u32 active;
> +   u32 mask;
> +   size_t i;
> +   u16 len;
> +   int rc;
> +
> +   rc = tcg2_get_active_pcr_banks(dev, );
> +   if (rc)
> +   return rc;
> +
> +   event_size = offsetof(struct tcg_efi_spec_id_event, digest_sizes);
> +   for (i = 0; i < ARRAY_SIZE(tcg2algos); ++i) {
> +   mask = tpm2_algorithm_to_mask(tcg2algos[i]);
> +
> +   if (!(active & mask))
> +   continue;
> +
> +   switch (tcg2algos[i]) {
> +   case TPM2_ALG_SHA1:
> +   case TPM2_ALG_SHA256:
> +   case TPM2_ALG_SHA384:
> +   case TPM2_ALG_SHA512:
> +   count++;
> +   break;
> +   default:
> +   continue;
> +   }
> +   }
> +
> +   event_size += 1 +
> +   (sizeof(struct tcg_efi_spec_id_event_algorithm_size) * count);
> +   log_size = offsetof(struct tcg_pcr_event, event) + event_size;
> +
> +   if (log_size > elog->log_size) {
> +   printf("%s: log too large: %u > %u\n", __func__, log_size,
> +  elog->log_size);
> +   return -ENOBUFS;
> +   }
> +
> +   log = (struct tcg_pcr_event *)elog->log;
> +   put_unaligned_le32(0, >pcr_index);
> +   put_unaligned_le32(EV_NO_ACTION, >event_type);
> +   memset(>digest, 0, sizeof(log->digest));
> +   put_unaligned_le32(event_size, >event_size);
> +
> +   ev = (struct tcg_efi_spec_id_event *)log->event;
> +   strlcpy((char *)ev->signature, TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03,

Same with all of this.

> +   sizeof(ev->signature));
> +   put_unaligned_le32(0, >platform_class);
> +   ev->spec_version_minor = 
> TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MINOR_TPM2;
> +   ev->spec_version_major = 
> TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MAJOR_TPM2;
> +   ev->spec_errata = TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2;

I'm not quite sure what is going on here...is this log in a format
defined by the EFI spec? What if we are not using EFI? How would a
different format be used?

Put another way, people using a TPM should not pull in EFI things just
to do that.

I'm just not quite sure of the best approach here...

Regards,
Simon


[PATCH v3 2/6] tpm: Support boot measurements

2023-01-12 Thread Eddie James
Add TPM2 functions to support boot measurement. This includes
starting up the TPM, initializing/appending the event log, and
measuring the U-Boot version. Much of the code was used in the
EFI subsystem, so remove it there and use the common functions.

Signed-off-by: Eddie James 
---
 include/efi_tcg2.h|  44 ---
 include/tpm-v2.h  | 211 
 lib/efi_loader/efi_tcg2.c | 362 +--
 lib/tpm-v2.c  | 708 ++
 4 files changed, 938 insertions(+), 387 deletions(-)

diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index 874306dc11..23016773f4 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -129,50 +129,6 @@ struct efi_tcg2_boot_service_capability {
 #define BOOT_SERVICE_CAPABILITY_MIN \
offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks)
 
-#define TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03 "Spec ID Event03"
-#define TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MAJOR_TPM2 2
-#define TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MINOR_TPM2 0
-#define TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2 2
-
-/**
- *  struct TCG_EfiSpecIdEventAlgorithmSize - hashing algorithm information
- *
- *  @algorithm_id: algorithm defined in enum tpm2_algorithms
- *  @digest_size:  size of the algorithm
- */
-struct tcg_efi_spec_id_event_algorithm_size {
-   u16  algorithm_id;
-   u16  digest_size;
-} __packed;
-
-/**
- * struct TCG_EfiSpecIDEventStruct - content of the event log header
- *
- * @signature: signature, set to Spec ID Event03
- * @platform_class:class defined in TCG ACPI Specification
- * Client  Common Header.
- * @spec_version_minor:minor version
- * @spec_version_major:major version
- * @spec_version_errata:   major version
- * @uintn_size:size of the efi_uintn_t fields used in 
various
- * data structures used in this specification.
- * 0x01 indicates u32  and 0x02  indicates u64
- * @number_of_algorithms:  hashing algorithms used in this event log
- * @digest_sizes:  array of number_of_algorithms pairs
- * 1st member defines the algorithm id
- * 2nd member defines the algorithm size
- */
-struct tcg_efi_spec_id_event {
-   u8 signature[16];
-   u32 platform_class;
-   u8 spec_version_minor;
-   u8 spec_version_major;
-   u8 spec_errata;
-   u8 uintn_size;
-   u32 number_of_algorithms;
-   struct tcg_efi_spec_id_event_algorithm_size digest_sizes[];
-} __packed;
-
 /**
  * struct tdEFI_TCG2_FINAL_EVENTS_TABLE - log entries after Get Event Log
  * @version:   version number for this structure
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index 85feda3e06..e3db3419dd 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -214,6 +214,50 @@ struct tcg_pcr_event2 {
u8 event[];
 } __packed;
 
+/**
+ *  struct TCG_EfiSpecIdEventAlgorithmSize - hashing algorithm information
+ *
+ *  @algorithm_id: algorithm defined in enum tpm2_algorithms
+ *  @digest_size:  size of the algorithm
+ */
+struct tcg_efi_spec_id_event_algorithm_size {
+   u16  algorithm_id;
+   u16  digest_size;
+} __packed;
+
+#define TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03 "Spec ID Event03"
+#define TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MAJOR_TPM2 2
+#define TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_MINOR_TPM2 0
+#define TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2 2
+
+/**
+ * struct TCG_EfiSpecIDEventStruct - content of the event log header
+ *
+ * @signature: signature, set to Spec ID Event03
+ * @platform_class:class defined in TCG ACPI Specification
+ * Client  Common Header.
+ * @spec_version_minor:minor version
+ * @spec_version_major:major version
+ * @spec_version_errata:   major version
+ * @uintn_size:size of the efi_uintn_t fields used in 
various
+ * data structures used in this specification.
+ * 0x01 indicates u32  and 0x02  indicates u64
+ * @number_of_algorithms:  hashing algorithms used in this event log
+ * @digest_sizes:  array of number_of_algorithms pairs
+ * 1st member defines the algorithm id
+ * 2nd member defines the algorithm size
+ */
+struct tcg_efi_spec_id_event {
+   u8 signature[16];
+   u32 platform_class;
+   u8 spec_version_minor;
+   u8 spec_version_major;
+   u8 spec_errata;
+   u8 uintn_size;
+   u32 number_of_algorithms;
+   struct tcg_efi_spec_id_event_algorithm_size digest_sizes[];
+} __packed;
+
 /**
  * TPM2 Structure Tags for command/response buffers.
  *
@@ -340,6 +384,24 @@ enum tpm2_algorithms {
TPM2_ALG_SM3_256