| Message ID | 1563802368-8460-1-git-send-email-zohar@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs | expand |
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@linux.ibm.com> > --- > 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
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@linux.ibm.com> > > --- > > 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)
Vitaly, Bruno, On Mon, 2019-07-22 at 15:52 -0300, Bruno E. O. Meneguele wrote: > 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@linux.ibm.com> Thank you for reviewing this patch so quickly! Your comments are much appreciated and will be addressed in the version. Mimi
diff --git a/configure.ac b/configure.ac index 9beb4b6c2377..40fea93fef2f 100644 --- a/configure.ac +++ b/configure.ac @@ -28,6 +28,8 @@ PKG_CHECK_MODULES(LIBCRYPTO, [libcrypto >= 0.9.8 ]) AC_SUBST(KERNEL_HEADERS) AC_CHECK_HEADER(unistd.h) AC_CHECK_HEADERS(openssl/conf.h) +AC_SEARCH_LIBS(TSS_Transmit, ibmtss, [have_ibmtss=yes], [have_ibmtss=no]) +AM_CONDITIONAL([CONFIG_IBMTSS], [test "x$have_ibmtss" = "xyes"]) AC_CHECK_HEADERS(sys/xattr.h, , [AC_MSG_ERROR([sys/xattr.h header not found. You need the c-library development package.])]) AC_CHECK_HEADERS(keyutils.h, , [AC_MSG_ERROR([keyutils.h header not found. You need the libkeyutils development package.])]) @@ -71,4 +73,5 @@ echo echo "Configuration:" echo " debug: $pkg_cv_enable_debug" echo " openssl-conf: $enable_openssl_conf" +echo " ibmtss: $have_ibmtss" echo diff --git a/src/Makefile.am b/src/Makefile.am index 9c037e21dc4f..f0990fb01210 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -21,6 +21,9 @@ evmctl_SOURCES = evmctl.c evmctl_CPPFLAGS = $(AM_CPPFLAGS) $(LIBCRYPTO_CFLAGS) evmctl_LDFLAGS = $(LDFLAGS_READLINE) evmctl_LDADD = $(LIBCRYPTO_LIBS) -lkeyutils libimaevm.la +if CONFIG_IBMTSS +evmctl_CFLAGS = -DIBMTSS +endif AM_CPPFLAGS = -I$(top_srcdir) -include config.h diff --git a/src/evmctl.c b/src/evmctl.c index 9e0926f10404..cbb9397be138 100644 --- a/src/evmctl.c +++ b/src/evmctl.c @@ -1383,10 +1383,8 @@ static int tpm_pcr_read(int idx, uint8_t *pcr, int len) if (!fp) fp = fopen(misc_pcrs, "r"); - if (!fp) { - log_err("Unable to open %s or %s\n", pcrs, misc_pcrs); + if (!fp) return -1; - } for (;;) { p = fgets(buf, sizeof(buf), fp); @@ -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); + + /* pcr might contain an error message */ + for (i = 0; i < strlen(pcr) - 1 && !ret; i++) { + if (isspace(pcr[i])) + ret = -1; + } + + if (!ret) + hex2bin(hwpcr, pcr, SHA_DIGEST_LENGTH); + else + *errmsg = pcr; + + 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));
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@linux.ibm.com> --- configure.ac | 3 +++ src/Makefile.am | 3 +++ src/evmctl.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 54 insertions(+), 5 deletions(-)