Re: [PATCH v9 2/6] tpm: sandbox: Update for needed TPM2 capabilities

2023-05-09 Thread Ilias Apalodimas
Hi Eddie

On Thu, 16 Mar 2023 at 10:32, Ilias Apalodimas
 wrote:
>
> Hi Eddie,
>
> Apologies for the late reply, I am now getting back on this.
> There are some failures on the CI wrt to sandbox here [0].  Can you have a
> look ?
> Also I believe some of the existing tests are wrong because they are
> using PCR0 (which is always going to be extended).  Can you also pick up
> [1] with your series?
>
>
> [0] https://source.denx.de/u-boot/custodians/u-boot-tpm/-/pipelines/15471
> [1] 
> https://source.denx.de/u-boot/custodians/u-boot-tpm/-/commit/0d28387cac5fafa59e4367d1548e021eeebe2004

I did manage to figure this out. You need this [0] with your patches.
Keep in mind that
("efi_loader: fix EFI_ENTRY point on get_active_pcr_banks ") needs to
be squashed with your patches as it's causing U-Boot to crash on
selftests.

You don't really need ("tpm: clean up tpm_init usage ") this is a WIP
i am exploring.  I'll go ahead and send the remaining patches and cc
you.  Once/if those are accepted, you can resend your series with the
fix I mentioned

[0] https://source.denx.de/u-boot/custodians/u-boot-tpm/-/commits/eddie
Regards
/Ilias
>
> Thanks
> /Ilias
>
> On Wed, Mar 08, 2023 at 03:25:33PM -0600, Eddie James wrote:
> > The driver needs to support getting the PCRs in the capabilities
> > command. Fix various other things and support the max number
> > of PCRs for TPM2.
> > Remove the !SANDBOX dependency for EFI TCG2 as well.
> >
> > Signed-off-by: Eddie James 
> > Reviewed-by: Simon Glass 
> > Acked-by: Ilias Apalodimas 
> > ---
> > Changes since v8:
> >  - Use >= for checking the property against TPM2_PROPERTIES_OFFSET
> >
> > Changes since v5:
> >  - Remove the !SANDBOX dependency for EFI TCG2
> >
> >  drivers/tpm/tpm2_tis_sandbox.c | 100 -
> >  lib/efi_loader/Kconfig |   2 -
> >  2 files changed, 72 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c
> > index e4004cfcca..d15a28d9fc 100644
> > --- a/drivers/tpm/tpm2_tis_sandbox.c
> > +++ b/drivers/tpm/tpm2_tis_sandbox.c
> > @@ -22,11 +22,6 @@ enum tpm2_hierarchy {
> >   TPM2_HIERARCHY_NB,
> >  };
> >
> > -/* Subset of supported capabilities */
> > -enum tpm2_capability {
> > - TPM_CAP_TPM_PROPERTIES = 0x6,
> > -};
> > -
> >  /* Subset of supported properties */
> >  #define TPM2_PROPERTIES_OFFSET 0x020E
> >
> > @@ -38,7 +33,8 @@ enum tpm2_cap_tpm_property {
> >   TPM2_PROPERTY_NB,
> >  };
> >
> > -#define SANDBOX_TPM_PCR_NB 1
> > +#define SANDBOX_TPM_PCR_NB TPM2_MAX_PCRS
> > +#define SANDBOX_TPM_PCR_SELECT_MAX   ((SANDBOX_TPM_PCR_NB + 7) / 8)
> >
> >  /*
> >   * Information about our TPM emulation. This is preserved in the sandbox
> > @@ -433,7 +429,7 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const 
> > u8 *sendbuf,
> >   int i, j;
> >
> >   /* TPM2_GetProperty */
> > - u32 capability, property, property_count;
> > + u32 capability, property, property_count, val;
> >
> >   /* TPM2_PCR_Read/Extend variables */
> >   int pcr_index = 0;
> > @@ -542,19 +538,32 @@ static int sandbox_tpm2_xfer(struct udevice *dev, 
> > const u8 *sendbuf,
> >   case TPM2_CC_GET_CAPABILITY:
> >   capability = get_unaligned_be32(sent);
> >   sent += sizeof(capability);
> > - if (capability != TPM_CAP_TPM_PROPERTIES) {
> > - printf("Sandbox TPM only support TPM_CAPABILITIES\n");
> > - return TPM2_RC_HANDLE;
> > - }
> > -
> >   property = get_unaligned_be32(sent);
> >   sent += sizeof(property);
> > - property -= TPM2_PROPERTIES_OFFSET;
> > -
> >   property_count = get_unaligned_be32(sent);
> >   sent += sizeof(property_count);
> > - if (!property_count ||
> > - property + property_count > TPM2_PROPERTY_NB) {
> > +
> > + switch (capability) {
> > + case TPM2_CAP_PCRS:
> > + break;
> > + case TPM2_CAP_TPM_PROPERTIES:
> > + if (!property_count) {
> > + rc = TPM2_RC_HANDLE;
> > + return sandbox_tpm2_fill_buf(recv, recv_len,
> > +  tag, rc);
> > + }
> > +
> > + if (property >= TPM2_PROPERTIES_OFFSET &&
> > + ((property - TPM2_PROPERTIES_OFFSET) +
> > +  property_count > TPM2_PROPERTY_NB)) {
> > + rc = TPM2_RC_HANDLE;
> > + return sandbox_tpm2_fill_buf(recv, recv_len,
> > +  tag, rc);
> > + }
> > + break;
> > + default:
> > + printf("Sandbox TPM2 only supports TPM2_CAP_PCRS or "
> > +

Re: [PATCH v9 2/6] tpm: sandbox: Update for needed TPM2 capabilities

2023-03-16 Thread Ilias Apalodimas
Hi Eddie,

Apologies for the late reply, I am now getting back on this.
There are some failures on the CI wrt to sandbox here [0].  Can you have a
look ?
Also I believe some of the existing tests are wrong because they are
using PCR0 (which is always going to be extended).  Can you also pick up
[1] with your series?


[0] https://source.denx.de/u-boot/custodians/u-boot-tpm/-/pipelines/15471
[1] 
https://source.denx.de/u-boot/custodians/u-boot-tpm/-/commit/0d28387cac5fafa59e4367d1548e021eeebe2004

Thanks
/Ilias

On Wed, Mar 08, 2023 at 03:25:33PM -0600, Eddie James wrote:
> The driver needs to support getting the PCRs in the capabilities
> command. Fix various other things and support the max number
> of PCRs for TPM2.
> Remove the !SANDBOX dependency for EFI TCG2 as well.
>
> Signed-off-by: Eddie James 
> Reviewed-by: Simon Glass 
> Acked-by: Ilias Apalodimas 
> ---
> Changes since v8:
>  - Use >= for checking the property against TPM2_PROPERTIES_OFFSET
>
> Changes since v5:
>  - Remove the !SANDBOX dependency for EFI TCG2
>
>  drivers/tpm/tpm2_tis_sandbox.c | 100 -
>  lib/efi_loader/Kconfig |   2 -
>  2 files changed, 72 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c
> index e4004cfcca..d15a28d9fc 100644
> --- a/drivers/tpm/tpm2_tis_sandbox.c
> +++ b/drivers/tpm/tpm2_tis_sandbox.c
> @@ -22,11 +22,6 @@ enum tpm2_hierarchy {
>   TPM2_HIERARCHY_NB,
>  };
>
> -/* Subset of supported capabilities */
> -enum tpm2_capability {
> - TPM_CAP_TPM_PROPERTIES = 0x6,
> -};
> -
>  /* Subset of supported properties */
>  #define TPM2_PROPERTIES_OFFSET 0x020E
>
> @@ -38,7 +33,8 @@ enum tpm2_cap_tpm_property {
>   TPM2_PROPERTY_NB,
>  };
>
> -#define SANDBOX_TPM_PCR_NB 1
> +#define SANDBOX_TPM_PCR_NB TPM2_MAX_PCRS
> +#define SANDBOX_TPM_PCR_SELECT_MAX   ((SANDBOX_TPM_PCR_NB + 7) / 8)
>
>  /*
>   * Information about our TPM emulation. This is preserved in the sandbox
> @@ -433,7 +429,7 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const 
> u8 *sendbuf,
>   int i, j;
>
>   /* TPM2_GetProperty */
> - u32 capability, property, property_count;
> + u32 capability, property, property_count, val;
>
>   /* TPM2_PCR_Read/Extend variables */
>   int pcr_index = 0;
> @@ -542,19 +538,32 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const 
> u8 *sendbuf,
>   case TPM2_CC_GET_CAPABILITY:
>   capability = get_unaligned_be32(sent);
>   sent += sizeof(capability);
> - if (capability != TPM_CAP_TPM_PROPERTIES) {
> - printf("Sandbox TPM only support TPM_CAPABILITIES\n");
> - return TPM2_RC_HANDLE;
> - }
> -
>   property = get_unaligned_be32(sent);
>   sent += sizeof(property);
> - property -= TPM2_PROPERTIES_OFFSET;
> -
>   property_count = get_unaligned_be32(sent);
>   sent += sizeof(property_count);
> - if (!property_count ||
> - property + property_count > TPM2_PROPERTY_NB) {
> +
> + switch (capability) {
> + case TPM2_CAP_PCRS:
> + break;
> + case TPM2_CAP_TPM_PROPERTIES:
> + if (!property_count) {
> + rc = TPM2_RC_HANDLE;
> + return sandbox_tpm2_fill_buf(recv, recv_len,
> +  tag, rc);
> + }
> +
> + if (property >= TPM2_PROPERTIES_OFFSET &&
> + ((property - TPM2_PROPERTIES_OFFSET) +
> +  property_count > TPM2_PROPERTY_NB)) {
> + rc = TPM2_RC_HANDLE;
> + return sandbox_tpm2_fill_buf(recv, recv_len,
> +  tag, rc);
> + }
> + break;
> + default:
> + printf("Sandbox TPM2 only supports TPM2_CAP_PCRS or "
> +"TPM2_CAP_TPM_PROPERTIES\n");
>   rc = TPM2_RC_HANDLE;
>   return sandbox_tpm2_fill_buf(recv, recv_len, tag, rc);
>   }
> @@ -578,18 +587,53 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const 
> u8 *sendbuf,
>   put_unaligned_be32(capability, recv);
>   recv += sizeof(capability);
>
> - /* Give the number of properties that follow */
> - put_unaligned_be32(property_count, recv);
> - recv += sizeof(property_count);
> -
> - /* Fill with the properties */
> - for (i = 0; i < property_count; i++) {
> - put_unaligned_be32(TPM2_PROPERTIES_OFFSET + property +
> -i, recv);
> - recv += sizeof(property);
> - 

[PATCH v9 2/6] tpm: sandbox: Update for needed TPM2 capabilities

2023-03-08 Thread Eddie James
The driver needs to support getting the PCRs in the capabilities
command. Fix various other things and support the max number
of PCRs for TPM2.
Remove the !SANDBOX dependency for EFI TCG2 as well.

Signed-off-by: Eddie James 
Reviewed-by: Simon Glass 
Acked-by: Ilias Apalodimas 
---
Changes since v8:
 - Use >= for checking the property against TPM2_PROPERTIES_OFFSET

Changes since v5:
 - Remove the !SANDBOX dependency for EFI TCG2

 drivers/tpm/tpm2_tis_sandbox.c | 100 -
 lib/efi_loader/Kconfig |   2 -
 2 files changed, 72 insertions(+), 30 deletions(-)

diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c
index e4004cfcca..d15a28d9fc 100644
--- a/drivers/tpm/tpm2_tis_sandbox.c
+++ b/drivers/tpm/tpm2_tis_sandbox.c
@@ -22,11 +22,6 @@ enum tpm2_hierarchy {
TPM2_HIERARCHY_NB,
 };
 
-/* Subset of supported capabilities */
-enum tpm2_capability {
-   TPM_CAP_TPM_PROPERTIES = 0x6,
-};
-
 /* Subset of supported properties */
 #define TPM2_PROPERTIES_OFFSET 0x020E
 
@@ -38,7 +33,8 @@ enum tpm2_cap_tpm_property {
TPM2_PROPERTY_NB,
 };
 
-#define SANDBOX_TPM_PCR_NB 1
+#define SANDBOX_TPM_PCR_NB TPM2_MAX_PCRS
+#define SANDBOX_TPM_PCR_SELECT_MAX ((SANDBOX_TPM_PCR_NB + 7) / 8)
 
 /*
  * Information about our TPM emulation. This is preserved in the sandbox
@@ -433,7 +429,7 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const u8 
*sendbuf,
int i, j;
 
/* TPM2_GetProperty */
-   u32 capability, property, property_count;
+   u32 capability, property, property_count, val;
 
/* TPM2_PCR_Read/Extend variables */
int pcr_index = 0;
@@ -542,19 +538,32 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const 
u8 *sendbuf,
case TPM2_CC_GET_CAPABILITY:
capability = get_unaligned_be32(sent);
sent += sizeof(capability);
-   if (capability != TPM_CAP_TPM_PROPERTIES) {
-   printf("Sandbox TPM only support TPM_CAPABILITIES\n");
-   return TPM2_RC_HANDLE;
-   }
-
property = get_unaligned_be32(sent);
sent += sizeof(property);
-   property -= TPM2_PROPERTIES_OFFSET;
-
property_count = get_unaligned_be32(sent);
sent += sizeof(property_count);
-   if (!property_count ||
-   property + property_count > TPM2_PROPERTY_NB) {
+
+   switch (capability) {
+   case TPM2_CAP_PCRS:
+   break;
+   case TPM2_CAP_TPM_PROPERTIES:
+   if (!property_count) {
+   rc = TPM2_RC_HANDLE;
+   return sandbox_tpm2_fill_buf(recv, recv_len,
+tag, rc);
+   }
+
+   if (property >= TPM2_PROPERTIES_OFFSET &&
+   ((property - TPM2_PROPERTIES_OFFSET) +
+property_count > TPM2_PROPERTY_NB)) {
+   rc = TPM2_RC_HANDLE;
+   return sandbox_tpm2_fill_buf(recv, recv_len,
+tag, rc);
+   }
+   break;
+   default:
+   printf("Sandbox TPM2 only supports TPM2_CAP_PCRS or "
+  "TPM2_CAP_TPM_PROPERTIES\n");
rc = TPM2_RC_HANDLE;
return sandbox_tpm2_fill_buf(recv, recv_len, tag, rc);
}
@@ -578,18 +587,53 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const 
u8 *sendbuf,
put_unaligned_be32(capability, recv);
recv += sizeof(capability);
 
-   /* Give the number of properties that follow */
-   put_unaligned_be32(property_count, recv);
-   recv += sizeof(property_count);
-
-   /* Fill with the properties */
-   for (i = 0; i < property_count; i++) {
-   put_unaligned_be32(TPM2_PROPERTIES_OFFSET + property +
-  i, recv);
-   recv += sizeof(property);
-   put_unaligned_be32(tpm->properties[property + i],
-  recv);
-   recv += sizeof(property);
+   switch (capability) {
+   case TPM2_CAP_PCRS:
+   /* Give the number of algorithms supported - just 
SHA256 */
+   put_unaligned_be32(1, recv);
+   recv += sizeof(u32);
+
+   /* Give SHA256 algorithm */
+   put_unaligned_be16(TPM2_ALG_SHA256, recv);
+   recv += sizeof(u16);
+
+   /* Select the PCRs supported */
+   *recv