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

From: Mimi Zohar <zohar@linux.ibm.com>
To: Petr Vorel <pvorel@suse.cz>
Cc: linux-integrity@vger.kernel.org,
	Vitaly Chikunov <vt@altlinux.org>,
	"Bruno E . O . Meneguele" <bmeneg@redhat.com>,
	Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Subject: Re: [PATCH v1] ima-evm-utils: use tsspcrread to read the TPM 2.0 PCRs
Date: Tue, 23 Jul 2019 11:14:36 -0400	[thread overview]
Message-ID: <1563894876.14396.120.camel@linux.ibm.com> (raw)
In-Reply-To: <20190723071545.GA26973@x230>

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