Use a single TPM context and avoid race conditions during tests #870
+671
−343
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ansasaki
force-pushed
the
improve_tpm_testing
branch
from
11c8157
to
ea16d5d
Compare
ansasaki
force-pushed
the
improve_tpm_testing
branch
from
ea16d5d
to
8f6f651
Compare
THS-on
approved these changes
Nov 21, 2024
|
/packit retest-failed |
ansasaki
force-pushed
the
improve_tpm_testing
branch
from
8f6f651
to
7774527
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
|
ansasaki
force-pushed
the
improve_tpm_testing
branch
from
7774527
to
6acaaab
Compare
28 tasks
ansasaki
force-pushed
the
improve_tpm_testing
branch
2 times, most recently
from
81d0f2b
to
4fac5f8
Compare
This implements a singleton to use a single instance of the tss_espi::Context. This also avoids race conditions by using a thread-safe mutex to access the context. The tests were modified to cleanup generated key handles between test cases, preventing leftover handles to fill the TPM memory. Introduce a mutex to allow only a single test that create keys in the TPM to run at once. It is required that tests that create keys in the TPM to run the tpm::testing::lock_tests() to obtain the mutex and be able to continue executing without the risk of other tests filling the TPM memory. The QuoteData::fixture implementation (used only during tests) was modified to flush the created key contexts when dropped. It also was modified to lock the global mutex by calling the tpm::testing::lock_tests() and provide the respective guard, preventing other tests that use the fixture (which necessarily create TPM keys) to run in parallel. Note that it is necessary to drop the QuoteData explicitly to guarantee that the keys are flushed before releasing the global mutex. Finally, the following changes were made to the tests/run.sh: * Do not use tpm2-abrmd anymore, and use only the swtpm socket instead * Stop the started processes at exit With all these changes, the tests/run.sh can now be executed locally without any special setting. Signed-off-by: Anderson Toshiyuki Sasaki <[email protected]>
Add the tests/setup_swtpm.sh script which setup a Software TPM in a temporary directory, starts the swtpm socket, and sets the environment TCTI accordingly. This allows the tests to be executed locally, even with the "testing" feature. Unfortunately, it is not possible to cleanup some of the transient objects created during tests, being necessary to cleanup manually between runs by running: $ tpm2_flushcontext -t -l -s Another caveat is that the tests need to run on a single thread to avoid test cases that create objects to run in parallel, which can fill up the TPM memory with transient object contexts. For this, please run the tests on a single thread: $ cargo test --features=testing -- --test-threads=1 The swtpm socket process is stopped when exiting from the started shell. Fixes: keylime#259 Signed-off-by: Anderson Toshiyuki Sasaki <[email protected]>
ansasaki
force-pushed
the
improve_tpm_testing
branch
from
4fac5f8
to
9b855cb
Compare
|
So I ended up implementing a mutex to let only a single test that creates keys in the TPM to run at once. This allows the tests to run in parallel, in multiple threads. I believe some of the flakyness we saw in the past is now solved. @THS-on Could you please take a look again? |
|
/packit retest-failed |
|
/packit test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements a singleton to use a single instance of the
tss_espi::Context.This also avoids race conditions by using a thread-safe mutex to access
the context.
The tests were modified to cleanup generated key handles between test
cases, preventing leftover handles to fill the TPM memory.
Introduce a mutex to allow only a single test that create keys in the
TPM to run at once. It is required that tests that create keys in the
TPM to run the
tpm::testing::lock_tests()to obtain the mutex and beable to continue executing without the risk of other tests filling the
TPM memory.
The
QuoteData::fixtureimplementation (used only during tests) wasmodified to flush the created key contexts when dropped. It also was
modified to lock the global mutex by calling the
tpm::testing::lock_tests()and provide the respective guard, preventingother tests that use the fixture (which necessarily create TPM keys) to
run in parallel.
Note that it is necessary to
droptheQuoteDatafixture explicitly to guaranteethat the keys are flushed before releasing the global mutex.
Finally, the following changes were made to the
tests/run.sh:tpm2-abrmdanymore, and use only theswtpmsocket insteadWith all these changes, the
tests/run.shcan now be executed locallywithout any special setting.
This also adds a script to initialize a software TPM locally for testing.
Fixes #259