Encrypting data with tpm2_encryptdecrypt is failing on an Intel PTT TPM2.0 #621

martinezjavier · 2017-11-15T17:04:53Z

When testing the upcoming 3.X branch, I noticed an issue (that's also present in master) with tpm2_encryptdecrypt and an Intel PTT firmware based TPM2.0 device.

The tpm2_encryptdecrypt tools now first tries to use the EncryptDecrypt2 command and only if that fails fallbacks to EncryptDecrypt. But it does so only if returns EncryptDecrypt2 a TPM_RC_COMMAND_CODE response code.

But on this TPM2, a call to EncryptDecrypt2 returns a TSS2_BASE_RC_IO_ERROR error:

$ tpm2_encryptdecrypt -Q -c decrypt.ctx -D NO -I secret.dat -o encrypt.out
ERROR: EncryptDecrypt failed, error code: 0xa000a

$ tpm2_rc_decode 0xa000a
error layer
  hex: 0xa0000
  identifier: TSS2_TCTI_ERROR_LEVEL
  description: Error from the TCTI
base error code
  identifier: TSS2_BASE_RC_IO_ERROR
  description: IO failure

I think that a solution could be to just check if the response code isn't TPM_RC_SUCCESS and fallback on any error. Something like the following:

diff --git a/tools/tpm2_encryptdecrypt.c b/tools/tpm2_encryptdecrypt.c
index 982fe7e3d757..e128cae3ecc3 100644
--- a/tools/tpm2_encryptdecrypt.c
+++ b/tools/tpm2_encryptdecrypt.c
@@ -102,7 +102,7 @@ static bool encrypt_decrypt(TSS2_SYS_CONTEXT *sapi_context) {
     TPM_RC rval = TSS2_RETRY_EXP(Tss2_Sys_EncryptDecrypt2(sapi_context, ctx.key_handle,
             &sessions_data, &ctx.data, ctx.is_decrypt, TPM_ALG_NULL, &iv_in, &out_data,
             &iv_out, &sessions_data_out));
-    if (rval == TPM_RC_COMMAND_CODE) {
+    if (rval != TPM_RC_SUCCESS) {
         rval = TSS2_RETRY_EXP(Tss2_Sys_EncryptDecrypt(sapi_context, ctx.key_handle,
                 &sessions_data, ctx.is_decrypt, TPM_ALG_NULL, &iv_in, &ctx.data,
                 &out_data, &iv_out, &sessions_data_out));

@flihp what do you suggest?

The text was updated successfully, but these errors were encountered:

flihp · 2017-11-15T17:39:28Z

This may be better split up into two separate issues. We're about to mix discussion of two separate issues into a single thread.

To your first item above @martinezjavier: The RC you're getting back says it's coming from the TCTI layer. Which TCTI are you using in your test setup? I'll try to repro to get some clarity on this.

martinezjavier · 2017-11-15T17:48:33Z

@flihp yes, sorry for reporting both problems in the same issue...

I was testing with the device TCTI and the in-kernel resource manager (/dev/tpmrm0), with the tpm2-abrmd and the abrmd TCTI it works correctly (EncryptDecrypt2 returns TPM_RC_COMMAND_CODE).

martinezjavier · 2017-11-15T18:03:00Z

I modified my original report to only include the EncryptDecrypt2 not returning a TPM_RC_COMMAND_CODE issue and will file a new issue for the other problem.

flihp · 2017-11-16T00:05:05Z

The TCTI data is very valuable. The issue then is either in the device TCTI or the kernel. if you're able to test the device TCTI against /dev/tpm0 then we'll know for sure. I'll try to track down a PTT system in the meantime to repro.

martinezjavier · 2017-11-16T00:24:12Z

Yes, I tried that too but got TPM_RC_OBJECT_MEMORY due the TPM only supporting 3 transients objects.

I don't have acceas to the machine now but I'll try tomorrow to make the primary key and the key persistent to prevent that and let you know how that goes.

martinezjavier · 2017-11-16T09:39:30Z

@flihp so I've tested now with the device TCTI but without the in-kernel resource manager, just using /dev/tpm0. And it does work indeed, the TPM properly returns a TPM_RC_COMMAND_CODE response code.

We don't have a TPM2_FlushContext tool yet (@jiazhang0 are you planning to share yours?) so I've used the one in the IBM TSS project. This is what I did:

$ export TPM2TOOLS_TCTI_NAME=device
$ export TPM2TOOLS_DEVICE_FILE=/dev/tpm0

# Check how many transitions objects slot are available in the TPM
$ tpm2_getcap -c properties-variable | grep TPM_PT_HR_TRANSIENT_AVAIL
TPM_PT_HR_TRANSIENT_AVAIL:   0x00000003

$ tpm2_createprimary -H o -g sha1 -G rsa -C primary.ctx
ObjectAttribute: 0x00030072

CreatePrimary Succeed ! Handle: 0x80000000

$ tpm2_getcap -c properties-variable | grep TPM_PT_HR_TRANSIENT_AVAIL
TPM_PT_HR_TRANSIENT_AVAIL:   0x00000002

$ TPM_INTERFACE_TYPE=dev ./flushcontext -ha 80000000

$ tpm2_getcap -c properties-variable | grep TPM_PT_HR_TRANSIENT_AVAIL
TPM_PT_HR_TRANSIENT_AVAIL:   0x00000003

$ tpm2_create -g sha256 -G symcipher -u key.pub -r key.priv -c primary.ctx 
ObjectAttribute: 0x00020072

Create Object Succeed !

$ tpm2_getcap -c properties-variable | grep TPM_PT_HR_TRANSIENT_AVAIL
TPM_PT_HR_TRANSIENT_AVAIL:   0x00000002

$ TPM_INTERFACE_TYPE=dev ./flushcontext -ha 80000000

$ tpm2_getcap -c properties-variable | grep TPM_PT_HR_TRANSIENT_AVAIL
TPM_PT_HR_TRANSIENT_AVAIL:   0x00000003

$ tpm2_load -c primary.ctx -u key.pub -r key.priv -n key.name -C decrypt.ctx

Load succ.
LoadedHandle: 0x80000001

$ tpm2_getcap -c properties-variable | grep TPM_PT_HR_TRANSIENT_AVAIL
TPM_PT_HR_TRANSIENT_AVAIL:   0x00000001

$ tpm2_encryptdecrypt -Q -c decrypt.ctx -D NO -I secret.dat -o encrypt.out
rval for decrypt2 0x143
ERROR: EncryptDecrypt failed, error code: 0x182

$ tpm2_rc_decode 0x143
error layer
  hex: 0x0
  identifier: TSS2_TPM_ERROR_LEVEL
  description: Error produced by the TPM
format 0 error code
  hex: 0x43
  name: TPM_RC_COMMAND_CODE
  description: command code not supported

So the problem appears to be in the kernel resource manager. I'll investigate there but it seems this isn't a tpm2-{tools,tss} issue after all. Thanks a lot for your help!

martinezjavier · 2017-11-16T10:03:34Z

So running the tpm2_encryptdecrypt with strace in both cases, I do see that the kernel is returning an -EINVAL when writing the TPM2_EncryptDecrypt2 command to /dev/tpmrm0 while the write succeeds when writing directly to the TPM by using /dev/tpm0.

openat(AT_FDCWD, "/dev/tpmrm0", O_RDWR) = 3
...
write(3, "\200\2\0\0\0009\0\0\1\223\200\377\377\377\0\0\0\t@\0\0\t\0\0\0\0\0\0\7foo"..., 57) = -1 EINVAL (Invalid argument)

openat(AT_FDCWD, "/dev/tpm0", O_RDWR)   = 3
...
write(3, "\200\2\0\0\0009\0\0\1\223\200\0\0\2\0\0\0\t@\0\0\t\0\0\0\0\0\0\7foo"..., 57) = 57

martinezjavier · 2017-11-16T11:03:57Z

@flihp @williamcroberts Ok, after studying the TPM kernel code I found the semantic difference.

When not sending TPM commands through the in-kernel resource manager (TPM spaces using the kernel nomenclature), the TPM commands are just sent to the TPM without checking if are supported or not by the TPM, by it does check if using the TPM spaces.

The tpm_transmit() function calls to tpm_validate_command() that's just a no-op if TPM spaces are not used:

static bool tpm_validate_command(struct tpm_chip *chip,
				 struct tpm_space *space,
				 const u8 *cmd,
				 size_t len)
{
	const struct tpm_input_header *header = (const void *)cmd;
	int i;
	u32 cc;
	u32 attrs;
	unsigned int nr_handles;

	if (len < TPM_HEADER_SIZE)
		return false;

	if (!space)
		return true;

	if (chip->flags & TPM_CHIP_FLAG_TPM2 && chip->nr_commands) {
		cc = be32_to_cpu(header->ordinal);

		i = tpm2_find_cc(chip, cc);
		if (i < 0) {
			dev_dbg(&chip->dev, "0x%04X is an invalid command\n",
				cc);
			return false;
		}

		attrs = chip->cc_attrs_tbl[i];
		nr_handles =
			4 * ((attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0));
		if (len < TPM_HEADER_SIZE + 4 * nr_handles)
			goto err_len;
	}

	return true;
err_len:
	dev_dbg(&chip->dev,
		"%s: insufficient command length %zu", __func__, len);
	return false;
}

And indeed, when enabling that dynamic debug printout about the invalid command, I see that the kernel reports it in its ring buffer that 0x193 (TPM_CC_EncryptDecrypt2) is an invalid command.

$ cd /sys/kernel/debug/dynamic_debug
$ grep " is an invalid command" control                         
drivers/char/tpm/tpm-interface.c:354 [tpm]tpm_validate_command =_ "0x%04X is an invalid command\012"
$ echo "file tpm-interface.c line 354 +p" > control

$ tpm2_encryptdecrypt -Q -c decrypt.ctx -D NO -I secret.dat -o encrypt.out

$ dmesg
[101234.921080] tpm tpm0: 0x0193 is an invalid command

I'll try to cook a patch for the kernel, but first I need to dig further on this and understand why this check is done in the TPM space code and not just let the TPM2 to return TPM_RC_COMMAND_CODE as is the case when not using the in-kernel RM.

But yeah, it's confirmed that's a kernel issue and not related to the TCTI device or SAPI layers. I'm still keeping the bug open since it could be of interest to others that face the same, at least until we have fixed the kernel.

martinezjavier · 2017-11-17T10:11:39Z

I've proposed an RFC patch for the Linux kernel that fixes the issue. I'm not completely sure if is the correct way to handle this, but let's see what they say on the list.

martinezjavier · 2017-11-17T18:13:35Z

@flihp @williamcroberts it would be great if you can answer in the Linux patch thread with your opinions on how this issue should be solved.

williamcroberts · 2017-11-17T19:05:42Z

@martinezjavier I'm not on that Mailing List so it would be a PITA for me to jump in and i'm lazy.
I see that it's likely failing in the tpm2_find_cc() check. Why don't we just implement that in the tools and check before sending it and pick encryptdecrypt2 if supported?

williamcroberts · 2017-11-17T19:07:42Z

@martinezjavier I see you copied me on them, disregard

martinezjavier · 2017-11-20T16:24:47Z

@williamcroberts from your last message in the thread, I see that we are on the same page on this. I really don't understand Jason's argument TBH, but maybe I'm missing something.

martinezjavier · 2017-11-21T13:10:57Z

For completeness, following is the second version of the RFC kernel patch [0] that sends a synthesized command response to user-space. I didn't post it because Jarkko doesn't agree with the approach.

@flihp I think our options then are to check for rval != TPM_RC_SUCCESS in the tool as I initially suggested or make the device TCTI interpret the -EINVAL returned error as a command is not implemented an return a TPM_RC_COMMAND_CODE. I don't like neither option TBH...

[0]:

From 1c43e384c19805e94566e3acdd1852dc3c1a7fb8 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <javierm@redhat.com>
Date: Tue, 21 Nov 2017 12:32:15 +0100
Subject: [RFC v2 PATCH] tpm: return a TPM_RC_COMMAND_CODE response if command
 isn't implemented

header validation before processing and return a TPM_RC_COMMAND_CODE code
if the command is not implemented and the TPM_RC_COMMAND_SIZE code if the
command buffer size is not correct.

So user-space will expect to handle these response codes as errors, but if
the in-kernel resource manager is used (/dev/tpmrm?) then an -EINVAL errno
code is returned instead if the command isn't implemented or the buffer
size isn't correct. This confuses user-space since doesn't expect that.

This is also not consistent with the behavior when not using TPM spaces
and accessing the TPM directly (/dev/tpm?), in this case the command is
is sent to the TPM anyways and user-space can get an error from the TPM.

Instead of returning an -EINVAL errno code when the tpm_validate_command()
function fails, synthesize a TPM command response so user-space can get a
TPM_RC_COMMAND_CODE as expected when a command isn't implemented by chip.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

---

 drivers/char/tpm/tpm-interface.c | 28 ++++++++++++++++++++--------
 drivers/char/tpm/tpm.h           |  1 +
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index ebe0a1d36d8c..b8d01897c0ba 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -328,7 +328,7 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
 }
 EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
 
-static bool tpm_validate_command(struct tpm_chip *chip,
+static int tpm_validate_command(struct tpm_chip *chip,
 				 struct tpm_space *space,
 				 const u8 *cmd,
 				 size_t len)
@@ -340,10 +340,10 @@ static bool tpm_validate_command(struct tpm_chip *chip,
 	unsigned int nr_handles;
 
 	if (len < TPM_HEADER_SIZE)
-		return false;
+		return -EINVAL;
 
 	if (!space)
-		return true;
+		return 0;
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2 && chip->nr_commands) {
 		cc = be32_to_cpu(header->ordinal);
@@ -352,7 +352,7 @@ static bool tpm_validate_command(struct tpm_chip *chip,
 		if (i < 0) {
 			dev_dbg(&chip->dev, "0x%04X is an invalid command\n",
 				cc);
-			return false;
+			return -EOPNOTSUPP;
 		}
 
 		attrs = chip->cc_attrs_tbl[i];
@@ -362,11 +362,11 @@ static bool tpm_validate_command(struct tpm_chip *chip,
 			goto err_len;
 	}
 
-	return true;
+	return 0;
 err_len:
 	dev_dbg(&chip->dev,
 		"%s: insufficient command length %zu", __func__, len);
-	return false;
+	return -EINVAL;
 }
 
 /**
@@ -391,8 +391,20 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	unsigned long stop;
 	bool need_locality;
 
-	if (!tpm_validate_command(chip, space, buf, bufsiz))
-		return -EINVAL;
+	rc = tpm_validate_command(chip, space, buf, bufsiz);
+	if (rc == -EINVAL)
+		return rc;
+	/*
+	 * If the command is not implemented by the TPM, synthesize a
+	 * response with a TPM2_RC_COMMAND_CODE return for user-space.
+	 */
+	if (rc == -EOPNOTSUPP) {
+		header->length = cpu_to_be32(sizeof(*header));
+		header->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+		header->return_code = cpu_to_be32(TPM2_RC_COMMAND_CODE);
+
+		return bufsiz;
+	}
 
 	if (bufsiz > TPM_BUFSIZE)
 		bufsiz = TPM_BUFSIZE;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index c1866cc02e30..40818fa59b05 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -100,6 +100,7 @@ enum tpm2_return_codes {
 	TPM2_RC_HANDLE		= 0x008B,
 	TPM2_RC_INITIALIZE	= 0x0100, /* RC_VER1 */
 	TPM2_RC_DISABLED	= 0x0120,
+	TPM2_RC_COMMAND_CODE    = 0x0143,
 	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
 	TPM2_RC_REFERENCE_H0	= 0x0910,
 };
-- 
2.14.3

flihp · 2017-11-21T17:49:06Z

I'm not a fan of either option too. I'm going back through the tpmdd thread now. Unfortunately changing minds upstream is more work than just hacking up workarounds.

martinezjavier · 2017-11-21T18:46:38Z

@flihp indeed... and as you can see from the v2 patch I shared here, the implementation of what Jason suggested is very trivial.

I really don't understand why Jarkko wouldn't take it in order to have a consistent behavior when using either /dev/tpm* or /dev/tpmrm* chardevs.

I guess he would value your opinion more than mine (for very good reasons). So if you think that it should be fixed as suggested by Jason, it would be great if you comment in the thread. If that's the case then I may post my v2 patch to have further discussions.

flihp · 2017-11-21T23:05:18Z

Just went to comment on the list and realized I haven't been getting tpmdd emails since it's been moved / retired. Only got this thread since I was directly copied. What was tpmdd replaced with?

martinezjavier · 2017-11-21T23:13:53Z

@flihp yes, the mailing list for TPM kernel stuff now is linux-integrity@vger.kernel.org and patchwork https://patchwork.kernel.org/project/linux-integrity/list.

It's a shared mailing list with other related projects (IMA, EVM, etc) and the idea is to have a more coordinate effort between these, so be prepared to receive a lot of non-TPM related stuff.

flihp · 2017-11-22T00:13:16Z

This is a question for @tstruk

martinezjavier · 2017-11-22T00:24:04Z

@flihp I guess your last comment was meant for #640 ?

flihp · 2017-11-22T16:10:26Z

whoops, sorry about that

williamcroberts · 2017-11-22T23:12:48Z

We can just check which command is supported via getcap and then choose the right one.

martinezjavier · 2017-11-22T23:18:46Z

@williamcroberts yeah, it's just sad having to workaround what I believe is a very clear bug in the kernel.

It seems we managed to convince Jason about my RFCv2, now let's hope that Jarkko could change his mind.

martinezjavier · 2017-11-26T23:38:43Z

@williamcroberts @flihp I've just posted a new version of the patch that takes into account the feedback on the RFCv2 patch.

Please review it and answer with your Reviewed-by/Acked-by tags if you think that's correct. It seems that Jarkko is changing his mind so we may get a fix for this issue in the kernel after all.

flihp · 2017-11-28T02:37:43Z

I think the initial 'no' was just @jsakkine-intel implementing a proof of work of sorts 😄 I'm looking over your patch now. Thanks for keeping after this.

martinezjavier · 2017-11-28T03:09:05Z

@flihp great, thanks a lot for your help.

There's a bug in the tools that's exposed by that patch though. When setting the RESMGRTPM error level (or anything that's not in the low-order 12 bits), the response code won't be TPM2_RC_COMMAND_CODE anymore so the TPM2_EncryptDecrypt command won't be called.

I've fixed that in PR #652.

flihp · 2017-11-28T03:36:02Z

Yeah this makes checking RCs more complex than just using a simple equality test. Growing pains.

martinezjavier · 2017-11-28T14:18:57Z

@flihp yeah, I've also added support in the tpm2_rc_decode tool to decode response codes for the Resource Manager layer (both internal RM errors and when sending errors on behalf of the TPM) in PR #657.

williamcroberts · 2017-11-28T23:49:36Z

@martinezjavier looks like you got traction on synthesizing the tpm response from the in-kernel RM when the CC is not supported, can we safely close this as a kernel bug?

martinezjavier · 2017-11-28T23:59:08Z

@williamcroberts yes, I think it's safe to close it now.

martinezjavier · 2017-11-30T11:04:58Z

Posted today what I hope it's the final version of the kernel resource manager fix for this issue, so I'm closing this bug.

Thanks a lot @flihp @williamcroberts and @jsakkine-intel for your help wit this!

martinezjavier · 2017-12-07T21:12:25Z

FYI, the fix for this was merged in the kernel.

martinezjavier closed this as completed Nov 30, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Encrypting data with tpm2_encryptdecrypt is failing on an Intel PTT TPM2.0 #621

Encrypting data with tpm2_encryptdecrypt is failing on an Intel PTT TPM2.0 #621

martinezjavier commented Nov 15, 2017 •

edited

Loading

flihp commented Nov 15, 2017

martinezjavier commented Nov 15, 2017

martinezjavier commented Nov 15, 2017

flihp commented Nov 16, 2017

martinezjavier commented Nov 16, 2017

martinezjavier commented Nov 16, 2017 •

edited

Loading

martinezjavier commented Nov 16, 2017

martinezjavier commented Nov 16, 2017 •

edited

Loading

martinezjavier commented Nov 17, 2017

martinezjavier commented Nov 17, 2017

williamcroberts commented Nov 17, 2017

williamcroberts commented Nov 17, 2017

martinezjavier commented Nov 20, 2017

martinezjavier commented Nov 21, 2017

flihp commented Nov 21, 2017

martinezjavier commented Nov 21, 2017

flihp commented Nov 21, 2017

martinezjavier commented Nov 21, 2017

flihp commented Nov 22, 2017

martinezjavier commented Nov 22, 2017

flihp commented Nov 22, 2017

williamcroberts commented Nov 22, 2017

martinezjavier commented Nov 22, 2017

martinezjavier commented Nov 26, 2017

flihp commented Nov 28, 2017

martinezjavier commented Nov 28, 2017

flihp commented Nov 28, 2017

martinezjavier commented Nov 28, 2017 •

edited

Loading

williamcroberts commented Nov 28, 2017

martinezjavier commented Nov 28, 2017

martinezjavier commented Nov 30, 2017

martinezjavier commented Dec 7, 2017

Encrypting data with tpm2_encryptdecrypt is failing on an Intel PTT TPM2.0 #621

Encrypting data with tpm2_encryptdecrypt is failing on an Intel PTT TPM2.0 #621

Comments

martinezjavier commented Nov 15, 2017 • edited Loading

flihp commented Nov 15, 2017

martinezjavier commented Nov 15, 2017

martinezjavier commented Nov 15, 2017

flihp commented Nov 16, 2017

martinezjavier commented Nov 16, 2017

martinezjavier commented Nov 16, 2017 • edited Loading

martinezjavier commented Nov 16, 2017

martinezjavier commented Nov 16, 2017 • edited Loading

martinezjavier commented Nov 17, 2017

martinezjavier commented Nov 17, 2017

williamcroberts commented Nov 17, 2017

williamcroberts commented Nov 17, 2017

martinezjavier commented Nov 20, 2017

martinezjavier commented Nov 21, 2017

flihp commented Nov 21, 2017

martinezjavier commented Nov 21, 2017

flihp commented Nov 21, 2017

martinezjavier commented Nov 21, 2017

flihp commented Nov 22, 2017

martinezjavier commented Nov 22, 2017

flihp commented Nov 22, 2017

williamcroberts commented Nov 22, 2017

martinezjavier commented Nov 22, 2017

martinezjavier commented Nov 26, 2017

flihp commented Nov 28, 2017

martinezjavier commented Nov 28, 2017

flihp commented Nov 28, 2017

martinezjavier commented Nov 28, 2017 • edited Loading

williamcroberts commented Nov 28, 2017

martinezjavier commented Nov 28, 2017

martinezjavier commented Nov 30, 2017

martinezjavier commented Dec 7, 2017

martinezjavier commented Nov 15, 2017 •

edited

Loading

martinezjavier commented Nov 16, 2017 •

edited

Loading

martinezjavier commented Nov 16, 2017 •

edited

Loading

martinezjavier commented Nov 28, 2017 •

edited

Loading