Hi Mirian, On Mon, Jul 22, 2019 at 06:58:35PM +0300, Vitaly Chikunov wrote: > Mimi, > > On Mon, Jul 22, 2019 at 09:32:48AM -0400, Mimi Zohar wrote: > > The kernel does not expose the crypto agile TPM 2.0 PCR banks to > > userspace like it exposes PCRs for TPM 1.2. As a result, a userspace > > application is required to read PCRs. > > > > This patch adds tsspcrread support for reading the TPM 2.0 PCRs. > > tsspcrread is one application included in the ibmtss package. > > > > Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxx> > > --- > > configure.ac | 3 +++ > > src/Makefile.am | 3 +++ > > src/evmctl.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++----- > > 3 files changed, 54 insertions(+), 5 deletions(-) > > > > diff --git a/src/evmctl.c b/src/evmctl.c > > index 9e0926f10404..cbb9397be138 100644 > > --- a/src/evmctl.c > > +++ b/src/evmctl.c > > @@ -1402,6 +1400,38 @@ static int tpm_pcr_read(int idx, uint8_t *pcr, int len) > > return result; > > } > > > > +#ifdef IBMTSS > > +static int tpm_pcr_read2(int idx, uint8_t *hwpcr, int len, char **errmsg) > > +{ > > + FILE *fp; > > + char pcr[100]; /* may contain an error */ > > + char cmd[36]; > > + int ret = 0; > > + int i; > > + > > + sprintf(cmd, "tsspcrread -halg sha1 -ha %d -ns", idx); > > + fp = popen(cmd, "r"); > > + if (!fp) > > + return -1; > > + > > + fgets(pcr, 100, fp); > > Should it be sizeof(pcr)? > > I don't know convention of `tsspcrread' but maybe fgets() return value > should be checked too in case of error of executing `tsspcrread' or > error inside of `tsspcrread' (like pcr read error). > > > + > > + /* pcr might contain an error message */ > > + for (i = 0; i < strlen(pcr) - 1 && !ret; i++) { > > + if (isspace(pcr[i])) > > + ret = -1; > > Probably `strlen(pcr)' should be without `- 1'. > > > + } > > + > > + if (!ret) > > + hex2bin(hwpcr, pcr, SHA_DIGEST_LENGTH); > > + else > > + *errmsg = pcr; > > pcr isn't static nor malloc'ed. > > > + > > + pclose(fp); > > + return ret; > > +} > > +#endif > > + > > #define TCG_EVENT_NAME_LEN_MAX 255 > > > > struct template_entry { > > @@ -1658,8 +1688,21 @@ static int ima_measurement(const char *file) > > log_info("PCRAgg %.2d: ", i); > > log_dump(pcr[i], SHA_DIGEST_LENGTH); > > > > - if (tpm_pcr_read(i, hwpcr, sizeof(hwpcr))) > > - exit(1); > > + if (tpm_pcr_read(i, hwpcr, sizeof(hwpcr))) { > > +#ifdef IBMTSS > > + char *errmsg = NULL; > > + > > + err = tpm_pcr_read2(i, hwpcr, sizeof(hwpcr), &errmsg); > > + if (err) { > > + log_info("Failed to read either TPM 1.2 or TPM 2.0 PCRs.\n\t %s", errmsg); > > + exit(-1); ^^^^^^^^ > > + } > > +#else > > + log_info("Failed to read TPM 1.2 PCRs.\n"); > > + exit(-1); ^^^^^^^^ > > +#endif > > + } > > + > > log_info("HW PCR-%d: ", i); > > log_dump(hwpcr, sizeof(hwpcr)); > > > > -- > > 2.7.5 Besides to what Vitaly have pointed... exit(1) has been the standard exit code in case of failure, wouldn't that be better to keep it instead of change it to -1? (points highlighted above)
Attachment:
signature.asc
Description: PGP signature