* [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs @ 2019-07-22 21:14 Mimi Zohar 2019-07-22 21:55 ` Bruno E. O. Meneguele 2019-07-23 7:15 ` Petr Vorel 0 siblings, 2 replies; 10+ messages in thread From: Mimi Zohar @ 2019-07-22 21:14 UTC (permalink / raw) To: linux-integrity Cc: Vitaly Chikunov, Petr Vorel, Bruno E . O . Meneguele, Dmitry Eremin-Solenikov, Mimi Zohar 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> --- Changelog v1: - Based on Vitaly's review, test the popen "cmd" return code. Use pclose to determine if the result of the popen command succeeded or failed. Removed the now unneeded checking for spaces. - Dynamically allocated the error msg and other changes based on Vitaly's review. - Based on Bruno's review, reverted the return code change. At some point, we'll need to re-visit the return codes in general. configure.ac | 3 +++ src/Makefile.am | 3 +++ src/evmctl.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 50 insertions(+), 4 deletions(-) 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..f726b2186678 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,32 @@ 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; + + sprintf(cmd, "tsspcrread -halg sha1 -ha %d -ns", idx); + fp = popen(cmd, "r"); + if (!fp) + return -1; + + fgets(pcr, sizeof(pcr), fp); + + /* get the popen "cmd" return code */ + ret = pclose(fp); + if (!ret) + hex2bin(hwpcr, pcr, SHA_DIGEST_LENGTH); + else + *errmsg = strdup(pcr); + + return ret; +} +#endif + #define TCG_EVENT_NAME_LEN_MAX 255 struct template_entry { @@ -1658,8 +1682,24 @@ 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))) + if (tpm_pcr_read(i, hwpcr, sizeof(hwpcr))) { +#ifdef IBMTSS + char *errmsg = NULL; + + err = tpm_pcr_read2(i, hwpcr, sizeof(hwpcr), &errmsg); + if (err) { + errmsg[strlen(errmsg) - 1] = 0; + + log_info("Failed to read either TPM 1.2 or TPM 2.0 PCRs.\n (%s)\n", errmsg); + free(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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs 2019-07-22 21:14 [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs Mimi Zohar @ 2019-07-22 21:55 ` Bruno E. O. Meneguele 2019-07-23 7:15 ` Petr Vorel 1 sibling, 0 replies; 10+ messages in thread From: Bruno E. O. Meneguele @ 2019-07-22 21:55 UTC (permalink / raw) To: Mimi Zohar Cc: linux-integrity, Vitaly Chikunov, Petr Vorel, Dmitry Eremin-Solenikov [-- Attachment #1: Type: text/plain, Size: 4703 bytes --] Hi Mirian, On Mon, Jul 22, 2019 at 05:14:40PM -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> > --- > Changelog v1: > - Based on Vitaly's review, test the popen "cmd" return code. Use > pclose to determine if the result of the popen command succeeded or > failed. Removed the now unneeded checking for spaces. > - Dynamically allocated the error msg and other changes based on > Vitaly's review. > - Based on Bruno's review, reverted the return code change. At some > point, we'll need to re-visit the return codes in general. > > configure.ac | 3 +++ > src/Makefile.am | 3 +++ > src/evmctl.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- > 3 files changed, 50 insertions(+), 4 deletions(-) > > 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..f726b2186678 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,32 @@ 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; > + > + sprintf(cmd, "tsspcrread -halg sha1 -ha %d -ns", idx); > + fp = popen(cmd, "r"); > + if (!fp) > + return -1; If popen() call fails, **errmsg won't be allocated and will crash in ima_measurement() when it make use of errmsg within if(err), see below. > + > + fgets(pcr, sizeof(pcr), fp); > + > + /* get the popen "cmd" return code */ > + ret = pclose(fp); > + if (!ret) > + hex2bin(hwpcr, pcr, SHA_DIGEST_LENGTH); > + else > + *errmsg = strdup(pcr); *errmsg is allocated only in case popen runs fine. > + > + return ret; > +} > +#endif > + > #define TCG_EVENT_NAME_LEN_MAX 255 > > struct template_entry { > @@ -1658,8 +1682,24 @@ 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))) > + if (tpm_pcr_read(i, hwpcr, sizeof(hwpcr))) { > +#ifdef IBMTSS > + char *errmsg = NULL; > + > + err = tpm_pcr_read2(i, hwpcr, sizeof(hwpcr), &errmsg); > + if (err) { > + errmsg[strlen(errmsg) - 1] = 0; crash here: errmsg wasn't initialized within tpm_pcr_read2(). > + > + log_info("Failed to read either TPM 1.2 or TPM 2.0 PCRs.\n (%s)\n", errmsg); > + free(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 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs 2019-07-22 21:14 [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs Mimi Zohar 2019-07-22 21:55 ` Bruno E. O. Meneguele @ 2019-07-23 7:15 ` Petr Vorel 2019-07-23 13:27 ` Bruno E. O. Meneguele 2019-07-23 15:14 ` Mimi Zohar 1 sibling, 2 replies; 10+ messages in thread From: Petr Vorel @ 2019-07-23 7:15 UTC (permalink / raw) To: Mimi Zohar Cc: linux-integrity, Vitaly Chikunov, Bruno E . O . Meneguele, Dmitry Eremin-Solenikov Hi Mimi, others commented C code, thus I'll comment autotools checks. > 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. OT: anyone aware why TPM 2.0 does not expose PCR banks to userspace via sysfs? > 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> Reviewed-by: Petr Vorel <pvorel@suse.cz> > --- > Changelog v1: > - Based on Vitaly's review, test the popen "cmd" return code. Use > pclose to determine if the result of the popen command succeeded or > failed. Removed the now unneeded checking for spaces. > - Dynamically allocated the error msg and other changes based on > Vitaly's review. > - Based on Bruno's review, reverted the return code change. At some > point, we'll need to re-visit the return codes in general. > configure.ac | 3 +++ > src/Makefile.am | 3 +++ > src/evmctl.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- > 3 files changed, 50 insertions(+), 4 deletions(-) > 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]) On some distros (at least openSUSE [1] and Debian [2], the library is called libtss.so (which distro is calling it libibmtss.so?) therefore this should work for both: AC_SEARCH_LIBS(TSS_Transmit, [ibmtss tss], [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 You can also use definition from config.h instead of passing it. AC_SEARCH_LIBS(TSS_Transmit, [ibmtss tss], [have_ibmtss=yes AC_DEFINE(HAVE_IBMTSS, 1, [Define to 1 if you have libibmtss installed])], [have_ibmtss=no]) Then you don't need to pass -DIBMTSS, use HAVE_IBMTSS from config.h instead. > AM_CPPFLAGS = -I$(top_srcdir) -include config.h > diff --git a/src/evmctl.c b/src/evmctl.c > index 9e0926f10404..f726b2186678 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,32 @@ 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; > + > + sprintf(cmd, "tsspcrread -halg sha1 -ha %d -ns", idx); Did I get it right, that in the end we don't use libibmtss, but tsspcrread. So wouldn't be safer to detect it with AC_CHECK_PROGS macro? See proposed diff. > + fp = popen(cmd, "r"); > + if (!fp) > + return -1; > + > + fgets(pcr, sizeof(pcr), fp); > + > + /* get the popen "cmd" return code */ > + ret = pclose(fp); > + if (!ret) > + hex2bin(hwpcr, pcr, SHA_DIGEST_LENGTH); > + else > + *errmsg = strdup(pcr); > + > + return ret; > +} > +#endif > + > #define TCG_EVENT_NAME_LEN_MAX 255 > struct template_entry { > @@ -1658,8 +1682,24 @@ 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))) > + if (tpm_pcr_read(i, hwpcr, sizeof(hwpcr))) { > +#ifdef IBMTSS > + char *errmsg = NULL; > + > + err = tpm_pcr_read2(i, hwpcr, sizeof(hwpcr), &errmsg); > + if (err) { > + errmsg[strlen(errmsg) - 1] = 0; > + > + log_info("Failed to read either TPM 1.2 or TPM 2.0 PCRs.\n (%s)\n", errmsg); > + free(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)); [1] https://packages.debian.org/sid/amd64/libtss0/filelist [2] https://build.opensuse.org/package/view_file/security/tss2/tss2.spec?expand=1 Kind regards, Petr Proposed diff: diff --git configure.ac configure.ac index 40fea93..09b111c 100644 --- configure.ac +++ configure.ac @@ -28,8 +28,11 @@ 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_PROG(TSSPCRREAD, [tsspcrread], yes, no) +if test "x$TSSPCRREAD" = "xyes"; then + AC_DEFINE(HAVE_TSSPCRREAD, 1, [Define to 1 if you have tsspcrread binary installed])], +fi 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.])]) @@ -73,5 +76,5 @@ echo echo "Configuration:" echo " debug: $pkg_cv_enable_debug" echo " openssl-conf: $enable_openssl_conf" -echo " ibmtss: $have_ibmtss" +echo " tsspcrread: $TSSPCRREAD" echo diff --git src/Makefile.am src/Makefile.am index f0990fb..9c037e2 100644 --- src/Makefile.am +++ src/Makefile.am @@ -21,9 +21,6 @@ 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 src/evmctl.c src/evmctl.c index f726b21..f5268ac 100644 --- src/evmctl.c +++ src/evmctl.c @@ -1400,7 +1400,7 @@ static int tpm_pcr_read(int idx, uint8_t *pcr, int len) return result; } -#ifdef IBMTSS +#ifdef HAVE_TSSPCRREAD static int tpm_pcr_read2(int idx, uint8_t *hwpcr, int len, char **errmsg) { FILE *fp; @@ -1683,7 +1683,7 @@ static int ima_measurement(const char *file) log_dump(pcr[i], SHA_DIGEST_LENGTH); if (tpm_pcr_read(i, hwpcr, sizeof(hwpcr))) { -#ifdef IBMTSS +#ifdef HAVE_TSSPCRREAD char *errmsg = NULL; err = tpm_pcr_read2(i, hwpcr, sizeof(hwpcr), &errmsg); ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs 2019-07-23 7:15 ` Petr Vorel @ 2019-07-23 13:27 ` Bruno E. O. Meneguele 2019-07-23 15:14 ` Mimi Zohar 1 sibling, 0 replies; 10+ messages in thread From: Bruno E. O. Meneguele @ 2019-07-23 13:27 UTC (permalink / raw) To: Petr Vorel Cc: Mimi Zohar, linux-integrity, Vitaly Chikunov, Dmitry Eremin-Solenikov [-- Attachment #1: Type: text/plain, Size: 8345 bytes --] On Tue, Jul 23, 2019 at 09:15:45AM +0200, Petr Vorel wrote: > Hi Mimi, > > others commented C code, thus I'll comment autotools checks. > > > 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. > OT: anyone aware why TPM 2.0 does not expose PCR banks to userspace via sysfs? > > > 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> > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > > --- > > Changelog v1: > > - Based on Vitaly's review, test the popen "cmd" return code. Use > > pclose to determine if the result of the popen command succeeded or > > failed. Removed the now unneeded checking for spaces. > > - Dynamically allocated the error msg and other changes based on > > Vitaly's review. > > - Based on Bruno's review, reverted the return code change. At some > > point, we'll need to re-visit the return codes in general. > > > configure.ac | 3 +++ > > src/Makefile.am | 3 +++ > > src/evmctl.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- > > 3 files changed, 50 insertions(+), 4 deletions(-) > > > 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]) > On some distros (at least openSUSE [1] and Debian [2], the library is called libtss.so > (which distro is calling it libibmtss.so?) therefore this should work for both: > AC_SEARCH_LIBS(TSS_Transmit, [ibmtss tss], [have_ibmtss=yes], [have_ibmtss=no]) > Fedora's ibmtss package [1] uses it as libibmtss, while libtss-* are libs provided by tpm2-tss package [2]. [1] https://src.fedoraproject.org/rpms/tss2/blob/master/f/tss2.spec#_85 [2] https://src.fedoraproject.org/rpms/tpm2-tss/blob/master/f/tpm2-tss.spec#_67 > > +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 > You can also use definition from config.h instead of passing it. > > AC_SEARCH_LIBS(TSS_Transmit, [ibmtss tss], > [have_ibmtss=yes > AC_DEFINE(HAVE_IBMTSS, 1, [Define to 1 if you have libibmtss installed])], > [have_ibmtss=no]) > > Then you don't need to pass -DIBMTSS, use HAVE_IBMTSS from config.h instead. > > > AM_CPPFLAGS = -I$(top_srcdir) -include config.h > > > diff --git a/src/evmctl.c b/src/evmctl.c > > index 9e0926f10404..f726b2186678 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,32 @@ 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; > > + > > + sprintf(cmd, "tsspcrread -halg sha1 -ha %d -ns", idx); > > Did I get it right, that in the end we don't use libibmtss, but tsspcrread. > So wouldn't be safer to detect it with AC_CHECK_PROGS macro? > See proposed diff. > +1 > > + fp = popen(cmd, "r"); > > + if (!fp) > > + return -1; > > + > > + fgets(pcr, sizeof(pcr), fp); > > + > > + /* get the popen "cmd" return code */ > > + ret = pclose(fp); > > + if (!ret) > > + hex2bin(hwpcr, pcr, SHA_DIGEST_LENGTH); > > + else > > + *errmsg = strdup(pcr); > > + > > + return ret; > > +} > > +#endif > > + > > #define TCG_EVENT_NAME_LEN_MAX 255 > > > struct template_entry { > > @@ -1658,8 +1682,24 @@ 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))) > > + if (tpm_pcr_read(i, hwpcr, sizeof(hwpcr))) { > > +#ifdef IBMTSS > > + char *errmsg = NULL; > > + > > + err = tpm_pcr_read2(i, hwpcr, sizeof(hwpcr), &errmsg); > > + if (err) { > > + errmsg[strlen(errmsg) - 1] = 0; > > + > > + log_info("Failed to read either TPM 1.2 or TPM 2.0 PCRs.\n (%s)\n", errmsg); > > + free(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)); > > [1] https://packages.debian.org/sid/amd64/libtss0/filelist > [2] https://build.opensuse.org/package/view_file/security/tss2/tss2.spec?expand=1 > > > Kind regards, > Petr > > Proposed diff: > > diff --git configure.ac configure.ac > index 40fea93..09b111c 100644 > --- configure.ac > +++ configure.ac > @@ -28,8 +28,11 @@ 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_PROG(TSSPCRREAD, [tsspcrread], yes, no) > +if test "x$TSSPCRREAD" = "xyes"; then > + AC_DEFINE(HAVE_TSSPCRREAD, 1, [Define to 1 if you have tsspcrread binary installed])], > +fi > > 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.])]) > @@ -73,5 +76,5 @@ echo > echo "Configuration:" > echo " debug: $pkg_cv_enable_debug" > echo " openssl-conf: $enable_openssl_conf" > -echo " ibmtss: $have_ibmtss" > +echo " tsspcrread: $TSSPCRREAD" > echo > diff --git src/Makefile.am src/Makefile.am > index f0990fb..9c037e2 100644 > --- src/Makefile.am > +++ src/Makefile.am > @@ -21,9 +21,6 @@ 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 src/evmctl.c src/evmctl.c > index f726b21..f5268ac 100644 > --- src/evmctl.c > +++ src/evmctl.c > @@ -1400,7 +1400,7 @@ static int tpm_pcr_read(int idx, uint8_t *pcr, int len) > return result; > } > > -#ifdef IBMTSS > +#ifdef HAVE_TSSPCRREAD > static int tpm_pcr_read2(int idx, uint8_t *hwpcr, int len, char **errmsg) > { > FILE *fp; > @@ -1683,7 +1683,7 @@ static int ima_measurement(const char *file) > log_dump(pcr[i], SHA_DIGEST_LENGTH); > > if (tpm_pcr_read(i, hwpcr, sizeof(hwpcr))) { > -#ifdef IBMTSS > +#ifdef HAVE_TSSPCRREAD > char *errmsg = NULL; > > err = tpm_pcr_read2(i, hwpcr, sizeof(hwpcr), &errmsg); [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs 2019-07-23 7:15 ` Petr Vorel 2019-07-23 13:27 ` Bruno E. O. Meneguele @ 2019-07-23 15:14 ` Mimi Zohar 2019-07-24 7:24 ` Petr Vorel 1 sibling, 1 reply; 10+ messages in thread From: Mimi Zohar @ 2019-07-23 15:14 UTC (permalink / raw) To: Petr Vorel Cc: linux-integrity, Vitaly Chikunov, Bruno E . O . Meneguele, Dmitry Eremin-Solenikov On Tue, 2019-07-23 at 09:15 +0200, Petr Vorel wrote: > Hi Mimi, > > others commented C code, thus I'll comment autotools checks. Perfect > > > 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. > OT: anyone aware why TPM 2.0 does not expose PCR banks to userspace via sysfs? TPM 2.0 support is slowly being upstreamed in stages. Initially the TPM 2.0 event log was not exported. Assuming that support for exposing the TPM 2.0 PCRs is upstreamed, it doesn't necessarily guarantee that it will be backported to stable. > > 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 > You can also use definition from config.h instead of passing it. > > AC_SEARCH_LIBS(TSS_Transmit, [ibmtss tss], > [have_ibmtss=yes > AC_DEFINE(HAVE_IBMTSS, 1, [Define to 1 if you have libibmtss installed])], > [have_ibmtss=no]) > > Then you don't need to pass -DIBMTSS, use HAVE_IBMTSS from config.h instead. Much better ... > > > AM_CPPFLAGS = -I$(top_srcdir) -include config.h > > > diff --git a/src/evmctl.c b/src/evmctl.c > > index 9e0926f10404..f726b2186678 100644 > > --- a/src/evmctl.c > > +++ b/src/evmctl.c > > @@ -1402,6 +1400,32 @@ 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; > > + > > + sprintf(cmd, "tsspcrread -halg sha1 -ha %d -ns", idx); > > Did I get it right, that in the end we don't use libibmtss, but tsspcrread. > So wouldn't be safer to detect it with AC_CHECK_PROGS macro? > See proposed diff. Yes, thank you! I've included it in the next version. thanks! Mimi ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs 2019-07-23 15:14 ` Mimi Zohar @ 2019-07-24 7:24 ` Petr Vorel 2019-07-24 13:56 ` Exposing the tpm 2.0 PCRs? (renamed subject) Mimi Zohar 0 siblings, 1 reply; 10+ messages in thread From: Petr Vorel @ 2019-07-24 7:24 UTC (permalink / raw) To: Mimi Zohar Cc: linux-integrity, Vitaly Chikunov, Bruno E . O . Meneguele, Dmitry Eremin-Solenikov Hi Mimi, > > > 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. > > OT: anyone aware why TPM 2.0 does not expose PCR banks to userspace via sysfs? > TPM 2.0 support is slowly being upstreamed in stages. Initially the > TPM 2.0 event log was not exported. Assuming that support for > exposing the TPM 2.0 PCRs is upstreamed, it doesn't necessarily > guarantee that it will be backported to stable. Thanks for info. I'm glad it's being addressed :). IMHO it'd be backporting it (once upstreamed), let's see. > > Did I get it right, that in the end we don't use libibmtss, but tsspcrread. > > So wouldn't be safer to detect it with AC_CHECK_PROGS macro? > > See proposed diff. > Yes, thank you! I've included it in the next version. You're welcome. > thanks! > Mimi Kind regards, Petr ^ permalink raw reply [flat|nested] 10+ messages in thread
* Exposing the tpm 2.0 PCRs? (renamed subject) 2019-07-24 7:24 ` Petr Vorel @ 2019-07-24 13:56 ` Mimi Zohar 2019-08-01 17:34 ` Jarkko Sakkinen 0 siblings, 1 reply; 10+ messages in thread From: Mimi Zohar @ 2019-07-24 13:56 UTC (permalink / raw) To: Petr Vorel, Jarkko Sakkinen Cc: linux-integrity, Vitaly Chikunov, Bruno E . O . Meneguele, Dmitry Eremin-Solenikov [Cc'ing Jarkko] On Wed, 2019-07-24 at 09:24 +0200, Petr Vorel 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. > > > OT: anyone aware why TPM 2.0 does not expose PCR banks to userspace via sysfs? > > > TPM 2.0 support is slowly being upstreamed in stages. Initially the > > TPM 2.0 event log was not exported. Assuming that support for > > exposing the TPM 2.0 PCRs is upstreamed, it doesn't necessarily > > guarantee that it will be backported to stable. > Thanks for info. I'm glad it's being addressed :). > IMHO it'd be backporting it (once upstreamed), let's see. Clarification, I'm not working on exposing the TPM 2.0 PCRs. Too much else to do. Jarkko, in case you missed this thread, is there any reason why the TPM 2.0 PCRs can not be exposed to userspace? thanks, Mimi ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Exposing the tpm 2.0 PCRs? (renamed subject) 2019-07-24 13:56 ` Exposing the tpm 2.0 PCRs? (renamed subject) Mimi Zohar @ 2019-08-01 17:34 ` Jarkko Sakkinen 2019-08-01 19:58 ` Matthew Garrett 0 siblings, 1 reply; 10+ messages in thread From: Jarkko Sakkinen @ 2019-08-01 17:34 UTC (permalink / raw) To: Mimi Zohar Cc: Petr Vorel, linux-integrity, Vitaly Chikunov, Bruno E . O . Meneguele, Dmitry Eremin-Solenikov On Wed, Jul 24, 2019 at 09:56:41AM -0400, Mimi Zohar wrote: > [Cc'ing Jarkko] > > On Wed, 2019-07-24 at 09:24 +0200, Petr Vorel 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. > > > > OT: anyone aware why TPM 2.0 does not expose PCR banks to userspace via sysfs? > > > > > TPM 2.0 support is slowly being upstreamed in stages. Initially the > > > TPM 2.0 event log was not exported. Assuming that support for > > > exposing the TPM 2.0 PCRs is upstreamed, it doesn't necessarily > > > guarantee that it will be backported to stable. > > > Thanks for info. I'm glad it's being addressed :). > > IMHO it'd be backporting it (once upstreamed), let's see. > > Clarification, I'm not working on exposing the TPM 2.0 PCRs. Too much > else to do. > > Jarkko, in case you missed this thread, is there any reason why the > TPM 2.0 PCRs can not be exposed to userspace? What is meant by TPM 2.0 PCRs support? /Jarkko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Exposing the tpm 2.0 PCRs? (renamed subject) 2019-08-01 17:34 ` Jarkko Sakkinen @ 2019-08-01 19:58 ` Matthew Garrett 2019-08-02 19:36 ` Jarkko Sakkinen 0 siblings, 1 reply; 10+ messages in thread From: Matthew Garrett @ 2019-08-01 19:58 UTC (permalink / raw) To: Jarkko Sakkinen Cc: Mimi Zohar, Petr Vorel, linux-integrity, Vitaly Chikunov, Bruno E . O . Meneguele, Dmitry Eremin-Solenikov On Thu, Aug 1, 2019 at 10:35 AM Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > What is meant by TPM 2.0 PCRs support? You can read TPM 1.2 PCR values directly through sysfs. There's no equivalent mechanism for obtaining the TPM 2 PCR banks - you have to talk to the TPM yourself. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Exposing the tpm 2.0 PCRs? (renamed subject) 2019-08-01 19:58 ` Matthew Garrett @ 2019-08-02 19:36 ` Jarkko Sakkinen 0 siblings, 0 replies; 10+ messages in thread From: Jarkko Sakkinen @ 2019-08-02 19:36 UTC (permalink / raw) To: Matthew Garrett Cc: Mimi Zohar, Petr Vorel, linux-integrity, Vitaly Chikunov, Bruno E . O . Meneguele, Dmitry Eremin-Solenikov On Thu, Aug 01, 2019 at 12:58:44PM -0700, Matthew Garrett wrote: > On Thu, Aug 1, 2019 at 10:35 AM Jarkko Sakkinen > <jarkko.sakkinen@linux.intel.com> wrote: > > What is meant by TPM 2.0 PCRs support? > > You can read TPM 1.2 PCR values directly through sysfs. There's no > equivalent mechanism for obtaining the TPM 2 PCR banks - you have to > talk to the TPM yourself. OK, when I did the TPM 2.0 support originally the conclusion was then that since you can easily get what you need through /dev/tpm0, we don't need such sysfs file. That does mean that it cannot be added if there are legit reasons to do so... /Jarkko ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-08-02 19:36 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-22 21:14 [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs Mimi Zohar 2019-07-22 21:55 ` Bruno E. O. Meneguele 2019-07-23 7:15 ` Petr Vorel 2019-07-23 13:27 ` Bruno E. O. Meneguele 2019-07-23 15:14 ` Mimi Zohar 2019-07-24 7:24 ` Petr Vorel 2019-07-24 13:56 ` Exposing the tpm 2.0 PCRs? (renamed subject) Mimi Zohar 2019-08-01 17:34 ` Jarkko Sakkinen 2019-08-01 19:58 ` Matthew Garrett 2019-08-02 19:36 ` Jarkko Sakkinen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.