| Message ID | 20211128041052.1395504-3-stefanb@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | selftests: tpm2: Determine available PCR bank | expand |
On Sat, Nov 27, 2021 at 11:10:52PM -0500, Stefan Berger wrote: > From: Stefan Berger <stefanb@linux.ibm.com> > > Reset the dictionary attack lock to avoid the following types of test > failures after running the test 2 times: > > ====================================================================== > ERROR: test_unseal_with_wrong_policy (tpm2_tests.SmokeTest) > ---------------------------------------------------------------------- > Traceback (most recent call last): > File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2_tests.py", line 105, in test_unseal_with_wrong_policy > blob = self.client.seal(self.root_key, data, auth, policy_dig) > File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2.py", line 620, in seal > rsp = self.send_cmd(cmd) > File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2.py", line 397, in send_cmd > raise ProtocolError(cc, rc) > tpm2.ProtocolError: TPM_RC_LOCKOUT: cc=0x00000153, rc=0x00000921 > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > tools/testing/selftests/tpm2/tpm2_tests.py | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tools/testing/selftests/tpm2/tpm2_tests.py b/tools/testing/selftests/tpm2/tpm2_tests.py > index e63a37819978..ad6f54c01adf 100644 > --- a/tools/testing/selftests/tpm2/tpm2_tests.py > +++ b/tools/testing/selftests/tpm2/tpm2_tests.py > @@ -139,6 +139,8 @@ class SmokeTest(unittest.TestCase): > except: > self.client.flush_context(handle) > raise > + finally: > + self.client.reset_da_lock() > > self.assertEqual(rc, tpm2.TPM2_RC_POLICY_FAIL) > > -- > 2.31.1 > I don't agree with this as a DA lock has legit use. This would be adequate for systems dedicated for kernel testing only. We could make this available in the folder where TPM2 tests are: https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/tpm2-scripts.git/tree/tpm2-reset-da-lock /Jarkko
On 11/29/21 18:43, Jarkko Sakkinen wrote: > On Sat, Nov 27, 2021 at 11:10:52PM -0500, Stefan Berger wrote: >> From: Stefan Berger <stefanb@linux.ibm.com> >> >> Reset the dictionary attack lock to avoid the following types of test >> failures after running the test 2 times: >> >> ====================================================================== >> ERROR: test_unseal_with_wrong_policy (tpm2_tests.SmokeTest) >> ---------------------------------------------------------------------- >> Traceback (most recent call last): >> File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2_tests.py", line 105, in test_unseal_with_wrong_policy >> blob = self.client.seal(self.root_key, data, auth, policy_dig) >> File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2.py", line 620, in seal >> rsp = self.send_cmd(cmd) >> File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2.py", line 397, in send_cmd >> raise ProtocolError(cc, rc) >> tpm2.ProtocolError: TPM_RC_LOCKOUT: cc=0x00000153, rc=0x00000921 >> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> --- >> tools/testing/selftests/tpm2/tpm2_tests.py | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/tools/testing/selftests/tpm2/tpm2_tests.py b/tools/testing/selftests/tpm2/tpm2_tests.py >> index e63a37819978..ad6f54c01adf 100644 >> --- a/tools/testing/selftests/tpm2/tpm2_tests.py >> +++ b/tools/testing/selftests/tpm2/tpm2_tests.py >> @@ -139,6 +139,8 @@ class SmokeTest(unittest.TestCase): >> except: >> self.client.flush_context(handle) >> raise >> + finally: >> + self.client.reset_da_lock() >> >> self.assertEqual(rc, tpm2.TPM2_RC_POLICY_FAIL) >> >> -- >> 2.31.1 >> > I don't agree with this as a DA lock has legit use. This would be adequate > for systems dedicated for kernel testing only. The problem is this particular test case I am patching here causes the above test failures upon rerun. We are testing the driver here presumably and not the TPM2, so I think we should leave the TPM2 as cleaned up as possible, thus my suggestion is to reset the DA lock and we won't hear any complaints after that. > We could make this available in the folder where TPM2 tests are: > > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/tpm2-scripts.git/tree/tpm2-reset-da-lock The tss packages also have command line tools to reset the DA lock, but it shouldn't be necessary to use them after running a **driver** test case. stefan > > /Jarkko
On Mon, Nov 29, 2021 at 07:26:12PM -0500, Stefan Berger wrote: > > On 11/29/21 18:43, Jarkko Sakkinen wrote: > > On Sat, Nov 27, 2021 at 11:10:52PM -0500, Stefan Berger wrote: > > > From: Stefan Berger <stefanb@linux.ibm.com> > > > > > > Reset the dictionary attack lock to avoid the following types of test > > > failures after running the test 2 times: > > > > > > ====================================================================== > > > ERROR: test_unseal_with_wrong_policy (tpm2_tests.SmokeTest) > > > ---------------------------------------------------------------------- > > > Traceback (most recent call last): > > > File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2_tests.py", line 105, in test_unseal_with_wrong_policy > > > blob = self.client.seal(self.root_key, data, auth, policy_dig) > > > File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2.py", line 620, in seal > > > rsp = self.send_cmd(cmd) > > > File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2.py", line 397, in send_cmd > > > raise ProtocolError(cc, rc) > > > tpm2.ProtocolError: TPM_RC_LOCKOUT: cc=0x00000153, rc=0x00000921 > > > > > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > > > --- > > > tools/testing/selftests/tpm2/tpm2_tests.py | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/tools/testing/selftests/tpm2/tpm2_tests.py b/tools/testing/selftests/tpm2/tpm2_tests.py > > > index e63a37819978..ad6f54c01adf 100644 > > > --- a/tools/testing/selftests/tpm2/tpm2_tests.py > > > +++ b/tools/testing/selftests/tpm2/tpm2_tests.py > > > @@ -139,6 +139,8 @@ class SmokeTest(unittest.TestCase): > > > except: > > > self.client.flush_context(handle) > > > raise > > > + finally: > > > + self.client.reset_da_lock() > > > self.assertEqual(rc, tpm2.TPM2_RC_POLICY_FAIL) > > > -- > > > 2.31.1 > > > > > I don't agree with this as a DA lock has legit use. This would be adequate > > for systems dedicated for kernel testing only. > > The problem is this particular test case I am patching here causes the above > test failures upon rerun. We are testing the driver here presumably and not > the TPM2, so I think we should leave the TPM2 as cleaned up as possible, > thus my suggestion is to reset the DA lock and we won't hear any complaints > after that. Ok. > > We could make this available in the folder where TPM2 tests are: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/tpm2-scripts.git/tree/tpm2-reset-da-lock > > > The tss packages also have command line tools to reset the DA lock, but it > shouldn't be necessary to use them after running a **driver** test case. If you speak about TSS, please alway say which one :-) Adding non-volatile state changes explicitly is to a test case is both intrusive and wrong. These type of choices are not to be done in the test case implementation for sure. An improvement that does not add extra side-effect, would be to read the TPM_PT_LOCKOUT_COUNTER and roll back with a proper error message for the lockout condition. You can also configure the maximum number of tries up to (2 << 31) - 1 = 4294967295 with TPM2_DictionaryAttackParameters. /Jarkko
On Wed, Dec 01, 2021 at 12:16:11PM +0200, Jarkko Sakkinen wrote: > On Mon, Nov 29, 2021 at 07:26:12PM -0500, Stefan Berger wrote: > > > > On 11/29/21 18:43, Jarkko Sakkinen wrote: > > > On Sat, Nov 27, 2021 at 11:10:52PM -0500, Stefan Berger wrote: > > > > From: Stefan Berger <stefanb@linux.ibm.com> > > > > > > > > Reset the dictionary attack lock to avoid the following types of test > > > > failures after running the test 2 times: > > > > > > > > ====================================================================== > > > > ERROR: test_unseal_with_wrong_policy (tpm2_tests.SmokeTest) > > > > ---------------------------------------------------------------------- > > > > Traceback (most recent call last): > > > > File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2_tests.py", line 105, in test_unseal_with_wrong_policy > > > > blob = self.client.seal(self.root_key, data, auth, policy_dig) > > > > File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2.py", line 620, in seal > > > > rsp = self.send_cmd(cmd) > > > > File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2.py", line 397, in send_cmd > > > > raise ProtocolError(cc, rc) > > > > tpm2.ProtocolError: TPM_RC_LOCKOUT: cc=0x00000153, rc=0x00000921 > > > > > > > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > > > > --- > > > > tools/testing/selftests/tpm2/tpm2_tests.py | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/tools/testing/selftests/tpm2/tpm2_tests.py b/tools/testing/selftests/tpm2/tpm2_tests.py > > > > index e63a37819978..ad6f54c01adf 100644 > > > > --- a/tools/testing/selftests/tpm2/tpm2_tests.py > > > > +++ b/tools/testing/selftests/tpm2/tpm2_tests.py > > > > @@ -139,6 +139,8 @@ class SmokeTest(unittest.TestCase): > > > > except: > > > > self.client.flush_context(handle) > > > > raise > > > > + finally: > > > > + self.client.reset_da_lock() > > > > self.assertEqual(rc, tpm2.TPM2_RC_POLICY_FAIL) > > > > -- > > > > 2.31.1 > > > > > > > I don't agree with this as a DA lock has legit use. This would be adequate > > > for systems dedicated for kernel testing only. > > > > The problem is this particular test case I am patching here causes the above > > test failures upon rerun. We are testing the driver here presumably and not > > the TPM2, so I think we should leave the TPM2 as cleaned up as possible, > > thus my suggestion is to reset the DA lock and we won't hear any complaints > > after that. > > Ok. > > > > We could make this available in the folder where TPM2 tests are: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/tpm2-scripts.git/tree/tpm2-reset-da-lock > > > > > > The tss packages also have command line tools to reset the DA lock, but it > > shouldn't be necessary to use them after running a **driver** test case. > > If you speak about TSS, please alway say which one :-) > > Adding non-volatile state changes explicitly is to a test case is both A typo, should be: "Adding non-volatile state changes explicitly to a test case is both" /Jarkko
On 12/1/21 05:16, Jarkko Sakkinen wrote: > On Mon, Nov 29, 2021 at 07:26:12PM -0500, Stefan Berger wrote: >> On 11/29/21 18:43, Jarkko Sakkinen wrote: >>> On Sat, Nov 27, 2021 at 11:10:52PM -0500, Stefan Berger wrote: >>>> From: Stefan Berger <stefanb@linux.ibm.com> >>>> >>>> Reset the dictionary attack lock to avoid the following types of test >>>> failures after running the test 2 times: >>>> >>>> ====================================================================== >>>> ERROR: test_unseal_with_wrong_policy (tpm2_tests.SmokeTest) >>>> ---------------------------------------------------------------------- >>>> Traceback (most recent call last): >>>> File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2_tests.py", line 105, in test_unseal_with_wrong_policy >>>> blob = self.client.seal(self.root_key, data, auth, policy_dig) >>>> File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2.py", line 620, in seal >>>> rsp = self.send_cmd(cmd) >>>> File "/root/linux-ima-namespaces/tools/testing/selftests/tpm2/tpm2.py", line 397, in send_cmd >>>> raise ProtocolError(cc, rc) >>>> tpm2.ProtocolError: TPM_RC_LOCKOUT: cc=0x00000153, rc=0x00000921 >>>> >>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >>>> --- >>>> tools/testing/selftests/tpm2/tpm2_tests.py | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/tools/testing/selftests/tpm2/tpm2_tests.py b/tools/testing/selftests/tpm2/tpm2_tests.py >>>> index e63a37819978..ad6f54c01adf 100644 >>>> --- a/tools/testing/selftests/tpm2/tpm2_tests.py >>>> +++ b/tools/testing/selftests/tpm2/tpm2_tests.py >>>> @@ -139,6 +139,8 @@ class SmokeTest(unittest.TestCase): >>>> except: >>>> self.client.flush_context(handle) >>>> raise >>>> + finally: >>>> + self.client.reset_da_lock() >>>> self.assertEqual(rc, tpm2.TPM2_RC_POLICY_FAIL) >>>> -- >>>> 2.31.1 >>>> >>> I don't agree with this as a DA lock has legit use. This would be adequate >>> for systems dedicated for kernel testing only. >> The problem is this particular test case I am patching here causes the above >> test failures upon rerun. We are testing the driver here presumably and not >> the TPM2, so I think we should leave the TPM2 as cleaned up as possible, >> thus my suggestion is to reset the DA lock and we won't hear any complaints >> after that. > >> The tss packages also have command line tools to reset the DA lock, but it >> shouldn't be necessary to use them after running a **driver** test case. > If you speak about TSS, please alway say which one :-) packages : both tpm2_dictionarylockout -c [ -p password ] tssdictionaryattacklockreset [-pwd password] > > Adding non-volatile state changes explicitly is to a test case is both > intrusive and wrong. These type of choices are not to be done in the > test case implementation for sure. I drop this patch because if someone changed the password the lock reset will not work as expected. One can also argue with having a test case that triggers a failure condition in the TPM2 is both intrusive and wrong. That said, the offending test cases should probably not even exist, especially since they test a user's knowledge about the device afterwards... > > An improvement that does not add extra side-effect, would be to read the > TPM_PT_LOCKOUT_COUNTER and roll back with a proper error message for the > lockout condition. > > You can also configure the maximum number of tries up to (2 << 31) - 1 > = 4294967295 with TPM2_DictionaryAttackParameters... ... which I think the user of a device driver test should not have to do. Stefan > > /Jarkko
diff --git a/tools/testing/selftests/tpm2/tpm2_tests.py b/tools/testing/selftests/tpm2/tpm2_tests.py index e63a37819978..ad6f54c01adf 100644 --- a/tools/testing/selftests/tpm2/tpm2_tests.py +++ b/tools/testing/selftests/tpm2/tpm2_tests.py @@ -139,6 +139,8 @@ class SmokeTest(unittest.TestCase): except: self.client.flush_context(handle) raise + finally: + self.client.reset_da_lock() self.assertEqual(rc, tpm2.TPM2_RC_POLICY_FAIL)