mbox series

[v5,0/4] TPM 2.0 fixes in IMA tests

Message ID 20201214221946.6340-1-pvorel@suse.cz (mailing list archive)
Headers show
Series TPM 2.0 fixes in IMA tests | expand

Message

Petr Vorel Dec. 14, 2020, 10:19 p.m. UTC 
Hi Mimi, Lakshmi, Tushar,

sending hopefully a final version. This version was done with big help
from Mimi. Mimi, thank you for your help with this!
I'd like to merge it this week and move on for your other IMA patches
(dm-crypt and SELinux).

Could you please test this, specially on TPM 2.0?
I tested it on tpm_tis MSFT0101:00: (Infineon 9665), which does not
export /sys/kernel/security/tpm0/binary_bios_measurements, but
reading PCR with tsspcrread works.

The only problem which bothers me is failure on ima_policy=tcb:

evmctl ima_measurement /sys/kernel/security/integrity/ima/binary_runtime_measurements -vv
...
sha256: PCRAgg  10: c19866f10132282d4cf20ca45f50078db843f95dc8d1ea8819d0e240cdf3b21c
sha256: TPM PCR-10: df913daa0437a2365f710f6d93a4f2d37146414425d9aaa60740dc635d187158
sha256: PCRAgg 10 does not match TPM PCR-10
Failed to match per TPM bank or SHA1 padded TPM digest(s) (count 1446)
errno: No such file or directory (2)

Thus test get failure for the fist run without --ignore-violations
...
ima_tpm 1 TINFO: using command: evmctl ima_boot_aggregate -v
Using tss2-rc-decode to read PCRs.
ima_tpm 1 TINFO: IMA boot aggregate: '0756853d9378ff6473966e20610a8d1cb97e4dc613cb87adf5e870c8eb93fd0f'
ima_tpm 1 TPASS: bios boot aggregate matches IMA boot aggregate
ima_tpm 2 TINFO: verify PCR values
ima_tpm 2 TINFO: real PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
ima_tpm 2 TFAIL: evmctl failed, trying with --ignore-violations
ima_tpm 2 TINFO: aggregate PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
ima_tpm 2 TPASS: aggregate PCR value matches real PCR value
ima_tpm 3 TINFO: AppArmor enabled, this may affect test results
ima_tpm 3 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
ima_tpm 3 TINFO: loaded AppArmor profiles: none

Summary:
passed   2
failed   1
skipped  0
warnings 0

IMHO unless this is specific for this particular TPM we should skip test
if ima_policy=tcb.

I tried LetsTrust TPM 2.0 for raspberry-pi (Infineon SLB9670, connected
over SPI), but that got even worse - TPM is registered after IMA, thus
unusable).

I'd also like you other IMA tests (dm-crypt and SELinux) before LTP release
(sometimes in January), but due summer vacation we have basically just
this week and maybe first week and maybe first week in January.

Changes v4->v5:
* improved TPM 2.0 detection (e.g. check for /dev/tpmrm0 and /dev/tpm0)
* test2: if evmctl ima_measurement fails, run again with --ignore-violations
* test2: assume TPM 2, if not detected
* print TPM kernel config
* cleanup

Kind regards,
Petr

Petr Vorel (4):
  IMA: Move get_algorithm_digest(), set_digest_index() to ima_setup.sh
  IMA: Rewrite ima_boot_aggregate.c to new API
  ima_tpm.sh: Fix calculating boot aggregate
  ima_tpm.sh: Fix calculating PCR aggregate

 .../integrity/ima/src/ima_boot_aggregate.c    | 114 +++---
 .../integrity/ima/tests/ima_measurements.sh   |  62 +---
 .../security/integrity/ima/tests/ima_setup.sh |  84 ++++-
 .../security/integrity/ima/tests/ima_tpm.sh   | 334 +++++++++++++++---
 4 files changed, 422 insertions(+), 172 deletions(-)

Comments

Mimi Zohar Dec. 17, 2020, 5:20 a.m. UTC | #1 
Hi Petr,

On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> The only problem which bothers me is failure on ima_policy=tcb:
> 
> evmctl ima_measurement /sys/kernel/security/integrity/ima/binary_runtime_measurements -vv
> ...
> sha256: PCRAgg  10: c19866f10132282d4cf20ca45f50078db843f95dc8d1ea8819d0e240cdf3b21c
> sha256: TPM PCR-10: df913daa0437a2365f710f6d93a4f2d37146414425d9aaa60740dc635d187158
> sha256: PCRAgg 10 does not match TPM PCR-10
> Failed to match per TPM bank or SHA1 padded TPM digest(s) (count 1446)
> errno: No such file or directory (2)
> 
> Thus test get failure for the fist run without --ignore-violations
> ...
> ima_tpm 1 TINFO: using command: evmctl ima_boot_aggregate -v
> Using tss2-rc-decode to read PCRs.
> ima_tpm 1 TINFO: IMA boot aggregate: '0756853d9378ff6473966e20610a8d1cb97e4dc613cb87adf5e870c8eb93fd0f'
> ima_tpm 1 TPASS: bios boot aggregate matches IMA boot aggregate
> ima_tpm 2 TINFO: verify PCR values
> ima_tpm 2 TINFO: real PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> ima_tpm 2 TFAIL: evmctl failed, trying with --ignore-violations
> ima_tpm 2 TINFO: aggregate PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> ima_tpm 2 TPASS: aggregate PCR value matches real PCR value
> ima_tpm 3 TINFO: AppArmor enabled, this may affect test results
> ima_tpm 3 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
> ima_tpm 3 TINFO: loaded AppArmor profiles: none
> 
> Summary:
> passed   2
> failed   1
> skipped  0
> warnings 0
> 
> IMHO unless this is specific for this particular TPM we should skip test
> if ima_policy=tcb.

No, I don't think so.  Violations are a result of a file being opened
for read and write at the same time.  Opening a file for write, when it
is already open for read, results in a Time of Measure/Time of Use
(ToMToU) violation.  Opening a file for read, when it is already open
for write, results in an open_writer violation.  One of the more common
reasons for these violations are log files.

With the builtin TCB measurement policy enabled on the boot command
line, files are measured from the beginning, before a custom policy is
loaded.  Normally a custom policy is loaded after an LSM policy has
been loaded, allowing IMA policy rules to be defined in terms of LSM
labels.

Verifying the IMA measurement list against the TPM PCRs is an important
test.  Ignoring violations doesn't make sense either.   Perhaps if a
custom policy has not been loaded, emit an informational message and
skip the test without "--ignore-violations".

thanks,

Mimi
Petr Vorel Dec. 17, 2020, 8:33 a.m. UTC | #2 
Hi Mimi,

> Hi Petr,

> On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> > The only problem which bothers me is failure on ima_policy=tcb:

> > evmctl ima_measurement /sys/kernel/security/integrity/ima/binary_runtime_measurements -vv
> > ...
> > sha256: PCRAgg  10: c19866f10132282d4cf20ca45f50078db843f95dc8d1ea8819d0e240cdf3b21c
> > sha256: TPM PCR-10: df913daa0437a2365f710f6d93a4f2d37146414425d9aaa60740dc635d187158
> > sha256: PCRAgg 10 does not match TPM PCR-10
> > Failed to match per TPM bank or SHA1 padded TPM digest(s) (count 1446)
> > errno: No such file or directory (2)

> > Thus test get failure for the fist run without --ignore-violations
> > ...
> > ima_tpm 1 TINFO: using command: evmctl ima_boot_aggregate -v
> > Using tss2-rc-decode to read PCRs.
> > ima_tpm 1 TINFO: IMA boot aggregate: '0756853d9378ff6473966e20610a8d1cb97e4dc613cb87adf5e870c8eb93fd0f'
> > ima_tpm 1 TPASS: bios boot aggregate matches IMA boot aggregate
> > ima_tpm 2 TINFO: verify PCR values
> > ima_tpm 2 TINFO: real PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> > ima_tpm 2 TFAIL: evmctl failed, trying with --ignore-violations
> > ima_tpm 2 TINFO: aggregate PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> > ima_tpm 2 TPASS: aggregate PCR value matches real PCR value
> > ima_tpm 3 TINFO: AppArmor enabled, this may affect test results
> > ima_tpm 3 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
> > ima_tpm 3 TINFO: loaded AppArmor profiles: none

> > Summary:
> > passed   2
> > failed   1
> > skipped  0
> > warnings 0

> > IMHO unless this is specific for this particular TPM we should skip test
> > if ima_policy=tcb.

> No, I don't think so.  Violations are a result of a file being opened
> for read and write at the same time.  Opening a file for write, when it
> is already open for read, results in a Time of Measure/Time of Use
> (ToMToU) violation.  Opening a file for read, when it is already open
> for write, results in an open_writer violation.  One of the more common
> reasons for these violations are log files.

> With the builtin TCB measurement policy enabled on the boot command
> line, files are measured from the beginning, before a custom policy is
> loaded.  Normally a custom policy is loaded after an LSM policy has
> been loaded, allowing IMA policy rules to be defined in terms of LSM
> labels.

> Verifying the IMA measurement list against the TPM PCRs is an important
> test.  Ignoring violations doesn't make sense either.   Perhaps if a
> custom policy has not been loaded, emit an informational message and
> skip the test without "--ignore-violations".

Thanks for an explanation. Agree, you're right. It's most likely wrong setup
(there were some temporary files in /tmp and even postfix pid file in /var/run/),
I need to properly setup dracut-ima. It'd be then good to document this, but I'd
do it as separate effort.

So, can I merge the patchset with your ack/review-by?

Kind regards,
Petr

> thanks,

> Mimi
Mimi Zohar Dec. 17, 2020, 7:23 p.m. UTC | #3 
On Thu, 2020-12-17 at 09:33 +0100, Petr Vorel wrote:
> Hi Mimi,
> 
> > Hi Petr,
> 
> > On Mon, 2020-12-14 at 23:19 +0100, Petr Vorel wrote:
> > > The only problem which bothers me is failure on ima_policy=tcb:
> 
> > > evmctl ima_measurement /sys/kernel/security/integrity/ima/binary_runtime_measurements -vv
> > > ...
> > > sha256: PCRAgg  10: c19866f10132282d4cf20ca45f50078db843f95dc8d1ea8819d0e240cdf3b21c
> > > sha256: TPM PCR-10: df913daa0437a2365f710f6d93a4f2d37146414425d9aaa60740dc635d187158
> > > sha256: PCRAgg 10 does not match TPM PCR-10
> > > Failed to match per TPM bank or SHA1 padded TPM digest(s) (count 1446)
> > > errno: No such file or directory (2)
> 
> > > Thus test get failure for the fist run without --ignore-violations
> > > ...
> > > ima_tpm 1 TINFO: using command: evmctl ima_boot_aggregate -v
> > > Using tss2-rc-decode to read PCRs.
> > > ima_tpm 1 TINFO: IMA boot aggregate: '0756853d9378ff6473966e20610a8d1cb97e4dc613cb87adf5e870c8eb93fd0f'
> > > ima_tpm 1 TPASS: bios boot aggregate matches IMA boot aggregate
> > > ima_tpm 2 TINFO: verify PCR values
> > > ima_tpm 2 TINFO: real PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> > > ima_tpm 2 TFAIL: evmctl failed, trying with --ignore-violations
> > > ima_tpm 2 TINFO: aggregate PCR-10: '6d8aec6291c0c19efdee50e20899939135be073cd4d6e9063e53386f54f9487d'
> > > ima_tpm 2 TPASS: aggregate PCR value matches real PCR value
> > > ima_tpm 3 TINFO: AppArmor enabled, this may affect test results
> > > ima_tpm 3 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
> > > ima_tpm 3 TINFO: loaded AppArmor profiles: none
> 
> > > Summary:
> > > passed   2
> > > failed   1
> > > skipped  0
> > > warnings 0
> 
> > > IMHO unless this is specific for this particular TPM we should skip test
> > > if ima_policy=tcb.
> 
> > No, I don't think so.  Violations are a result of a file being opened
> > for read and write at the same time.  Opening a file for write, when it
> > is already open for read, results in a Time of Measure/Time of Use
> > (ToMToU) violation.  Opening a file for read, when it is already open
> > for write, results in an open_writer violation.  One of the more common
> > reasons for these violations are log files.
> 
> > With the builtin TCB measurement policy enabled on the boot command
> > line, files are measured from the beginning, before a custom policy is
> > loaded.  Normally a custom policy is loaded after an LSM policy has
> > been loaded, allowing IMA policy rules to be defined in terms of LSM
> > labels.
> 
> > Verifying the IMA measurement list against the TPM PCRs is an important
> > test.  Ignoring violations doesn't make sense either.   Perhaps if a
> > custom policy has not been loaded, emit an informational message and
> > skip the test without "--ignore-violations".
> 
> Thanks for an explanation. Agree, you're right. It's most likely wrong setup
> (there were some temporary files in /tmp and even postfix pid file in /var/run/),
> I need to properly setup dracut-ima. It'd be then good to document this, but I'd
> do it as separate effort.
> 
> So, can I merge the patchset with your ack/review-by?

Yes, I just finished reviewing the patches.   Other that clarifying the
patch descriptions and fixing the one typo  ("tmp" -> "tpm"), it looks
really.

thanks! 

Mimi
Petr Vorel Dec. 18, 2020, 11:45 a.m. UTC | #4 
Hi Mimi,

> > So, can I merge the patchset with your ack/review-by?

> Yes, I just finished reviewing the patches.   Other that clarifying the
> patch descriptions and fixing the one typo  ("tmp" -> "tpm"), it looks
> really.
Fixed those typos and commit message and finally merged.
Thanks a lot for your several patient reviews and suggestions!

Petr

> thanks! 

> Mimi