2021-11-28 04:13:09

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v4 0/2] selftests: tpm2: Determine available PCR bank

From: Stefan Berger <[email protected]>

This series of patches fixes two issues with TPM2 selftest.
- Determines available PCR banks for use by test cases
- Resets DA lock on TPM2 to avoid subsequent test failures

Stefan

v4:
- Switch to query TPM2_GET_CAP to determine the available PCR banks
- Moved call to reset DA lock into finally branch at end of test
- Dropped patch 3

v3:
- Mention SHA-256 PCR bank as alternative in patch 1 description

v2:
- Clarified patch 1 description
- Added patch 3 with support for SHA-384 and SHA-512



Stefan Berger (2):
selftests: tpm2: Determine available PCR bank
selftests: tpm2: Reset the dictionary attack lock

tools/testing/selftests/tpm2/tpm2.py | 31 ++++++++++++++++++++++
tools/testing/selftests/tpm2/tpm2_tests.py | 31 ++++++++++++++++------
2 files changed, 54 insertions(+), 8 deletions(-)

--
2.31.1



2021-11-28 04:13:11

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v4 1/2] selftests: tpm2: Determine available PCR bank

From: Stefan Berger <[email protected]>

Determine an available PCR bank to be used by a test case by querying the
capability TPM2_GET_CAP. The TPM2 returns TPML_PCR_SELECTIONS that
contains an array of TPMS_PCR_SELECTIONs indicating available PCR banks
and the bitmasks that show which PCRs are enabled in each bank. Collect
the data in a dictionary. From the dictionary determine the PCR bank that
has the PCRs enabled that the test needs. This avoids test failures with
TPM2's that either to not have a SHA-1 bank or whose SHA-1 bank is
disabled.

Signed-off-by: Stefan Berger <[email protected]>
---
tools/testing/selftests/tpm2/tpm2.py | 31 ++++++++++++++++++++++
tools/testing/selftests/tpm2/tpm2_tests.py | 29 ++++++++++++++------
2 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/tpm2/tpm2.py b/tools/testing/selftests/tpm2/tpm2.py
index f34486cd7342..057a4f49c79d 100644
--- a/tools/testing/selftests/tpm2/tpm2.py
+++ b/tools/testing/selftests/tpm2/tpm2.py
@@ -56,6 +56,7 @@ TSS2_RESMGR_TPM_RC_LAYER = (11 << TSS2_RC_LAYER_SHIFT)

TPM2_CAP_HANDLES = 0x00000001
TPM2_CAP_COMMANDS = 0x00000002
+TPM2_CAP_PCRS = 0x00000005
TPM2_CAP_TPM_PROPERTIES = 0x00000006

TPM2_PT_FIXED = 0x100
@@ -712,3 +713,33 @@ class Client:
pt += 1

return handles
+
+ def get_cap_pcrs(self):
+ pcr_banks = {}
+
+ fmt = '>HII III'
+
+ cmd = struct.pack(fmt,
+ TPM2_ST_NO_SESSIONS,
+ struct.calcsize(fmt),
+ TPM2_CC_GET_CAPABILITY,
+ TPM2_CAP_PCRS, 0, 1)
+ rsp = self.send_cmd(cmd)[10:]
+ _, _, cnt = struct.unpack('>BII', rsp[:9])
+ rsp = rsp[9:]
+
+ # items are TPMS_PCR_SELECTION's
+ for i in range(0, cnt):
+ hash, sizeOfSelect = struct.unpack('>HB', rsp[:3])
+ rsp = rsp[3:]
+
+ pcrSelect = 0
+ if sizeOfSelect > 0:
+ pcrSelect, = struct.unpack('%ds' % sizeOfSelect,
+ rsp[:sizeOfSelect])
+ rsp = rsp[sizeOfSelect:]
+ pcrSelect = int.from_bytes(pcrSelect, byteorder='big')
+
+ pcr_banks[hash] = pcrSelect
+
+ return pcr_banks
diff --git a/tools/testing/selftests/tpm2/tpm2_tests.py b/tools/testing/selftests/tpm2/tpm2_tests.py
index 9d764306887b..e63a37819978 100644
--- a/tools/testing/selftests/tpm2/tpm2_tests.py
+++ b/tools/testing/selftests/tpm2/tpm2_tests.py
@@ -27,7 +27,17 @@ class SmokeTest(unittest.TestCase):
result = self.client.unseal(self.root_key, blob, auth, None)
self.assertEqual(data, result)

+ def determine_bank_alg(self, mask):
+ pcr_banks = self.client.get_cap_pcrs()
+ for bank_alg, pcrSelection in pcr_banks.items():
+ if pcrSelection & mask == mask:
+ return bank_alg
+ return None
+
def test_seal_with_policy(self):
+ bank_alg = self.determine_bank_alg(1 << 16)
+ self.assertIsNotNone(bank_alg)
+
handle = self.client.start_auth_session(tpm2.TPM2_SE_TRIAL)

data = ('X' * 64).encode()
@@ -35,7 +45,7 @@ class SmokeTest(unittest.TestCase):
pcrs = [16]

try:
- self.client.policy_pcr(handle, pcrs)
+ self.client.policy_pcr(handle, pcrs, bank_alg=bank_alg)
self.client.policy_password(handle)

policy_dig = self.client.get_policy_digest(handle)
@@ -47,7 +57,7 @@ class SmokeTest(unittest.TestCase):
handle = self.client.start_auth_session(tpm2.TPM2_SE_POLICY)

try:
- self.client.policy_pcr(handle, pcrs)
+ self.client.policy_pcr(handle, pcrs, bank_alg=bank_alg)
self.client.policy_password(handle)

result = self.client.unseal(self.root_key, blob, auth, handle)
@@ -72,6 +82,9 @@ class SmokeTest(unittest.TestCase):
self.assertEqual(rc, tpm2.TPM2_RC_AUTH_FAIL)

def test_unseal_with_wrong_policy(self):
+ bank_alg = self.determine_bank_alg(1 << 16 | 1 << 1)
+ self.assertIsNotNone(bank_alg)
+
handle = self.client.start_auth_session(tpm2.TPM2_SE_TRIAL)

data = ('X' * 64).encode()
@@ -79,7 +92,7 @@ class SmokeTest(unittest.TestCase):
pcrs = [16]

try:
- self.client.policy_pcr(handle, pcrs)
+ self.client.policy_pcr(handle, pcrs, bank_alg=bank_alg)
self.client.policy_password(handle)

policy_dig = self.client.get_policy_digest(handle)
@@ -91,13 +104,13 @@ class SmokeTest(unittest.TestCase):
# Extend first a PCR that is not part of the policy and try to unseal.
# This should succeed.

- ds = tpm2.get_digest_size(tpm2.TPM2_ALG_SHA1)
- self.client.extend_pcr(1, ('X' * ds).encode())
+ ds = tpm2.get_digest_size(bank_alg)
+ self.client.extend_pcr(1, ('X' * ds).encode(), bank_alg=bank_alg)

handle = self.client.start_auth_session(tpm2.TPM2_SE_POLICY)

try:
- self.client.policy_pcr(handle, pcrs)
+ self.client.policy_pcr(handle, pcrs, bank_alg=bank_alg)
self.client.policy_password(handle)

result = self.client.unseal(self.root_key, blob, auth, handle)
@@ -109,14 +122,14 @@ class SmokeTest(unittest.TestCase):

# Then, extend a PCR that is part of the policy and try to unseal.
# This should fail.
- self.client.extend_pcr(16, ('X' * ds).encode())
+ self.client.extend_pcr(16, ('X' * ds).encode(), bank_alg=bank_alg)

handle = self.client.start_auth_session(tpm2.TPM2_SE_POLICY)

rc = 0

try:
- self.client.policy_pcr(handle, pcrs)
+ self.client.policy_pcr(handle, pcrs, bank_alg=bank_alg)
self.client.policy_password(handle)

result = self.client.unseal(self.root_key, blob, auth, handle)
--
2.31.1


2021-11-28 04:13:14

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock

From: Stefan Berger <[email protected]>

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 <[email protected]>
---
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


2021-11-29 23:39:54

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] selftests: tpm2: Determine available PCR bank

On Sat, Nov 27, 2021 at 11:10:51PM -0500, Stefan Berger wrote:
> From: Stefan Berger <[email protected]>
>
> Determine an available PCR bank to be used by a test case by querying the
> capability TPM2_GET_CAP. The TPM2 returns TPML_PCR_SELECTIONS that
> contains an array of TPMS_PCR_SELECTIONs indicating available PCR banks
> and the bitmasks that show which PCRs are enabled in each bank. Collect
> the data in a dictionary. From the dictionary determine the PCR bank that
> has the PCRs enabled that the test needs. This avoids test failures with
> TPM2's that either to not have a SHA-1 bank or whose SHA-1 bank is
> disabled.
>
> Signed-off-by: Stefan Berger <[email protected]>

Acked-by: Jarkko Sakkinen <[email protected]>

/Jarkko

2021-11-29 23:43:32

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock

On Sat, Nov 27, 2021 at 11:10:52PM -0500, Stefan Berger wrote:
> From: Stefan Berger <[email protected]>
>
> 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 <[email protected]>
> ---
> 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

2021-11-30 00:26:20

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock


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 <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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

2021-12-01 10:16:15

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock

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 <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> > > 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

2021-12-01 10:19:17

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock

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 <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > > > ---
> > > > 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

2021-12-01 14:53:04

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock

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 <[email protected]>
>>>>
>>>> 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 <[email protected]>
>>>> ---
>>>> 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

2021-12-24 01:12:35

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] selftests: tpm2: Determine available PCR bank

Shuah,

  are you going to take this fix here - only 1/2 ?

https://lore.kernel.org/lkml/[email protected]/T/#m21209a978c237368499ce5f082f3c0fc03bcbbeb

  Stefan

On 11/29/21 18:39, Jarkko Sakkinen wrote:
> On Sat, Nov 27, 2021 at 11:10:51PM -0500, Stefan Berger wrote:
>> From: Stefan Berger <[email protected]>
>>
>> Determine an available PCR bank to be used by a test case by querying the
>> capability TPM2_GET_CAP. The TPM2 returns TPML_PCR_SELECTIONS that
>> contains an array of TPMS_PCR_SELECTIONs indicating available PCR banks
>> and the bitmasks that show which PCRs are enabled in each bank. Collect
>> the data in a dictionary. From the dictionary determine the PCR bank that
>> has the PCRs enabled that the test needs. This avoids test failures with
>> TPM2's that either to not have a SHA-1 bank or whose SHA-1 bank is
>> disabled.
>>
>> Signed-off-by: Stefan Berger <[email protected]>
> Acked-by: Jarkko Sakkinen <[email protected]>
>
> /Jarkko

2022-01-13 18:04:15

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] selftests: tpm2: Determine available PCR bank

Jarkko,

  can you take this patch 1/2?

 https://lore.kernel.org/lkml/[email protected]/T/#m21209a978c237368499ce5f082f3c0fc03bcbbeb

  Stefan


On 12/23/21 20:12, Stefan Berger wrote:
> Shuah,
>
>   are you going to take this fix here - only 1/2 ?
>
> https://lore.kernel.org/lkml/[email protected]/T/#m21209a978c237368499ce5f082f3c0fc03bcbbeb
>
>
>   Stefan
>
> On 11/29/21 18:39, Jarkko Sakkinen wrote:
>> On Sat, Nov 27, 2021 at 11:10:51PM -0500, Stefan Berger wrote:
>>> From: Stefan Berger <[email protected]>
>>>
>>> Determine an available PCR bank to be used by a test case by
>>> querying the
>>> capability TPM2_GET_CAP. The TPM2 returns TPML_PCR_SELECTIONS that
>>> contains an array of TPMS_PCR_SELECTIONs indicating available PCR banks
>>> and the bitmasks that show which PCRs are enabled in each bank. Collect
>>> the data in a dictionary. From the dictionary determine the PCR bank
>>> that
>>> has the PCRs enabled that the test needs. This avoids test failures
>>> with
>>> TPM2's that either to not have a SHA-1 bank or whose SHA-1 bank is
>>> disabled.
>>>
>>> Signed-off-by: Stefan Berger <[email protected]>
>> Acked-by: Jarkko Sakkinen <[email protected]>
>>
>> /Jarkko

2022-01-16 06:46:44

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] selftests: tpm2: Determine available PCR bank

On Thu, Jan 13, 2022 at 01:04:03PM -0500, Stefan Berger wrote:
> Jarkko,
>
> ? can you take this patch 1/2?
>
> ?https://lore.kernel.org/lkml/[email protected]/T/#m21209a978c237368499ce5f082f3c0fc03bcbbeb
>
> ? Stefan

Oops. Sorry, I missed your request at 23rd.

Yes, we can for sure take that. I now tested by with SHA256 only
configuration so:

Tested-by: Jarkko Sakkinen <[email protected]>

I'm considering 5.17-rc2 pull rquest but want to leave the final
decision to the time when it can be sent. If I'll make rc2 PR in
the first place, I'll include this to the pull request.

/Jarkko

2022-01-16 13:46:29

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] selftests: tpm2: Determine available PCR bank

On Sat, Jan 15, 2022 at 05:53:18PM +0200, Jarkko Sakkinen wrote:
> On Thu, Jan 13, 2022 at 01:04:03PM -0500, Stefan Berger wrote:
> > Jarkko,
> >
> > ? can you take this patch 1/2?
> >
> > ?https://lore.kernel.org/lkml/[email protected]/T/#m21209a978c237368499ce5f082f3c0fc03bcbbeb
> >
> > ? Stefan
>
> Oops. Sorry, I missed your request at 23rd.
>
> Yes, we can for sure take that. I now tested by with SHA256 only
> configuration so:
>
> Tested-by: Jarkko Sakkinen <[email protected]>
>
> I'm considering 5.17-rc2 pull rquest but want to leave the final
> decision to the time when it can be sent. If I'll make rc2 PR in
> the first place, I'll include this to the pull request.

OK, it's now applied, thank you.

BR, Jarkko