* [PATCH v4 0/2] selftests: tpm2: Determine available PCR bank @ 2021-11-28 4:10 Stefan Berger 2021-11-28 4:10 ` [PATCH v4 1/2] " Stefan Berger 2021-11-28 4:10 ` [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock Stefan Berger 0 siblings, 2 replies; 13+ messages in thread From: Stefan Berger @ 2021-11-28 4:10 UTC (permalink / raw) To: jarkko, linux-integrity Cc: peterhuewe, linux-kernel, linux-kselftest, skhan, Stefan Berger From: Stefan Berger <stefanb@linux.ibm.com> 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 1/2] selftests: tpm2: Determine available PCR bank 2021-11-28 4:10 [PATCH v4 0/2] selftests: tpm2: Determine available PCR bank Stefan Berger @ 2021-11-28 4:10 ` Stefan Berger 2021-11-29 23:39 ` Jarkko Sakkinen 2021-11-28 4:10 ` [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock Stefan Berger 1 sibling, 1 reply; 13+ messages in thread From: Stefan Berger @ 2021-11-28 4:10 UTC (permalink / raw) To: jarkko, linux-integrity Cc: peterhuewe, linux-kernel, linux-kselftest, skhan, Stefan Berger From: Stefan Berger <stefanb@linux.ibm.com> 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 <stefanb@linux.ibm.com> --- 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 1/2] selftests: tpm2: Determine available PCR bank 2021-11-28 4:10 ` [PATCH v4 1/2] " Stefan Berger @ 2021-11-29 23:39 ` Jarkko Sakkinen 2021-12-24 1:12 ` Stefan Berger 0 siblings, 1 reply; 13+ messages in thread From: Jarkko Sakkinen @ 2021-11-29 23:39 UTC (permalink / raw) To: Stefan Berger Cc: linux-integrity, peterhuewe, linux-kernel, linux-kselftest, skhan, Stefan Berger On Sat, Nov 27, 2021 at 11:10:51PM -0500, Stefan Berger wrote: > From: Stefan Berger <stefanb@linux.ibm.com> > > 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 <stefanb@linux.ibm.com> Acked-by: Jarkko Sakkinen <jarkko@kernel.org> /Jarkko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 1/2] selftests: tpm2: Determine available PCR bank 2021-11-29 23:39 ` Jarkko Sakkinen @ 2021-12-24 1:12 ` Stefan Berger 2022-01-13 18:04 ` Stefan Berger 0 siblings, 1 reply; 13+ messages in thread From: Stefan Berger @ 2021-12-24 1:12 UTC (permalink / raw) To: Jarkko Sakkinen, Stefan Berger Cc: linux-integrity, peterhuewe, linux-kernel, linux-kselftest, skhan Shuah, are you going to take this fix here - only 1/2 ? https://lore.kernel.org/lkml/20211128041052.1395504-1-stefanb@linux.vnet.ibm.com/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 <stefanb@linux.ibm.com> >> >> 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 <stefanb@linux.ibm.com> > Acked-by: Jarkko Sakkinen <jarkko@kernel.org> > > /Jarkko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 1/2] selftests: tpm2: Determine available PCR bank 2021-12-24 1:12 ` Stefan Berger @ 2022-01-13 18:04 ` Stefan Berger 2022-01-15 15:53 ` Jarkko Sakkinen 0 siblings, 1 reply; 13+ messages in thread From: Stefan Berger @ 2022-01-13 18:04 UTC (permalink / raw) To: Jarkko Sakkinen, Stefan Berger Cc: linux-integrity, peterhuewe, linux-kernel, linux-kselftest, skhan Jarkko, can you take this patch 1/2? https://lore.kernel.org/lkml/20211128041052.1395504-1-stefanb@linux.vnet.ibm.com/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/20211128041052.1395504-1-stefanb@linux.vnet.ibm.com/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 <stefanb@linux.ibm.com> >>> >>> 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 <stefanb@linux.ibm.com> >> Acked-by: Jarkko Sakkinen <jarkko@kernel.org> >> >> /Jarkko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 1/2] selftests: tpm2: Determine available PCR bank 2022-01-13 18:04 ` Stefan Berger @ 2022-01-15 15:53 ` Jarkko Sakkinen 2022-01-15 17:02 ` Jarkko Sakkinen 0 siblings, 1 reply; 13+ messages in thread From: Jarkko Sakkinen @ 2022-01-15 15:53 UTC (permalink / raw) To: Stefan Berger Cc: Stefan Berger, linux-integrity, peterhuewe, linux-kernel, linux-kselftest, skhan 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/20211128041052.1395504-1-stefanb@linux.vnet.ibm.com/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 <jarkko@kernel.org> 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 1/2] selftests: tpm2: Determine available PCR bank 2022-01-15 15:53 ` Jarkko Sakkinen @ 2022-01-15 17:02 ` Jarkko Sakkinen 0 siblings, 0 replies; 13+ messages in thread From: Jarkko Sakkinen @ 2022-01-15 17:02 UTC (permalink / raw) To: Stefan Berger Cc: Stefan Berger, linux-integrity, peterhuewe, linux-kernel, linux-kselftest, skhan 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/20211128041052.1395504-1-stefanb@linux.vnet.ibm.com/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 <jarkko@kernel.org> > > 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock 2021-11-28 4:10 [PATCH v4 0/2] selftests: tpm2: Determine available PCR bank Stefan Berger 2021-11-28 4:10 ` [PATCH v4 1/2] " Stefan Berger @ 2021-11-28 4:10 ` Stefan Berger 2021-11-29 23:43 ` Jarkko Sakkinen 1 sibling, 1 reply; 13+ messages in thread From: Stefan Berger @ 2021-11-28 4:10 UTC (permalink / raw) To: jarkko, linux-integrity Cc: peterhuewe, linux-kernel, linux-kselftest, skhan, Stefan Berger 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock 2021-11-28 4:10 ` [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock Stefan Berger @ 2021-11-29 23:43 ` Jarkko Sakkinen 2021-11-30 0:26 ` Stefan Berger 0 siblings, 1 reply; 13+ messages in thread From: Jarkko Sakkinen @ 2021-11-29 23:43 UTC (permalink / raw) To: Stefan Berger Cc: linux-integrity, peterhuewe, linux-kernel, linux-kselftest, skhan, Stefan Berger 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock 2021-11-29 23:43 ` Jarkko Sakkinen @ 2021-11-30 0:26 ` Stefan Berger 2021-12-01 10:16 ` Jarkko Sakkinen 0 siblings, 1 reply; 13+ messages in thread From: Stefan Berger @ 2021-11-30 0:26 UTC (permalink / raw) To: Jarkko Sakkinen, Stefan Berger Cc: linux-integrity, peterhuewe, linux-kernel, linux-kselftest, skhan 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock 2021-11-30 0:26 ` Stefan Berger @ 2021-12-01 10:16 ` Jarkko Sakkinen 2021-12-01 10:19 ` Jarkko Sakkinen 2021-12-01 14:52 ` Stefan Berger 0 siblings, 2 replies; 13+ messages in thread From: Jarkko Sakkinen @ 2021-12-01 10:16 UTC (permalink / raw) To: Stefan Berger Cc: Stefan Berger, linux-integrity, peterhuewe, linux-kernel, linux-kselftest, skhan 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock 2021-12-01 10:16 ` Jarkko Sakkinen @ 2021-12-01 10:19 ` Jarkko Sakkinen 2021-12-01 14:52 ` Stefan Berger 1 sibling, 0 replies; 13+ messages in thread From: Jarkko Sakkinen @ 2021-12-01 10:19 UTC (permalink / raw) To: Stefan Berger Cc: Stefan Berger, linux-integrity, peterhuewe, linux-kernel, linux-kselftest, skhan 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock 2021-12-01 10:16 ` Jarkko Sakkinen 2021-12-01 10:19 ` Jarkko Sakkinen @ 2021-12-01 14:52 ` Stefan Berger 1 sibling, 0 replies; 13+ messages in thread From: Stefan Berger @ 2021-12-01 14:52 UTC (permalink / raw) To: Jarkko Sakkinen Cc: Stefan Berger, linux-integrity, peterhuewe, linux-kernel, linux-kselftest, skhan 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-01-15 17:02 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-28 4:10 [PATCH v4 0/2] selftests: tpm2: Determine available PCR bank Stefan Berger 2021-11-28 4:10 ` [PATCH v4 1/2] " Stefan Berger 2021-11-29 23:39 ` Jarkko Sakkinen 2021-12-24 1:12 ` Stefan Berger 2022-01-13 18:04 ` Stefan Berger 2022-01-15 15:53 ` Jarkko Sakkinen 2022-01-15 17:02 ` Jarkko Sakkinen 2021-11-28 4:10 ` [PATCH v4 2/2] selftests: tpm2: Reset the dictionary attack lock Stefan Berger 2021-11-29 23:43 ` Jarkko Sakkinen 2021-11-30 0:26 ` Stefan Berger 2021-12-01 10:16 ` Jarkko Sakkinen 2021-12-01 10:19 ` Jarkko Sakkinen 2021-12-01 14:52 ` Stefan Berger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).