On Tue, Jun 16, 2020 at 06:21:48PM -0700, Jerry Snitselaar wrote:
> On Mon Jun 15 20, Mimi Zohar wrote:
> > On Mon, 2020-06-15 at 16:41 -0300, Bruno Meneguele wrote:
> > > On Thu, May 28, 2020 at 06:05:27PM +0200, Petr Vorel wrote:
> > > > Hi Mimi,
> > > > ...
> > > > > > > With just this change, the ima_tpm.sh test is failing. I assume it is
> > > > > > > failing because it is reading the SHA1 TPM bank, not the SHA256 bank
> > > > > > > to calculate the boot_aggregate hash.
> > > > > > First question: is it correct to take sha256? Because on my test below it's
> > > > > > reading sha1, because that's the content of /sys/kernel/security/ima/ascii_runtime_measurements
> > > >
> > > > > > I thought just kernel commit: 6f1a1d103b48 ima: ("Switch to ima_hash_algo for
> > > > > > boot aggregate") from current linux-integrity tree is needed, but I tested it on
> > > > > > b59fda449cf0 ("ima: Set again build_ima_appraise variable") (i.e. having all
> > > > > > Robeto's ima patches, missing just last 2 commits from next-integrity head).
> > > > > > What is needed to get your setup?
> > > >
> > > > > This isn't a configuration problem, but an issue of reading PCRs and
> > > > > calculating the TPM bank appropriate boot_aggregate. If you're
> > > > > calculating a sha256 boot_aggregate, then the test needs to read and
> > > > > calculate the boot_aggregate by reading the SHA256 TPM bank.
> > > > OK, I tested it on TPM 1.2 (no TPM 2.0 available atm).
> > > > I guess you have TPM 2.0, that's why I didn't spot this issue.
> > > >
> > > > To sum that: my patch is required for any system without physical TPM with with
> > > > kernel with b59fda449cf0 + it also works for TPM 1.2 (regardless kernel
> > > > version), because TPM 1.2 supports sha1 only boot aggregate.
> > > >
> > > > But testing on kernel with b59fda449cf0 with TPM 2.0 is not only broken with
> > > > this patch, but also on current version in master, right? As you have
> > > > sha256:3fd5dc717f886ff7182526efc5edc3abb179a5aac1ab589c8ec888398233ae5 anyway.
> > > > So this patch would help at least testing on VM without vTPM.
> > > >
> > >
> > > If we consider to delay this change until we have the ima-evm-utils
> > > released with the ima_boot_aggregate + make this test dependent on
> > > both ima-evm-utils and tsspcrread, would it be worth to SKIP the test in
> > > case a TPM2.0 sha256 bank is detected instead of FAIL? Thus we could
> > > have the test fixed for TPM1.2 && no-TPM cases until we get the full
> > > support for multiple banks?
> >
> > As long as we're dealing with the "boot_aggregate", Maurizio just
> > posted a kernel patch for including PCR 8 & 9 in the boot_aggregate.
> > The existing IMA LTP "boot_aggregate" test is going to need to
> > support this change.
> >
> > I'd appreciate if someone could send me a TPM event log, the PCRs, and
> > the associated IMA ascii_runtime_measurements "boot_aggregate" from a
> > system with a discrete TPM 2.0 with PCRs 8 & 9 events.
> >
Maybe Maurizio already have it at hand?
I can try to setup a system with grub2+tpm to get the log with pcr 8 and
9 filled.
> > >
> > > > ...
> > > > > > > The ima-evm-utils next-testing branch has code to calculate the
> > > > > > > boot_aggregate based on multiple banks.
> > > > > > I see, 696bf0b ("ima-evm-utils: calculate the digests for multiple TPM banks")
> > > > > > I wonder whether it's reasonable trying to port that to ima_boot_aggregate.c or
> > > > > > just depend on evmctl. External dependencies are sometimes complicated, but for
> > > > > > IMA I incline to just require evmctl.
> > > >
> > > > > Unlike TPM 1.2, the TPM 2.0 device driver doesn't export the TPM PCRs.
> > > > > Not only would you have a dependency on ima-evm-utils, but also on a
> > > > > userspace application(s) for reading the TPM PCRs. That dependency
> > > > > exists whether you're using evmctl to calculate the boot_aggregate or
> > > > > doing it yourself.
> > > > Hm, things get complicated.
> > > > Yep I remember your patch to skip verifying TPM 2.0 PCR values
> > > > https://patchwork.ozlabs.org/project/ltp/patch/1558041162.3971.2.camel@xxxxxxxxxxxxx/
> > > > At least thanks to Jerry Snitselaar since v5.6 we have
> > > > /sys/class/tpm/tpm*/tpm_version_major. We could check this (+ try also
> > > > /sys/class/tpm/tpm0/device/description for older kernels).
> > > >
> > > > BTW on my system there is also /sys/class/tpm/tpm0/ppi/version, which has 1.2,
> > > > not sure if it indicate TPM 1.2, but I wouldn't rely on that.
> > > >
Missed this last paragraph.. but /sys/class/tpm/tpm0/ppi/version has
relation to the Physical Presence Interface version, which is the
communication interface between firmware and OS afaik, and doesn't
points to the TPM version: TPM2.0 may have PPI version 1.2 or 1.3.
> > >
> > > IIUC 'tpm_version_major' should be the only safe reference of the actual
> > > TCG spec version being implemented by the hw TPM, in a sysfs standard
> > > output.
> >
> > That was only upstreamed in linux-v5.6. Has it been backported?
> >
Hmm, well pointed.
>
> Not that I know of. Since it isn't using chip->ops and only
> looks at chip->flags it could probably be backported, but I'd
> have to take a look at it.
>
> > The PCRs are not exported for TPM 2.0, unfortunately, making
> > regression tests dependent on a userspace app. The existing LTP
> > ima_tpm.sh test looks for the PCRs in either
> > /sys/class/tpm/tpm0/device/pcrs or /sys/class/misc/tpm0/device/pcrs.
> > Perhaps piggyback on the pseudo PCR file to test for TPM 1.2.
> >
> > >
> > > > ...
> > > > > > > There's also a new test to verify the boot_aggregate.
> > > >
> > > > > > > $ VERBOSE=1 make check TESTS=boog_aggregate.test
> > > > > > BTW I got some errors
> > > > > > ...
> > > > > > make check-TESTS
> > > > > > make[2]: Entering directory '/home/foo/ima-evm-utils/tests'
> > > > > > make[3]: Entering directory '/home/foo/ima-evm-utils/tests'
> > > > > > make[4]: Entering directory '/home/foo/ima-evm-utils/tests'
> > > > > > make[4]: Nothing to be done for 'boog_aggregate.log'.
> > > > > > make[4]: Leaving directory '/home/foo/ima-evm-utils/tests'
> > > > > > fatal: making test-suite.log: failed to create boog_aggregate.trs
> > > > > > fatal: making test-suite.log: failed to create boog_aggregate.log
> > > > > > make[3]: *** [Makefile:516: test-suite.log] Error 1
> > > > > > make[3]: Leaving directory '/home/foo/ima-evm-utils/tests'
> > > > > > make[2]: *** [Makefile:625: check-TESTS] Error 2
> > > > > > make[2]: Leaving directory '/home/foo/ima-evm-utils/tests'
> > > > > > make[1]: *** [Makefile:692: check-am] Error 2
> > > > > > make[1]: Leaving directory '/home/foo/ima-evm-utils/tests'
> > > > > > make: *** [Makefile:514: check-recursive] Error 1
> > > >
> > > > > [Cc'ing Vitaly]
> > > >
> > > > > The boot_aggregate.trs and boot_aggregate.log files are being created
> > > > > in the tests/ directory. Is that directory read-only?
> > > > Yes, drwxr-xr-x. Testing on fresh clone and issue persists.
> > > >
> > >
> > > Yes, same thing here.. but didn't really check the reason for that. Will
> > > take a time later to see what's happening.
> >
> > Thanks, much appreciated. I'm not seeing that here.
> >
> > Mimi
> >
>
--
bmeneg
PGP Key: http://bmeneg.com/pubkey.txt
Attachment:
signature.asc
Description: PGP signature
