tpm2-software / tpm2-tss

OSS implementation of the TCG TPM2 Software Stack (TSS2)
https://tpm2-software.github.io
BSD 2-Clause "Simplified" License
730 stars 359 forks source link

make check fails at asymmetric-encrypt-decrypt if run twice #414

Closed PeterHuewe closed 7 years ago

PeterHuewe commented 7 years ago

Steps to reproduce:

======================================
   tpm2.0-tss 1.0: ./test-suite.log
======================================

# TOTAL: 14
# PASS:  13
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: test/integration/asymmetric-encrypt-decrypt
=================================================

test/integration/asymmetric-encrypt-decrypt.c:88:test_invoke(): Asymmetric Encryption and Decryption Tests started.
test/integration/asymmetric-encrypt-decrypt.c:91:test_invoke(): CreatePrimary FAILED! Response Code : 0x902
FAIL test/integration/asymmetric-encrypt-decrypt (exit status: 1)
flihp commented 7 years ago

Odds are this is caused by the integration tests leaving objects loaded in the simulator: 0x902 decodes to TPM_RC_OBJECT_MEMORY. Now that we're running these tests against the simulator directly (not using a resource manager) they need to clean up after themselves.

I've considered two approaches to this:

  1. Instrument the test harness so that it clears all objects out of the simulator after each integration test runs. This may be as simple as clearing / restarting the simulator programatically.
  2. Instrument the test harness to start a new simulator for each integration test. This would tie the lifetime of a simulator instance to each test. It would also allow us to use parallel execution for the 'check' target (make -j$(nproc) check).

1 above is definitely easier, 2 is "the right way" to do it but it requires I figure out how to use an alternative test script which may take a while.

Any thoughts on the approach @PeterHuewe?

webmeister commented 7 years ago

Isn't it possible for the tests to be written in a way that (explicitly) deletes all objects that were created successfully? That would allow running the tests against a hardware TPM, that cannot be reset as easily as the simulator.

For running tests against the simualtor option 2 looks nice, as long as the overhead of launching another simulator instance is not too high compared to running the test case.

flihp commented 7 years ago

@webmeister: You're not wrong in your first point. It's possible to instrument the test harness to do all of this. The problem here is that while flushing contexts is pretty easy, cleaning out any NV indexes created etc can get pretty complex. This is doubly so if the test has created these objects with policies that may restrict their removal. It's quite possible that the cleanup code that's added to the test harness won't be able to satisfy some policies and thus not be able to do it's job.

Additionally enabling or encouraging people to run these tests against a physical TPM will likely result in a very long maintenance tail. Ignoring that some test cases may actually damage a physical TPM (NV wearout) we will still have to cope with all of the quirks that exist across all possible TPMs and their firmware versions.

So I'm leaning toward the later option of tying the lifetime of a simulator instance to that of an integration test. Your point about overhead above is valid but process creation in Linux is heavily optimized. I doubt it will even be noticeable but even if it is, it's a small price to pay for isolating side effects between test cases.

webmeister commented 7 years ago

Additionally enabling or encouraging people to run these tests against a physical TPM will likely result in a very long maintenance tail.

People are already running the tests against a physical TPM, whether we encourage them or not ;-) Therefore my hope was to reduce the maintenance effort by ensuring that the tests usually work with a physical TPM, so that everything that still gets reported is likely a real issue and not a wrong assumption in the test case.

Also, I'd like to have a way to "prove compatibility" between the TSS and a specific TPM. If not these tests, what else would you recommend?

some test cases may actually damage a physical TPM (NV wearout)

Is that a real problem, that you have seen somewhere? So far the tests look like common usage examples. Of course if you run some NV stress test 24 hours a day, 7 days a week, the TPM will stop working at some point. But as long as the tests complete within a couple of minutes, even if you run them multiple times per day, I would not expect any problems. After all, this is not so different from the TPM being used in real applications.

we will still have to cope with all of the quirks that exist across all possible TPMs and their firmware versions

Isn't that something that the TSS should eventually try to do anyway? Abstract away the differences between TPMs, so that application authors do not need to worry about them?

It's quite possible that the cleanup code that's added to the test harness won't be able to satisfy some policies and thus not be able to do it's job.

What problems do you see there?

The only way I could find to create objects that cannot be deleted anymore are NV indices with TPMA_NV_POLICY_DELETE set ("An Index with this attribute and a policy that cannot be satisfied (e.g., an Empty Policy) cannot be deleted.", TPM Library Part 2: Structures 01.38). But Platform Authorization is required to create such an index in the first place, so it is not something a normal user can do and I would not expect it to be a common scenario in the TSS tests.

For all other NV indices it says: "TPM2_NV_UndefineSpace is used to delete other Indices from the NV. The authorization given for deleting the Index is required to be the same as the authorization given to allocate the Index.", TPM Library Part 1: Architecture 01.38

flihp commented 7 years ago

The integration test harness added to master and picked over into the 1.x resolves this issue. It does so by giving each integration test a unique instance of the tpm simulator. It doesn't fix the "root cause" which is the test leaving stuff in the TPM.

Would gladly take patches to fix the root cause but for the scope of this issue you should now be able to run make check as many times as you like.