| Message ID | 20210705154849.2083972-10-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | tpm: Enhance sandbox tpm2 emulation | expand |
On Mon, Jul 05, 2021 at 09:48:49AM -0600, Simon Glass wrote: > It is fairly easy to handle this case and it makes the emulator more > useful, since PCRs are commonly extended several times. > > Add support for this, using U-Boot's sha256 support. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > drivers/tpm/tpm2_tis_sandbox.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c > index ed9c9a0bc9f..e84bcc7c4c8 100644 > --- a/drivers/tpm/tpm2_tis_sandbox.c > +++ b/drivers/tpm/tpm2_tis_sandbox.c > @@ -11,6 +11,7 @@ > #include <asm/unaligned.h> > #include <linux/bitops.h> > #include <u-boot/crc.h> > +#include <u-boot/sha256.h> > #include "sandbox_common.h" > > /* Hierarchies */ > @@ -407,15 +408,19 @@ static int sandbox_tpm2_extend(struct udevice *dev, int pcr_index, > const u8 *extension) > { > struct sandbox_tpm2 *tpm = dev_get_priv(dev); > - int i; > > - /* Only simulate the first extensions from all '0' with only '0' */ > - for (i = 0; i < TPM2_DIGEST_LEN; i++) > - if (tpm->pcr[pcr_index][i] || extension[i]) > - return TPM2_RC_FAILURE; > + if (!pcr_index) { > + memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr, > + TPM2_DIGEST_LEN); Why do we have to treat pcr 0 specially? Can't we just get rid of sandbox_extended_once_pcr and just extend all the PCRs with the value wee get? > + } else { > + sha256_context ctx; > + > + sha256_starts(&ctx); > + sha256_update(&ctx, tpm->pcr[pcr_index], TPM2_DIGEST_LEN); > + sha256_update(&ctx, extension, TPM2_DIGEST_LEN); > + sha256_finish(&ctx, tpm->pcr[pcr_index]); > + } > > - memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr, > - TPM2_DIGEST_LEN); > tpm->pcr_extensions[pcr_index]++; > > return 0; > -- > 2.32.0.93.g670b81a890-goog >
On Thu, 15 Jul 2021 at 22:04, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > On Mon, Jul 05, 2021 at 09:48:49AM -0600, Simon Glass wrote: > > It is fairly easy to handle this case and it makes the emulator more > > useful, since PCRs are commonly extended several times. > > > > Add support for this, using U-Boot's sha256 support. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > drivers/tpm/tpm2_tis_sandbox.c | 19 ++++++++++++------- > > 1 file changed, 12 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c > > index ed9c9a0bc9f..e84bcc7c4c8 100644 > > --- a/drivers/tpm/tpm2_tis_sandbox.c > > +++ b/drivers/tpm/tpm2_tis_sandbox.c > > @@ -11,6 +11,7 @@ > > #include <asm/unaligned.h> > > #include <linux/bitops.h> > > #include <u-boot/crc.h> > > +#include <u-boot/sha256.h> > > #include "sandbox_common.h" > > > > /* Hierarchies */ > > @@ -407,15 +408,19 @@ static int sandbox_tpm2_extend(struct udevice *dev, int pcr_index, > > const u8 *extension) > > { > > struct sandbox_tpm2 *tpm = dev_get_priv(dev); > > - int i; > > > > - /* Only simulate the first extensions from all '0' with only '0' */ > > - for (i = 0; i < TPM2_DIGEST_LEN; i++) > > - if (tpm->pcr[pcr_index][i] || extension[i]) > > - return TPM2_RC_FAILURE; > > + if (!pcr_index) { > > + memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr, > > + TPM2_DIGEST_LEN); > > Why do we have to treat pcr 0 specially? > Can't we just get rid of sandbox_extended_once_pcr and just extend all the > PCRs with the value wee get? > Also looking at the entire series, SANDBOX_TPM_PCR_NB doesn't seem to change. I don't know too much about the tpm sandbox, but that effectively limits us to a single PCR? > > + } else { > > + sha256_context ctx; > > + > > + sha256_starts(&ctx); > > + sha256_update(&ctx, tpm->pcr[pcr_index], TPM2_DIGEST_LEN); > > + sha256_update(&ctx, extension, TPM2_DIGEST_LEN); > > + sha256_finish(&ctx, tpm->pcr[pcr_index]); > > + } > > > > - memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr, > > - TPM2_DIGEST_LEN); > > tpm->pcr_extensions[pcr_index]++; > > > > return 0; > > -- > > 2.32.0.93.g670b81a890-goog > >
Hi Ilias, On Thu, 15 Jul 2021 at 13:21, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > On Thu, 15 Jul 2021 at 22:04, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > On Mon, Jul 05, 2021 at 09:48:49AM -0600, Simon Glass wrote: > > > It is fairly easy to handle this case and it makes the emulator more > > > useful, since PCRs are commonly extended several times. > > > > > > Add support for this, using U-Boot's sha256 support. > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > --- > > > > > > drivers/tpm/tpm2_tis_sandbox.c | 19 ++++++++++++------- > > > 1 file changed, 12 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c > > > index ed9c9a0bc9f..e84bcc7c4c8 100644 > > > --- a/drivers/tpm/tpm2_tis_sandbox.c > > > +++ b/drivers/tpm/tpm2_tis_sandbox.c > > > @@ -11,6 +11,7 @@ > > > #include <asm/unaligned.h> > > > #include <linux/bitops.h> > > > #include <u-boot/crc.h> > > > +#include <u-boot/sha256.h> > > > #include "sandbox_common.h" > > > > > > /* Hierarchies */ > > > @@ -407,15 +408,19 @@ static int sandbox_tpm2_extend(struct udevice *dev, int pcr_index, > > > const u8 *extension) > > > { > > > struct sandbox_tpm2 *tpm = dev_get_priv(dev); > > > - int i; > > > > > > - /* Only simulate the first extensions from all '0' with only '0' */ > > > - for (i = 0; i < TPM2_DIGEST_LEN; i++) > > > - if (tpm->pcr[pcr_index][i] || extension[i]) > > > - return TPM2_RC_FAILURE; > > > + if (!pcr_index) { > > > + memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr, > > > + TPM2_DIGEST_LEN); > > > > Why do we have to treat pcr 0 specially? > > Can't we just get rid of sandbox_extended_once_pcr and just extend all the > > PCRs with the value wee get? > > > > Also looking at the entire series, SANDBOX_TPM_PCR_NB doesn't seem to change. > I don't know too much about the tpm sandbox, but that effectively > limits us to a single PCR? I did send an updated series. That's right, only one PCR is currently used by the tests. Regards, Simon
diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c index ed9c9a0bc9f..e84bcc7c4c8 100644 --- a/drivers/tpm/tpm2_tis_sandbox.c +++ b/drivers/tpm/tpm2_tis_sandbox.c @@ -11,6 +11,7 @@ #include <asm/unaligned.h> #include <linux/bitops.h> #include <u-boot/crc.h> +#include <u-boot/sha256.h> #include "sandbox_common.h" /* Hierarchies */ @@ -407,15 +408,19 @@ static int sandbox_tpm2_extend(struct udevice *dev, int pcr_index, const u8 *extension) { struct sandbox_tpm2 *tpm = dev_get_priv(dev); - int i; - /* Only simulate the first extensions from all '0' with only '0' */ - for (i = 0; i < TPM2_DIGEST_LEN; i++) - if (tpm->pcr[pcr_index][i] || extension[i]) - return TPM2_RC_FAILURE; + if (!pcr_index) { + memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr, + TPM2_DIGEST_LEN); + } else { + sha256_context ctx; + + sha256_starts(&ctx); + sha256_update(&ctx, tpm->pcr[pcr_index], TPM2_DIGEST_LEN); + sha256_update(&ctx, extension, TPM2_DIGEST_LEN); + sha256_finish(&ctx, tpm->pcr[pcr_index]); + } - memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr, - TPM2_DIGEST_LEN); tpm->pcr_extensions[pcr_index]++; return 0;
It is fairly easy to handle this case and it makes the emulator more useful, since PCRs are commonly extended several times. Add support for this, using U-Boot's sha256 support. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/tpm/tpm2_tis_sandbox.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)