[PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs

[PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs

[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

Re: [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs

[Bruno E. O. Meneguele]

[-- 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 --]

Re: [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs

[Petr Vorel]

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);

Re: [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs

[Bruno E. O. Meneguele]

[-- 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 --]

Re: [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs

[Mimi Zohar]

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

Re: [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs

[Petr Vorel]

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

Exposing the tpm 2.0 PCRs? (renamed subject)

[Mimi Zohar]

[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

Re: Exposing the tpm 2.0 PCRs? (renamed subject)

[Jarkko Sakkinen]

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

Re: Exposing the tpm 2.0 PCRs? (renamed subject)

[Matthew Garrett]

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.

Re: Exposing the tpm 2.0 PCRs? (renamed subject)

[Jarkko Sakkinen]

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