| Message ID | 20230308212537.1725343-3-eajames@linux.ibm.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilias Apalodimas |
| Headers | show |
| Series | tpm: Support boot measurements | expand |
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 <eajames@linux.ibm.com> > Reviewed-by: Simon Glass <sjg@chromium.org> > Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > 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 0x0000020E > > @@ -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 = SANDBOX_TPM_PCR_SELECT_MAX; > + recv++; > + > + /* Activate all the PCR bits */ > + for (i = 0; i < SANDBOX_TPM_PCR_SELECT_MAX; ++i) { > + *recv = 0xff; > + recv++; > + } > + break; > + case TPM2_CAP_TPM_PROPERTIES: > + /* 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(property + i, recv); > + recv += sizeof(property); > + if (property >= TPM2_PROPERTIES_OFFSET) { > + val = tpm->properties[(property - > + TPM2_PROPERTIES_OFFSET) + i]; > + } else { > + switch (property) { > + case TPM2_PT_PCR_COUNT: > + val = SANDBOX_TPM_PCR_NB; > + break; > + default: > + val = 0xffffffff; > + break; > + } > + } > + > + put_unaligned_be32(val, recv); > + recv += sizeof(property); > + } > + break; > } > > /* Add trailing \0 */ > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index c5835e6ef6..605719d2b6 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -333,8 +333,6 @@ config EFI_TCG2_PROTOCOL > bool "EFI_TCG2_PROTOCOL support" > default y > depends on TPM_V2 > - # Sandbox TPM currently fails on GetCapabilities needed for TCG2 > - depends on !SANDBOX > select SHA1 > select SHA256 > select SHA384 > -- > 2.31.1 >
Hi Eddie On Thu, 16 Mar 2023 at 10:32, Ilias Apalodimas <ilias.apalodimas@linaro.org> 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 <eajames@linux.ibm.com> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > --- > > 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 0x0000020E > > > > @@ -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 = SANDBOX_TPM_PCR_SELECT_MAX; > > + recv++; > > + > > + /* Activate all the PCR bits */ > > + for (i = 0; i < SANDBOX_TPM_PCR_SELECT_MAX; ++i) { > > + *recv = 0xff; > > + recv++; > > + } > > + break; > > + case TPM2_CAP_TPM_PROPERTIES: > > + /* 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(property + i, recv); > > + recv += sizeof(property); > > + if (property >= TPM2_PROPERTIES_OFFSET) { > > + val = tpm->properties[(property - > > + TPM2_PROPERTIES_OFFSET) + i]; > > + } else { > > + switch (property) { > > + case TPM2_PT_PCR_COUNT: > > + val = SANDBOX_TPM_PCR_NB; > > + break; > > + default: > > + val = 0xffffffff; > > + break; > > + } > > + } > > + > > + put_unaligned_be32(val, recv); > > + recv += sizeof(property); > > + } > > + break; > > } > > > > /* Add trailing \0 */ > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index c5835e6ef6..605719d2b6 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -333,8 +333,6 @@ config EFI_TCG2_PROTOCOL > > bool "EFI_TCG2_PROTOCOL support" > > default y > > depends on TPM_V2 > > - # Sandbox TPM currently fails on GetCapabilities needed for TCG2 > > - depends on !SANDBOX > > select SHA1 > > select SHA256 > > select SHA384 > > -- > > 2.31.1 > > > >
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 0x0000020E @@ -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 = SANDBOX_TPM_PCR_SELECT_MAX; + recv++; + + /* Activate all the PCR bits */ + for (i = 0; i < SANDBOX_TPM_PCR_SELECT_MAX; ++i) { + *recv = 0xff; + recv++; + } + break; + case TPM2_CAP_TPM_PROPERTIES: + /* 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(property + i, recv); + recv += sizeof(property); + if (property >= TPM2_PROPERTIES_OFFSET) { + val = tpm->properties[(property - + TPM2_PROPERTIES_OFFSET) + i]; + } else { + switch (property) { + case TPM2_PT_PCR_COUNT: + val = SANDBOX_TPM_PCR_NB; + break; + default: + val = 0xffffffff; + break; + } + } + + put_unaligned_be32(val, recv); + recv += sizeof(property); + } + break; } /* Add trailing \0 */ diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index c5835e6ef6..605719d2b6 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -333,8 +333,6 @@ config EFI_TCG2_PROTOCOL bool "EFI_TCG2_PROTOCOL support" default y depends on TPM_V2 - # Sandbox TPM currently fails on GetCapabilities needed for TCG2 - depends on !SANDBOX select SHA1 select SHA256 select SHA384