Date   
[PATCH v3] Truncate hashes for ECDSA signing, to allow SHA512 to work

David Woodhouse
 

The TPM has no business knowing what hash we were using. ECDSA signatures
truncate the digest to the size of the curve anyway. So just pick any
hash between the curve size and the actual size of the digest, and tell
the TPM it's that. Then we don't care that most TPMs don't support SHA512.

Signed-off-by: David Woodhouse <dwmw2@...>
---
e_tpm2-ecc.c | 74 +++++++++++++++++++++++++++++++++++++--------
tests/create_ecc.sh | 2 +-
2 files changed, 62 insertions(+), 14 deletions(-)

diff --git a/e_tpm2-ecc.c b/e_tpm2-ecc.c
index 21a636c..2cbb3aa 100644
--- a/e_tpm2-ecc.c
+++ b/e_tpm2-ecc.c
@@ -123,6 +123,33 @@ static void tpm2_ecc_free(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
tpm2_delete(data);
}

+#if OPENSSL_VERSION_NUMBER < 0x10100000
+static int EC_GROUP_order_bits(const EC_GROUP *group)
+{
+ if (!group)
+ return 0;
+
+ BIGNUM *order = BN_new();
+
+ if (order == NULL) {
+ ERR_clear_error();
+ return 0;
+ }
+
+ int ret = 0;
+
+ if (!EC_GROUP_get_order(group, order, NULL)) {
+ ERR_clear_error();
+ BN_free(order);
+ return 0;
+ }
+
+ ret = BN_num_bits(order);
+ BN_free(order);
+ return ret;
+}
+#endif
+
static ECDSA_SIG *tpm2_ecdsa_sign(const unsigned char *dgst, int dgst_len,
const BIGNUM *kinv, const BIGNUM *rp,
EC_KEY *eck)
@@ -139,25 +166,46 @@ static ECDSA_SIG *tpm2_ecdsa_sign(const unsigned char *dgst, int dgst_len,
int num_commands;
struct policy_command *commands;
TPM_ALG_ID nameAlg;
-
- /* The TPM insists on knowing the digest type, so
- * calculate that from the size */
- switch (dgst_len) {
- case SHA_DIGEST_LENGTH:
+ int curve_len;
+
+ /*
+ * ECDSA signatures truncate the incoming hash to fit the curve,
+ * and the signature mechanism is the same regardless of the
+ * hash being used.
+ *
+ * The TPM bizarrely wants to be told the hash algorithm, and
+ * either it or the TSS will validate that the digest length
+ * matches the hash that it's told, despite it having no business
+ * caring about such things.
+ *
+ * So, we can truncate the digest and pretend it's any smaller
+ * digest that the TPM actually does support, as long as that
+ * digest is larger than the size of the curve.
+ */
+ curve_len = (EC_GROUP_order_bits(EC_KEY_get0_group(eck)) + 7) / 8;
+ /* If we couldn't work it out, don't truncate */
+ if (!curve_len)
+ curve_len = dgst_len;
+
+ if (dgst_len == SHA_DIGEST_LENGTH ||
+ (curve_len <= SHA_DIGEST_LENGTH && dgst_len > SHA_DIGEST_LENGTH)) {
in.inScheme.details.ecdsa.hashAlg = TPM_ALG_SHA1;
- break;
- case SHA256_DIGEST_LENGTH:
+ dgst_len = SHA_DIGEST_LENGTH;
+ } else if (dgst_len == SHA256_DIGEST_LENGTH ||
+ (curve_len <= SHA256_DIGEST_LENGTH && dgst_len > SHA256_DIGEST_LENGTH)) {
in.inScheme.details.ecdsa.hashAlg = TPM_ALG_SHA256;
- break;
- case SHA384_DIGEST_LENGTH:
+ dgst_len = SHA256_DIGEST_LENGTH;
+ } else if (dgst_len == SHA384_DIGEST_LENGTH ||
+ (curve_len <= SHA384_DIGEST_LENGTH && dgst_len > SHA384_DIGEST_LENGTH)) {
in.inScheme.details.ecdsa.hashAlg = TPM_ALG_SHA384;
- break;
+ dgst_len = SHA384_DIGEST_LENGTH;
#ifdef TPM_ALG_SHA512
- case SHA512_DIGEST_LENGTH:
+ } else if (dgst_len == SHA512_DIGEST_LENGTH ||
+ (curve_len <= SHA512_DIGEST_LENGTH && dgst_len > SHA512_DIGEST_LENGTH)) {
in.inScheme.details.ecdsa.hashAlg = TPM_ALG_SHA512;
- break;
+ dgst_len = SHA512_DIGEST_LENGTH;
#endif
- default:
+ } else {
printf("ECDSA signature: Unknown digest length, cannot deduce hash type for TPM\n");
return NULL;
}
diff --git a/tests/create_ecc.sh b/tests/create_ecc.sh
index 061cedb..092c743 100755
--- a/tests/create_ecc.sh
+++ b/tests/create_ecc.sh
@@ -14,7 +14,7 @@ for curve in $(${bindir}/create_tpm2_key --list-curves); do
echo "Checking curve ${curve}"
${bindir}/create_tpm2_key -p 81000001 --ecc ${curve} key.tpm || \
exit 1
- for hash in sha1 sha256 sha384; do
+ for hash in sha1 sha256 sha384 sha512; do
openssl req -new -x509 -${hash} -subj '/CN=test/' -key key.tpm -engine tpm2 -keyform engine -out tmp.crt && \
openssl verify -CAfile tmp.crt -check_ss_sig tmp.crt || \
exit 1

Re: [PATCH v2] Truncate hashes for ECDSA signing, to allow SHA512 to work

David Woodhouse
 

On Sat, 2019-08-24 at 11:00 +0100, David Woodhouse wrote:
On Sat, 2019-08-24 at 10:03 +0100, James Bottomley wrote:
The TPM wants to do the digest truncation itself, so this will fail if
the curvebits converted to bytes aren't a recognized digest size.
Of the currently listed curves in the TCG registry, this will happen
for NIST_P192 and NIST_P224. I don't believe anyone has actually
implemented a TPM with these curves, but I think I'd like us to be a
bit more robust than the above just in case a future expansion causes
this problem.

What about probing the accepted hashes, then taking the digest verbatim
if it matches an accepted size regardless of curve, then trying to
truncate to the curve size if that fails? That should allow sha512 to
work on a TPM which doesn't support it using any of the 256 bit curves.
Actually, it's not the curve size that's important. We should probably
just truncate to another digest size.

To sign SHA512 I'm truncating it and telling the TPM it's a SHA256.

That would work for *any* curve of up to 256 bits; not just curves of
precisely 256 bits.

We should probably probe for the supported hashes, and if offered a
digest which doesn't match then we could truncate to the length of any
other supported hash which is larger than the curve bit length.
Something like this...
https://github.com/tpm2-software/tpm2-tss-engine/pull/139/commits/c50e7db6691bf8f2aa7244ed77378fef7beb1610

If you want to make it probe for the digests that are actually
supported, that would be even better perhaps.

Re: [PATCH v2] Truncate hashes for ECDSA signing, to allow SHA512 to work

David Woodhouse
 

On Sat, 2019-08-24 at 10:03 +0100, James Bottomley wrote:
The TPM wants to do the digest truncation itself, so this will fail if
the curvebits converted to bytes aren't a recognized digest size.
Of the currently listed curves in the TCG registry, this will happen
for NIST_P192 and NIST_P224. I don't believe anyone has actually
implemented a TPM with these curves, but I think I'd like us to be a
bit more robust than the above just in case a future expansion causes
this problem.

What about probing the accepted hashes, then taking the digest verbatim
if it matches an accepted size regardless of curve, then trying to
truncate to the curve size if that fails? That should allow sha512 to
work on a TPM which doesn't support it using any of the 256 bit curves.
Actually, it's not the curve size that's important. We should probably
just truncate to another digest size.

To sign SHA512 I'm truncating it and telling the TPM it's a SHA256.

That would work for *any* curve of up to 256 bits; not just curves of
precisely 256 bits.

We should probably probe for the supported hashes, and if offered a
digest which doesn't match then we could truncate to the length of any
other supported hash which is larger than the curve bit length.

Re: [PATCH v2] Truncate hashes for ECDSA signing, to allow SHA512 to work

James Bottomley
 

On Fri, 2019-08-23 at 16:50 +0100, David Woodhouse wrote:
It does mean that we are lying to the TPM about what the hash is, but
since it had no business knowing that anyway, it doesn't matter.

Signed-off-by: David Woodhouse <dwmw2@...>
---
v2: Fix truncation for curves which aren't a multiple of 8 bits.

The fact that the TPM pointlessly demands to know the hash algorithm,
and that something appears to be validating the length of the digest
against that algorithm, is still a theoretical problem. But at least
this part is correct and I'm not making it worse.

cf. https://github.com/openssl/openssl/pull/9680

e_tpm2-ecc.c | 10 ++++++++++
tests/create_ecc.sh | 2 +-
2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/e_tpm2-ecc.c b/e_tpm2-ecc.c
index 21a636c..9b0be7f 100644
--- a/e_tpm2-ecc.c
+++ b/e_tpm2-ecc.c
@@ -139,6 +139,16 @@ static ECDSA_SIG *tpm2_ecdsa_sign(const unsigned
char *dgst, int dgst_len,
int num_commands;
struct policy_command *commands;
TPM_ALG_ID nameAlg;
+ int curvebits;
+
+ /* FIPS-186-4 §6.4 says "When the length of the output of
the hash
+ * function is greater than the bit length of n, then the
leftmost
+ * n bits of the hash function output block shall be used in
any
+ * calculation using the hash function output during the
generation
+ * or verification of a digital signature." */
+ curvebits = EC_GROUP_order_bits(EC_KEY_get0_group(eck));
+ if (curvebits && dgst_len > (curvebits + 7) / 8)
+ dgst_len = (curvebits + 7) / 8;
The TPM wants to do the digest truncation itself, so this will fail if
the curvebits converted to bytes aren't a recognized digest size.

Of the currently listed curves in the TCG registry, this will happen
for NIST_P192 and NIST_P224. I don't believe anyone has actually
implemented a TPM with these curves, but I think I'd like us to be a
bit more robust than the above just in case a future expansion causes
this problem.

What about probing the accepted hashes, then taking the digest verbatim
if it matches an accepted size regardless of curve, then trying to
truncate to the curve size if that fails? That should allow sha512 to
work on a TPM which doesn't support it using any of the 256 bit curves.

James

[PATCH v2] Truncate hashes for ECDSA signing, to allow SHA512 to work

David Woodhouse
 

It does mean that we are lying to the TPM about what the hash is, but
since it had no business knowing that anyway, it doesn't matter.

Signed-off-by: David Woodhouse <dwmw2@...>
---
v2: Fix truncation for curves which aren't a multiple of 8 bits.

The fact that the TPM pointlessly demands to know the hash algorithm,
and that something appears to be validating the length of the digest
against that algorithm, is still a theoretical problem. But at least
this part is correct and I'm not making it worse.

cf. https://github.com/openssl/openssl/pull/9680

e_tpm2-ecc.c | 10 ++++++++++
tests/create_ecc.sh | 2 +-
2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/e_tpm2-ecc.c b/e_tpm2-ecc.c
index 21a636c..9b0be7f 100644
--- a/e_tpm2-ecc.c
+++ b/e_tpm2-ecc.c
@@ -139,6 +139,16 @@ static ECDSA_SIG *tpm2_ecdsa_sign(const unsigned char *dgst, int dgst_len,
int num_commands;
struct policy_command *commands;
TPM_ALG_ID nameAlg;
+ int curvebits;
+
+ /* FIPS-186-4 §6.4 says "When the length of the output of the hash
+ * function is greater than the bit length of n, then the leftmost
+ * n bits of the hash function output block shall be used in any
+ * calculation using the hash function output during the generation
+ * or verification of a digital signature." */
+ curvebits = EC_GROUP_order_bits(EC_KEY_get0_group(eck));
+ if (curvebits && dgst_len > (curvebits + 7) / 8)
+ dgst_len = (curvebits + 7) / 8;

/* The TPM insists on knowing the digest type, so
* calculate that from the size */
diff --git a/tests/create_ecc.sh b/tests/create_ecc.sh
index 061cedb..092c743 100755
--- a/tests/create_ecc.sh
+++ b/tests/create_ecc.sh
@@ -14,7 +14,7 @@ for curve in $(${bindir}/create_tpm2_key --list-curves); do
echo "Checking curve ${curve}"
${bindir}/create_tpm2_key -p 81000001 --ecc ${curve} key.tpm || \
exit 1
- for hash in sha1 sha256 sha384; do
+ for hash in sha1 sha256 sha384 sha512; do
openssl req -new -x509 -${hash} -subj '/CN=test/' -key key.tpm -engine tpm2 -keyform engine -out tmp.crt && \
openssl verify -CAfile tmp.crt -check_ss_sig tmp.crt || \
exit 1

[PATCH] Truncate hashes for ECDSA signing, to allow SHA512 to work

David Woodhouse
 

It does mean that we are lying to the TPM about what the hash is, but
since it had no business knowing that anyway, it doesn't matter.

This fixes the problems which occur when OpenSSL decides to use SHA512
for CertificateVerify.

Signed-off-by: David Woodhouse <dwmw2@...>
---
e_tpm2-ecc.c | 10 ++++++++++
tests/create_ecc.sh | 2 +-
2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/e_tpm2-ecc.c b/e_tpm2-ecc.c
index 21a636c..e622180 100644
--- a/e_tpm2-ecc.c
+++ b/e_tpm2-ecc.c
@@ -139,6 +139,16 @@ static ECDSA_SIG *tpm2_ecdsa_sign(const unsigned char *dgst, int dgst_len,
int num_commands;
struct policy_command *commands;
TPM_ALG_ID nameAlg;
+ int curvebits;
+
+ /* FIPS-186-4 §6.4 says "When the length of the output of the hash
+ * function is greater than the bit length of n, then the leftmost
+ * n bits of the hash function output block shall be used in any
+ * calculation using the hash function output during the generation
+ * or verification of a digital signature." */
+ curvebits = EC_GROUP_order_bits(EC_KEY_get0_group(eck));
+ if (curvebits && dgst_len > curvebits / 8)
+ dgst_len = curvebits / 8;

/* The TPM insists on knowing the digest type, so
* calculate that from the size */
diff --git a/tests/create_ecc.sh b/tests/create_ecc.sh
index 061cedb..092c743 100755
--- a/tests/create_ecc.sh
+++ b/tests/create_ecc.sh
@@ -14,7 +14,7 @@ for curve in $(${bindir}/create_tpm2_key --list-curves); do
echo "Checking curve ${curve}"
${bindir}/create_tpm2_key -p 81000001 --ecc ${curve} key.tpm || \
exit 1
- for hash in sha1 sha256 sha384; do
+ for hash in sha1 sha256 sha384 sha512; do
openssl req -new -x509 -${hash} -subj '/CN=test/' -key key.tpm -engine tpm2 -keyform engine -out tmp.crt && \
openssl verify -CAfile tmp.crt -check_ss_sig tmp.crt || \
exit 1

Verification via public key fails...

jvert
 

Hi,

I'm able to sign using the following:

$ openssl dgst -engine tpm2 -keyform engine -sha256 -sign //nvkey:81800001 xxx >xxx.sig
engine "tpm2" set.
$ xxd xxx.sig
00000000: 3044 0220 5444 ae8c 36a1 c308 ef65 8f06 0D. TD..6....e..
00000010: e554 0e1a f136 948e eeed a6ab 5a74 c4e3 .T...6......Zt..
00000020: a717 ccac 0220 2e29 f92c 9cb3 733b b295 ..... .).,..s;..
00000030: d9cf 6fe8 b17e b1dc 00cd 3c92 fe1f d6ec ..o..~....<.....
00000040: 0b47 0448 0178 .G.H.x

What’s curious is that I’m not able to verify in a symmetric manner using the engine and the nvkey moniker:

$ openssl dgst -engine tpm2 -keyform engine -verify //nvkey:81800001 -signature xxx.sig xxx
engine "tpm2" set.
cannot load key file from engine
140075721995456:error:26097075:engine routines:ENGINE_load_public_key:not initialised:../crypto/engine/eng_pkey.c:97:
unable to load key file

I _am_ able to verify using -prverify, so it would seem that using the public key via the nvkey moniker doesn't work.

Thanks.

Re: Installing openssl_tpm2_engine-2.3.0...

James Bottomley
 

[adding the list because we actually have one now]

On Wed, 2019-05-29 at 18:48 +0000, Sievert, James wrote:
Hi James,

I've configured openssl_tpm2_engine-2.3.0<https://git.kernel.org/pub/
scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/snapshot/openssl_tp
m2_engine-2.3.0.tar.gz> as follows:

CFLAGS=-I../rootfs/include LDFLAGS="-L../rootfs/lib" ./configure --
prefix=$(pwd)/../rootfs

I'm finding that "make install" doesn't abide by the --prefix option:

ubuntu@ip-10-132-42-78:~/platforms/vss/CS/Tempest/src/packages/src/Op
ensslTpm2EnginePackage/openssl_tpm2_engine-2.3.0$ make install
Making install in tests
make[1]: Entering directory
'/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2En
ginePackage/openssl_tpm2_engine-2.3.0/tests'
make[2]: Entering directory
'/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2En
ginePackage/openssl_tpm2_engine-2.3.0/tests'
make[2]: Nothing to be done for 'install-exec-am'.
make[2]: Nothing to be done for 'install-data-am'.
make[2]: Leaving directory
'/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2En
ginePackage/openssl_tpm2_engine-2.3.0/tests'
make[1]: Leaving directory
'/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2En
ginePackage/openssl_tpm2_engine-2.3.0/tests'
make[1]: Entering directory
'/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2En
ginePackage/openssl_tpm2_engine-2.3.0'
make[2]: Entering directory
'/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2En
ginePackage/openssl_tpm2_engine-2.3.0'
/bin/mkdir -p
'/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2En
ginePackage/openssl_tpm2_engine-2.3.0/../rootfs/bin'
/bin/bash ./libtool --mode=install /usr/bin/install -c
create_tpm2_key load_tpm2_key
'/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2En
ginePackage/openssl_tpm2_engine-2.3.0/../rootfs/bin'
libtool: install: /usr/bin/install -c create_tpm2_key
/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2Eng
inePackage/openssl_tpm2_engine-2.3.0/../rootfs/bin/create_tpm2_key
libtool: install: /usr/bin/install -c load_tpm2_key
/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2Eng
inePackage/openssl_tpm2_engine-2.3.0/../rootfs/bin/load_tpm2_key
/bin/mkdir -p
'/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2En
ginePackage/openssl_tpm2_engine-2.3.0/../rootfs/share/man/man1'
/usr/bin/install -c -m 644 create_tpm2_key.1 load_tpm2_key.1
'/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2En
ginePackage/openssl_tpm2_engine-2.3.0/../rootfs/share/man/man1'
/bin/mkdir -p '/usr/lib/x86_64-linux-gnu/engines-1.1'
/bin/bash ./libtool --mode=install /usr/bin/install -c libtpm2.la
'/usr/lib/x86_64-linux-gnu/engines-1.1'
libtool: install: /usr/bin/install -c .libs/libtpm2.so
/usr/lib/x86_64-linux-gnu/engines-1.1/libtpm2.so
/usr/bin/install: cannot create regular file '/usr/lib/x86_64-linux-
gnu/engines-1.1/libtpm2.so': Permission denied
Makefile:474: recipe for target 'install-openssl_engineLTLIBRARIES'
failed
make[2]: *** [install-openssl_engineLTLIBRARIES] Error 1
make[2]: Leaving directory
'/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2En
ginePackage/openssl_tpm2_engine-2.3.0'
Makefile:1046: recipe for target 'install-am' failed
make[1]: *** [install-am] Error 2
make[1]: Leaving directory
'/home/ubuntu/platforms/vss/CS/Tempest/src/packages/src/OpensslTpm2En
ginePackage/openssl_tpm2_engine-2.3.0'
Makefile:745: recipe for target 'install-recursive' failed
make: *** [install-recursive] Error 1
Got to confess, this isn't something I've tried directly (so I will
test it out and see what the problem is). However, both the openSUSE
and debian packages which are built from this, use the --prefix option
in some form and they all seem to install correctly, so there's
something different about the way you did it that I'll have to figure
out.

James

Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Jerry Snitselaar <jsnitsel@...>
 

On Mon Mar 25 19, Doug Fraser wrote:

Jerry,

I captured the following out of order. You added line is the first emitted at the time of the fault.

/var/log/messages:Mar 25 13:28:27 C05BUB0000A000004124 kern.info kernel: [ 1461.843825] tpm tpm0: tpm_tis_send_data: TPM_STS_DATA_EXPECT == 0: locality: 0 status: ff access: 81
Yes, that was what was being seen before with the registers. The
access register says it has a valid status and it is saying the
locality is not active. I've been dragged into another issue, so I
haven't been able to deal with backporting the patch to 4.14. It
should be straightforward, the only difference that I think needs to
be dealt with is the fact the release_locality has a void return
instead of int. I will hopefully get to it in the next couple of days.


/var/log/messages:Mar 25 13:28:27 C05BUB0000A000004124 kern.err kernel: [ 1461.858321] tpm tpm0: tpm_try_transmit: tpm_send: error -5
/var/log/messages:Mar 25 13:28:27 C05BUB0000A000004124 kern.err kernel: [ 1461.865369] tpm tpm0: A TPM error (357) occurred flushing context

Note also that the 4.18 TPM pull into my 4.14 kernel continues to perform well.

Doug

Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Doug Fraser
 

Jerry,

I captured the following out of order. You added line is the first emitted at the time of the fault.

/var/log/messages:Mar 25 13:28:27 C05BUB0000A000004124 kern.info kernel: [ 1461.843825] tpm tpm0: tpm_tis_send_data: TPM_STS_DATA_EXPECT == 0: locality: 0 status: ff access: 81


/var/log/messages:Mar 25 13:28:27 C05BUB0000A000004124 kern.err kernel: [ 1461.858321] tpm tpm0: tpm_try_transmit: tpm_send: error -5
/var/log/messages:Mar 25 13:28:27 C05BUB0000A000004124 kern.err kernel: [ 1461.865369] tpm tpm0: A TPM error (357) occurred flushing context

Note also that the 4.18 TPM pull into my 4.14 kernel continues to perform well.

Doug

Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Jerry Snitselaar <jsnitsel@...>
 

On Wed Mar 20 19, Doug Fraser wrote:
Jerry, if there is still some information you would like me to gather from this at the low level, let me know.

I don't know how many people are still on 4.14, but if you do back port, it will be a great help to them.
I was a little leery of the wholesale replacement with the 4.18 TPM, but I found that trying to cherry pick your changes into the 4.14 base was not as easy as I had hoped, given the difference in the underpinnings, and the rip and replace went more easily.

I tested it all day today while doing other work. Running one particular multi-thread test against the openssl engine that was resulting in sporadic timeouts at the engine layers about once every three seconds for an hour. Throughout that test, not a single low level driver error. Not one.

Then when I brought it back down to a single thread, all the timeouts ceased, and it went on its merry way.

All good.

Doug

-----Original Message-----
From: Doug Fraser
Sent: Wednesday, March 20, 2019 10:57 AM
To: 'Jerry Snitselaar' <jsnitsel@...>
Cc: James Bottomley <James.Bottomley@...>; openssl-tpm2-engine@groups.io; Kenneth Goldman <kgoldman@...>; Ibmtpm20tss-users@...
Subject: RE: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Hello All,

Last night I completely pulled 4.18 TPM into 4.14 base that we are running.

This required a couple file changes up in include/linux as well as pulling the tpm_hwrng compilation from:

linux/drivers/char/hw_random/Makefile

I need to test to see if the hw_rng (which is now instantiated in the TPM driver) is still functioning properly.

I now longer get the '-5' errors from tpm_send()

Totally gone.

Also, If I pound on the openssl engine really really really hard (really hard) I get timeouts, but the chip is just fine, and if I halt the test load, and restart with a single thread, it comes up fine. On the old driver code base (from 4.14) after a couple tpm_send() failures, the device was toast, and I had to unload, strobe the reset line, and reload the tpm driver to make it usable again. That failure mode is now longer occurring.

So pulling in 4.18 TPM took a little effort, but I am getting an error free build that brings up the TPM and is not throwing the tpm_send errors that it was throwing before.

I have a giant diff file that performs all the work on a 4.14 base to bring it up, so I can just run it as a patch against a 4.14 pull and it all builds.

Thanks to all for your support and feedback.
A special shout out to Jerry for picking up on that patch.

Doug

-----Original Message-----
From: Jerry Snitselaar <jsnitsel@...>
Sent: Tuesday, March 19, 2019 1:31 PM
To: Doug Fraser <doug.fraser@...>
Cc: James Bottomley <James.Bottomley@...>; openssl-tpm2-engine@groups.io; Kenneth Goldman <kgoldman@...>; Ibmtpm20tss-users@...
Subject: Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

On Tue Mar 19 19, Doug Fraser wrote:
Jerry,

We are on 4.14.77

I will look at cherry picking the tpm from 4.18
I'll take a stab at backporting the commit to 4.14 this afternoon. There are some minor differences back then, but it shouldn't too bad. Since it sounds like you are building the kernel, I can also send along a debugging patch that will spit out the values in the access and status registers when the status expect data check fails.

Regards,
Jerry


We are currently on Alpine 3.8 hoping to move to 3.9 (for other reasons) and also looking to move to 4.19 kernel.
This is going to take some time, but now I have a greater incentive to push on that.

Thank you all for your help in this.

Doug
-client communications.
Hi Doug,

Here is a debug patch against the 4.14.y rpi branch to dump the access and status registers when
the status data expect checks fail. I'm starting on backporting the release locality patch now.

--8<--

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 63bc6c3b949e..7c0394765aea 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
int rc, status, burstcnt;
size_t count = 0;
bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
+ u8 access;

status = tpm_tis_status(chip);
if ((status & TPM_STS_COMMAND_READY) == 0) {
@@ -292,6 +293,13 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
}
status = tpm_tis_status(chip);
if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
+ rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access);
+ if (rc < 0)
+ dev_info(&chip->dev, "%s: TPM_STS_DATA_EXPECT == 0: read failure TPM_ACCESS(%d)\n",
+ __func__, priv->locality);
+ else
+ dev_info(&chip->dev, "%s: TPM_STS_DATA_EXPECT == 0: locality: %d status: %x access: %x\n",
+ __func__, priv->locality, status, access);
rc = -EIO;
goto out_err;
}
@@ -309,6 +317,13 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
}
status = tpm_tis_status(chip);
if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) {
+ rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access);
+ if (rc < 0)
+ dev_info(&chip->dev, "%s: TPM_STS_DATA_EXPECT != 0: read failure TPM_ACCESS(%d)\n",
+ __func__, priv->locality);
+ else
+ dev_info(&chip->dev, "%s: TPM_STS_DATA_EXPECT != 0: locality: %d status: %x access: %x\n",
+ __func__, priv->locality, status, access);
rc = -EIO;
goto out_err;
}
--
2.21.0

Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Doug Fraser
 

Gave your patch a shot against my 4.14 (with no other patches)
Last line is your addition.

/var/log/messages:Mar 25 13:28:27 C05BUB0000A000004124 kern.err kernel: [ 1461.858321] tpm tpm0: tpm_try_transmit: tpm_send: error -5
/var/log/messages:Mar 25 13:28:27 C05BUB0000A000004124 kern.err kernel: [ 1461.865369] tpm tpm0: A TPM error (357) occurred flushing context
/var/log/messages:Mar 25 13:28:27 C05BUB0000A000004124 kern.info kernel: [ 1461.873871] tpm tpm0: tpm_tis_send_data: TPM_STS_DATA_EXPECT == 0: locality: 0 status: ff access: 81

-----Original Message-----
From: Jerry Snitselaar <jsnitsel@...>
Sent: Wednesday, March 20, 2019 4:37 PM
To: Doug Fraser <doug.fraser@...>
Cc: James Bottomley <James.Bottomley@...>; openssl-tpm2-engine@groups.io; Kenneth Goldman <kgoldman@...>; Ibmtpm20tss-users@...
Subject: Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

On Wed Mar 20 19, Doug Fraser wrote:
Jerry, if there is still some information you would like me to gather from this at the low level, let me know.

I don't know how many people are still on 4.14, but if you do back port, it will be a great help to them.
I was a little leery of the wholesale replacement with the 4.18 TPM, but I found that trying to cherry pick your changes into the 4.14 base was not as easy as I had hoped, given the difference in the underpinnings, and the rip and replace went more easily.

I tested it all day today while doing other work. Running one particular multi-thread test against the openssl engine that was resulting in sporadic timeouts at the engine layers about once every three seconds for an hour. Throughout that test, not a single low level driver error. Not one.

Then when I brought it back down to a single thread, all the timeouts ceased, and it went on its merry way.

All good.

Doug

-----Original Message-----
From: Doug Fraser
Sent: Wednesday, March 20, 2019 10:57 AM
To: 'Jerry Snitselaar' <jsnitsel@...>
Cc: James Bottomley <James.Bottomley@...>;
openssl-tpm2-engine@groups.io; Kenneth Goldman <kgoldman@...>;
Ibmtpm20tss-users@...
Subject: RE: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Hello All,

Last night I completely pulled 4.18 TPM into 4.14 base that we are running.

This required a couple file changes up in include/linux as well as pulling the tpm_hwrng compilation from:

linux/drivers/char/hw_random/Makefile

I need to test to see if the hw_rng (which is now instantiated in the TPM driver) is still functioning properly.

I now longer get the '-5' errors from tpm_send()

Totally gone.

Also, If I pound on the openssl engine really really really hard (really hard) I get timeouts, but the chip is just fine, and if I halt the test load, and restart with a single thread, it comes up fine. On the old driver code base (from 4.14) after a couple tpm_send() failures, the device was toast, and I had to unload, strobe the reset line, and reload the tpm driver to make it usable again. That failure mode is now longer occurring.

So pulling in 4.18 TPM took a little effort, but I am getting an error free build that brings up the TPM and is not throwing the tpm_send errors that it was throwing before.

I have a giant diff file that performs all the work on a 4.14 base to bring it up, so I can just run it as a patch against a 4.14 pull and it all builds.

Thanks to all for your support and feedback.
A special shout out to Jerry for picking up on that patch.

Doug

-----Original Message-----
From: Jerry Snitselaar <jsnitsel@...>
Sent: Tuesday, March 19, 2019 1:31 PM
To: Doug Fraser <doug.fraser@...>
Cc: James Bottomley <James.Bottomley@...>;
openssl-tpm2-engine@groups.io; Kenneth Goldman <kgoldman@...>;
Ibmtpm20tss-users@...
Subject: Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

On Tue Mar 19 19, Doug Fraser wrote:
Jerry,

We are on 4.14.77

I will look at cherry picking the tpm from 4.18
I'll take a stab at backporting the commit to 4.14 this afternoon. There are some minor differences back then, but it shouldn't too bad. Since it sounds like you are building the kernel, I can also send along a debugging patch that will spit out the values in the access and status registers when the status expect data check fails.

Regards,
Jerry


We are currently on Alpine 3.8 hoping to move to 3.9 (for other reasons) and also looking to move to 4.19 kernel.
This is going to take some time, but now I have a greater incentive to push on that.

Thank you all for your help in this.

Doug
-client communications.
Hi Doug,

Here is a debug patch against the 4.14.y rpi branch to dump the access and status registers when the status data expect checks fail. I'm starting on backporting the release locality patch now.

--8<--

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index 63bc6c3b949e..7c0394765aea 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
int rc, status, burstcnt;
size_t count = 0;
bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
+ u8 access;

status = tpm_tis_status(chip);
if ((status & TPM_STS_COMMAND_READY) == 0) { @@ -292,6 +293,13 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
}
status = tpm_tis_status(chip);
if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
+ rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access);
+ if (rc < 0)
+ dev_info(&chip->dev, "%s: TPM_STS_DATA_EXPECT == 0: read failure TPM_ACCESS(%d)\n",
+ __func__, priv->locality);
+ else
+ dev_info(&chip->dev, "%s: TPM_STS_DATA_EXPECT == 0: locality: %d status: %x access: %x\n",
+ __func__, priv->locality, status, access);
rc = -EIO;
goto out_err;
}
@@ -309,6 +317,13 @@ static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
}
status = tpm_tis_status(chip);
if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) {
+ rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access);
+ if (rc < 0)
+ dev_info(&chip->dev, "%s: TPM_STS_DATA_EXPECT != 0: read failure TPM_ACCESS(%d)\n",
+ __func__, priv->locality);
+ else
+ dev_info(&chip->dev, "%s: TPM_STS_DATA_EXPECT != 0: locality: %d status: %x access: %x\n",
+ __func__, priv->locality, status, access);
rc = -EIO;
goto out_err;
}
--
2.21.0


This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient:(a) any dissemination or copying of this message is strictly prohibited; and (b) immediately notify the sender by return message and destroy any copies of this message in any form (electronic, paper or otherwise) that you have. The delivery of this message and its information is neither intended to be nor constitutes a disclosure or waiver of any trade secrets, intellectual property, attorney work product, or attorney-client communications.

Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Doug Fraser
 

Jerry, if there is still some information you would like me to gather from this at the low level, let me know.

I don't know how many people are still on 4.14, but if you do back port, it will be a great help to them.
I was a little leery of the wholesale replacement with the 4.18 TPM, but I found that trying to cherry pick your changes into the 4.14 base was not as easy as I had hoped, given the difference in the underpinnings, and the rip and replace went more easily.

I tested it all day today while doing other work. Running one particular multi-thread test against the openssl engine that was resulting in sporadic timeouts at the engine layers about once every three seconds for an hour. Throughout that test, not a single low level driver error. Not one.

Then when I brought it back down to a single thread, all the timeouts ceased, and it went on its merry way.

All good.

Doug

-----Original Message-----
From: Doug Fraser
Sent: Wednesday, March 20, 2019 10:57 AM
To: 'Jerry Snitselaar' <jsnitsel@...>
Cc: James Bottomley <James.Bottomley@...>; openssl-tpm2-engine@groups.io; Kenneth Goldman <kgoldman@...>; Ibmtpm20tss-users@...
Subject: RE: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Hello All,

Last night I completely pulled 4.18 TPM into 4.14 base that we are running.

This required a couple file changes up in include/linux as well as pulling the tpm_hwrng compilation from:

linux/drivers/char/hw_random/Makefile

I need to test to see if the hw_rng (which is now instantiated in the TPM driver) is still functioning properly.

I now longer get the '-5' errors from tpm_send()

Totally gone.

Also, If I pound on the openssl engine really really really hard (really hard) I get timeouts, but the chip is just fine, and if I halt the test load, and restart with a single thread, it comes up fine. On the old driver code base (from 4.14) after a couple tpm_send() failures, the device was toast, and I had to unload, strobe the reset line, and reload the tpm driver to make it usable again. That failure mode is now longer occurring.

So pulling in 4.18 TPM took a little effort, but I am getting an error free build that brings up the TPM and is not throwing the tpm_send errors that it was throwing before.

I have a giant diff file that performs all the work on a 4.14 base to bring it up, so I can just run it as a patch against a 4.14 pull and it all builds.

Thanks to all for your support and feedback.
A special shout out to Jerry for picking up on that patch.

Doug

-----Original Message-----
From: Jerry Snitselaar <jsnitsel@...>
Sent: Tuesday, March 19, 2019 1:31 PM
To: Doug Fraser <doug.fraser@...>
Cc: James Bottomley <James.Bottomley@...>; openssl-tpm2-engine@groups.io; Kenneth Goldman <kgoldman@...>; Ibmtpm20tss-users@...
Subject: Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

On Tue Mar 19 19, Doug Fraser wrote:
Jerry,

We are on 4.14.77

I will look at cherry picking the tpm from 4.18
I'll take a stab at backporting the commit to 4.14 this afternoon. There are some minor differences back then, but it shouldn't too bad. Since it sounds like you are building the kernel, I can also send along a debugging patch that will spit out the values in the access and status registers when the status expect data check fails.

Regards,
Jerry


We are currently on Alpine 3.8 hoping to move to 3.9 (for other reasons) and also looking to move to 4.19 kernel.
This is going to take some time, but now I have a greater incentive to push on that.

Thank you all for your help in this.

Doug
-client communications.

Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Doug Fraser
 

Hello All,

Last night I completely pulled 4.18 TPM into 4.14 base that we are running.

This required a couple file changes up in include/linux as well as pulling the tpm_hwrng compilation from:

linux/drivers/char/hw_random/Makefile

I need to test to see if the hw_rng (which is now instantiated in the TPM driver) is still functioning properly.

I now longer get the '-5' errors from tpm_send()

Totally gone.

Also, If I pound on the openssl engine really really really hard (really hard) I get timeouts, but the chip is just fine, and if I halt the test load, and restart with a single thread, it comes up fine. On the old driver code base (from 4.14) after a couple tpm_send() failures, the device was toast, and I had to unload, strobe the reset line, and reload the tpm driver to make it usable again. That failure mode is now longer occurring.

So pulling in 4.18 TPM took a little effort, but I am getting an error free build that brings up the TPM and is not throwing the tpm_send errors that it was throwing before.

I have a giant diff file that performs all the work on a 4.14 base to bring it up, so I can just run it as a patch against a 4.14 pull and it all builds.

Thanks to all for your support and feedback.
A special shout out to Jerry for picking up on that patch.

Doug

-----Original Message-----
From: Jerry Snitselaar <jsnitsel@...>
Sent: Tuesday, March 19, 2019 1:31 PM
To: Doug Fraser <doug.fraser@...>
Cc: James Bottomley <James.Bottomley@...>; openssl-tpm2-engine@groups.io; Kenneth Goldman <kgoldman@...>; Ibmtpm20tss-users@...
Subject: Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

On Tue Mar 19 19, Doug Fraser wrote:
Jerry,

We are on 4.14.77

I will look at cherry picking the tpm from 4.18
I'll take a stab at backporting the commit to 4.14 this afternoon. There are some minor differences back then, but it shouldn't too bad. Since it sounds like you are building the kernel, I can also send along a debugging patch that will spit out the values in the access and status registers when the status expect data check fails.

Regards,
Jerry


We are currently on Alpine 3.8 hoping to move to 3.9 (for other reasons) and also looking to move to 4.19 kernel.
This is going to take some time, but now I have a greater incentive to push on that.

Thank you all for your help in this.

Doug

-----Original Message-----
From: Jerry Snitselaar <jsnitsel@...>
Sent: Tuesday, March 19, 2019 12:07 AM
To: James Bottomley <James.Bottomley@...>
Cc: openssl-tpm2-engine@groups.io; Doug Fraser <doug.fraser@...>;
Kenneth Goldman <kgoldman@...>;
Ibmtpm20tss-users@...
Subject: Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

On Mon Mar 18 19, James Bottomley wrote:
On Mon, 2019-03-18 at 21:05 +0000, Doug Fraser wrote:
Ken,

We are interfacing to the TPM via openssl with engine = tpm2, so at
that level, do we have direct access to session control?

The test server requests openssl to encrypt a block of data, then
passes it to the client. The client requests openssl to decrypt the
block, and checks its content. At that point, at the application
level, the transaction is complete.

Is there some other operation that the application has to request
from openssl to flush the related context?
No as I explained in the other email, the openssl_tpm2_engine mostly
keeps the TPM clear when it's not doing anything even if the key is
present and referenced from an openssl point of view.

I'll go back to something James mentioned in his reply, the failure
seems to be percolating up from SPI device layer.
Is there a serialization point within the SPI layer to marshal
commands into and out of the TPM?
Yes, it's chip->tpm_mutex which is taken in tpm_common_write via
tpm_try_get_ops.

I noticed a flag based mutex() with tpm_command(), but I haven't
spilled data from that yet.
Is that the serialization point for the interface?

I don't suspect signal integrity at the low clock rate we are
currently running, particularly in regard to the extreme long term
test that was run over the weekend.
well something caused the -EIO in tpm_try_transmit and that can only
come from the underlying driver. For the tis driver it means that
STS_DATA_EXPECT wasn't signalled as it should for transmitting data.

James
I wonder if this is problem I was dealing with last year where someone had a tpm getting into a state where they were in the send codepath and getting EIO, Debugging code added to where the expect data checks are showed the access register reading that the locality wasn't active, and that the status register was reading 0xff. Looking at ftrace output was odd, because it showed it going into release_locality, and then it would request_locality and see the locality as active, and not try to request the locality. So a patch went into 4.18 to change the release locality code in the tis driver to check that the access register shows the locality as no longer active before returning instead of returning immediately.

Doug, are you able to test a 4.18 or later kernel?

Regards,
Jerry



_______________________________________________
Ibmtpm20tss-users mailing list
Ibmtpm20tss-users@...
https://lists.sourceforge.net/lists/listinfo/ibmtpm20tss-users
This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient:(a) any dissemination or copying of this message is strictly prohibited; and (b) immediately notify the sender by return message and destroy any copies of this message in any form (electronic, paper or otherwise) that you have. The delivery of this message and its information is neither intended to be nor constitutes a disclosure or waiver of any trade secrets, intellectual property, attorney work product, or attorney-client communications.
This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient:(a) any dissemination or copying of this message is strictly prohibited; and (b) immediately notify the sender by return message and destroy any copies of this message in any form (electronic, paper or otherwise) that you have. The delivery of this message and its information is neither intended to be nor constitutes a disclosure or waiver of any trade secrets, intellectual property, attorney work product, or attorney-client communications.

Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Jerry Snitselaar <jsnitsel@...>
 

On Tue Mar 19 19, Doug Fraser wrote:
Jerry,

Was there anything outside of drivers/char/tpm tree?

I diffed that whole tree 4.14 vs 4.18 and got a small number of diffs.

Doug

These five files...

diff --unified --recursive --minimal a/linux/drivers/char/tpm/Kconfig b/linux/drivers/char/tpm/Kconfig
diff --unified --recursive --minimal a/linux/drivers/char/tpm/tpm_crb.c b/linux/drivers/char/tpm/tpm_crb.c
diff --unified --recursive --minimal a/linux/drivers/char/tpm/tpm_i2c_nuvoton.c b/linux/drivers/char/tpm/tpm_i2c_nuvoton.c
diff --unified --recursive --minimal a/linux/drivers/char/tpm/tpm-interface.c b/linux/drivers/char/tpm/tpm-interface.c
diff --unified --recursive --minimal a/linux/drivers/char/tpm/xen-tpmfront.c b/linux/drivers/char/tpm/xen-tpmfront.c
I went back through the email chain again, and it looked like you were
running the tis driver with spi. Is that correct?


With what looks to be the relevant changes in tpm-interface.c, with about a dozen lines spread across five sections of code.

diff --unified --recursive --minimal a/linux/drivers/char/tpm/tpm-interface.c b/linux/drivers/char/tpm/tpm-interface.c
--- a/linux/drivers/char/tpm/tpm-interface.c 2019-02-25 12:55:59.000000000 -0500
+++ b/linux/drivers/char/tpm/tpm-interface.c 2019-03-19 09:36:57.601582514 -0400
@@ -479,13 +479,15 @@

if (need_locality) {
rc = tpm_request_locality(chip, flags);
- if (rc < 0)
- goto out_no_locality;
+ if (rc < 0) {
+ need_locality = false;
+ goto out_locality;
+ }
}

rc = tpm_cmd_ready(chip, flags);
if (rc)
- goto out;
+ goto out_locality;

rc = tpm2_prepare_space(chip, space, ordinal, buf);
if (rc)
@@ -549,14 +551,13 @@
dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);

out:
- rc = tpm_go_idle(chip, flags);
- if (rc)
- goto out;
+ /* may fail but do not override previous error value in rc */
+ tpm_go_idle(chip, flags);

+out_locality:
if (need_locality)
tpm_relinquish_locality(chip, flags);

-out_no_locality:
if (chip->ops->clk_enable != NULL)
chip->ops->clk_enable(chip, false);

@@ -611,12 +612,13 @@
rc = be32_to_cpu(header->return_code);
if (rc != TPM2_RC_RETRY)
break;
- delay_msec *= 2;
+
if (delay_msec > TPM2_DURATION_LONG) {
dev_err(&chip->dev, "TPM is in retry loop\n");
break;
}
tpm_msleep(delay_msec);
+ delay_msec *= 2;
memcpy(buf, save, save_size);
}
return ret;
@@ -652,7 +654,8 @@
return len;

err = be32_to_cpu(header->return_code);
- if (err != 0 && desc)
+ if (err != 0 && err != TPM_ERR_DISABLED && err != TPM_ERR_DEACTIVATED
+ && desc)
dev_err(&chip->dev, "A TPM error (%d) occurred %s\n", err,
desc);
if (err)

-----Original Message-----
From: Jerry Snitselaar <jsnitsel@...>
Sent: Tuesday, March 19, 2019 1:31 PM
To: Doug Fraser <doug.fraser@...>
Cc: James Bottomley <James.Bottomley@...>; openssl-tpm2-engine@groups.io; Kenneth Goldman <kgoldman@...>; Ibmtpm20tss-users@...
Subject: Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

On Tue Mar 19 19, Doug Fraser wrote:
Jerry,

We are on 4.14.77

I will look at cherry picking the tpm from 4.18
I'll take a stab at backporting the commit to 4.14 this afternoon. There are some minor differences back then, but it shouldn't too bad. Since it sounds like you are building the kernel, I can also send along a debugging patch that will spit out the values in the access and status registers when the status expect data check fails.

Regards,
Jerry


We are currently on Alpine 3.8 hoping to move to 3.9 (for other reasons) and also looking to move to 4.19 kernel.
This is going to take some time, but now I have a greater incentive to push on that.

Thank you all for your help in this.

Doug

Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Doug Fraser
 

Ah, sorry, yes we are using the TIS driver

tpm_tis_spi 16384 0
tpm_tis_core 20480 1 tpm_tis_spi
tpm 57344 18 tpm_tis_spi,tpm_tis_core

I was just showing all the diffs that showed up in drivers/char/tpm

It seemed the most interesting (to us) would be in linux/drivers/char/tpm/tpm-interface.c

Is that not used in TIS? I didn't capture any other diffs.


Doug

-----Original Message-----
From: Jerry Snitselaar <jsnitsel@...>
Sent: Tuesday, March 19, 2019 1:52 PM
To: Doug Fraser <doug.fraser@...>
Cc: James Bottomley <James.Bottomley@...>; openssl-tpm2-engine@groups.io; Kenneth Goldman <kgoldman@...>; Ibmtpm20tss-users@...
Subject: Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

On Tue, Mar 19, 2019 at 10:43 AM Doug Fraser <doug.fraser@...> wrote:

Jerry,

Was there anything outside of drivers/char/tpm tree?

I diffed that whole tree 4.14 vs 4.18 and got a small number of diffs.

Doug

These five files...

diff --unified --recursive --minimal a/linux/drivers/char/tpm/Kconfig
b/linux/drivers/char/tpm/Kconfig diff --unified --recursive --minimal
a/linux/drivers/char/tpm/tpm_crb.c b/linux/drivers/char/tpm/tpm_crb.c
diff --unified --recursive --minimal
a/linux/drivers/char/tpm/tpm_i2c_nuvoton.c
b/linux/drivers/char/tpm/tpm_i2c_nuvoton.c
diff --unified --recursive --minimal
a/linux/drivers/char/tpm/tpm-interface.c
b/linux/drivers/char/tpm/tpm-interface.c
diff --unified --recursive --minimal
a/linux/drivers/char/tpm/xen-tpmfront.c
b/linux/drivers/char/tpm/xen-tpmfront.c

With what looks to be the relevant changes in tpm-interface.c, with about a dozen lines spread across five sections of code.
I somehow got it in my mind reading this earlier that you were using the tis driver. My apologies on that, ignore the 4.18 suggestion then.
So you are using
tpm_i2c_nuvoton and the crb driver?



diff --unified --recursive --minimal a/linux/drivers/char/tpm/tpm-interface.c b/linux/drivers/char/tpm/tpm-interface.c
--- a/linux/drivers/char/tpm/tpm-interface.c 2019-02-25 12:55:59.000000000 -0500
+++ b/linux/drivers/char/tpm/tpm-interface.c 2019-03-19 09:36:57.601582514 -0400
@@ -479,13 +479,15 @@

if (need_locality) {
rc = tpm_request_locality(chip, flags);
- if (rc < 0)
- goto out_no_locality;
+ if (rc < 0) {
+ need_locality = false;
+ goto out_locality;
+ }
}

rc = tpm_cmd_ready(chip, flags);
if (rc)
- goto out;
+ goto out_locality;

rc = tpm2_prepare_space(chip, space, ordinal, buf);
if (rc)
@@ -549,14 +551,13 @@
dev_err(&chip->dev, "tpm2_commit_space: error %d\n",
rc);

out:
- rc = tpm_go_idle(chip, flags);
- if (rc)
- goto out;
+ /* may fail but do not override previous error value in rc */
+ tpm_go_idle(chip, flags);

+out_locality:
if (need_locality)
tpm_relinquish_locality(chip, flags);

-out_no_locality:
if (chip->ops->clk_enable != NULL)
chip->ops->clk_enable(chip, false);

@@ -611,12 +612,13 @@
rc = be32_to_cpu(header->return_code);
if (rc != TPM2_RC_RETRY)
break;
- delay_msec *= 2;
+
if (delay_msec > TPM2_DURATION_LONG) {
dev_err(&chip->dev, "TPM is in retry loop\n");
break;
}
tpm_msleep(delay_msec);
+ delay_msec *= 2;
memcpy(buf, save, save_size);
}
return ret;
@@ -652,7 +654,8 @@
return len;

err = be32_to_cpu(header->return_code);
- if (err != 0 && desc)
+ if (err != 0 && err != TPM_ERR_DISABLED && err != TPM_ERR_DEACTIVATED
+ && desc)
dev_err(&chip->dev, "A TPM error (%d) occurred %s\n", err,
desc);
if (err)

-----Original Message-----
From: Jerry Snitselaar <jsnitsel@...>
Sent: Tuesday, March 19, 2019 1:31 PM
To: Doug Fraser <doug.fraser@...>
Cc: James Bottomley <James.Bottomley@...>;
openssl-tpm2-engine@groups.io; Kenneth Goldman <kgoldman@...>;
Ibmtpm20tss-users@...
Subject: Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

On Tue Mar 19 19, Doug Fraser wrote:
Jerry,

We are on 4.14.77

I will look at cherry picking the tpm from 4.18
I'll take a stab at backporting the commit to 4.14 this afternoon. There are some minor differences back then, but it shouldn't too bad. Since it sounds like you are building the kernel, I can also send along a debugging patch that will spit out the values in the access and status registers when the status expect data check fails.

Regards,
Jerry


We are currently on Alpine 3.8 hoping to move to 3.9 (for other reasons) and also looking to move to 4.19 kernel.
This is going to take some time, but now I have a greater incentive to push on that.

Thank you all for your help in this.

Doug
This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient:(a) any dissemination or copying of this message is strictly prohibited; and (b) immediately notify the sender by return message and destroy any copies of this message in any form (electronic, paper or otherwise) that you have. The delivery of this message and its information is neither intended to be nor constitutes a disclosure or waiver of any trade secrets, intellectual property, attorney work product, or attorney-client communications.

Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Jerry Snitselaar <jsnitsel@...>
 

On Tue, Mar 19, 2019 at 10:43 AM Doug Fraser <doug.fraser@...> wrote:

Jerry,

Was there anything outside of drivers/char/tpm tree?

I diffed that whole tree 4.14 vs 4.18 and got a small number of diffs.

Doug

These five files...

diff --unified --recursive --minimal a/linux/drivers/char/tpm/Kconfig b/linux/drivers/char/tpm/Kconfig
diff --unified --recursive --minimal a/linux/drivers/char/tpm/tpm_crb.c b/linux/drivers/char/tpm/tpm_crb.c
diff --unified --recursive --minimal a/linux/drivers/char/tpm/tpm_i2c_nuvoton.c b/linux/drivers/char/tpm/tpm_i2c_nuvoton.c
diff --unified --recursive --minimal a/linux/drivers/char/tpm/tpm-interface.c b/linux/drivers/char/tpm/tpm-interface.c
diff --unified --recursive --minimal a/linux/drivers/char/tpm/xen-tpmfront.c b/linux/drivers/char/tpm/xen-tpmfront.c

With what looks to be the relevant changes in tpm-interface.c, with about a dozen lines spread across five sections of code.
I somehow got it in my mind reading this earlier that you were using
the tis driver. My apologies on that, ignore the 4.18 suggestion then.
So you are using
tpm_i2c_nuvoton and the crb driver?



diff --unified --recursive --minimal a/linux/drivers/char/tpm/tpm-interface.c b/linux/drivers/char/tpm/tpm-interface.c
--- a/linux/drivers/char/tpm/tpm-interface.c 2019-02-25 12:55:59.000000000 -0500
+++ b/linux/drivers/char/tpm/tpm-interface.c 2019-03-19 09:36:57.601582514 -0400
@@ -479,13 +479,15 @@

if (need_locality) {
rc = tpm_request_locality(chip, flags);
- if (rc < 0)
- goto out_no_locality;
+ if (rc < 0) {
+ need_locality = false;
+ goto out_locality;
+ }
}

rc = tpm_cmd_ready(chip, flags);
if (rc)
- goto out;
+ goto out_locality;

rc = tpm2_prepare_space(chip, space, ordinal, buf);
if (rc)
@@ -549,14 +551,13 @@
dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);

out:
- rc = tpm_go_idle(chip, flags);
- if (rc)
- goto out;
+ /* may fail but do not override previous error value in rc */
+ tpm_go_idle(chip, flags);

+out_locality:
if (need_locality)
tpm_relinquish_locality(chip, flags);

-out_no_locality:
if (chip->ops->clk_enable != NULL)
chip->ops->clk_enable(chip, false);

@@ -611,12 +612,13 @@
rc = be32_to_cpu(header->return_code);
if (rc != TPM2_RC_RETRY)
break;
- delay_msec *= 2;
+
if (delay_msec > TPM2_DURATION_LONG) {
dev_err(&chip->dev, "TPM is in retry loop\n");
break;
}
tpm_msleep(delay_msec);
+ delay_msec *= 2;
memcpy(buf, save, save_size);
}
return ret;
@@ -652,7 +654,8 @@
return len;

err = be32_to_cpu(header->return_code);
- if (err != 0 && desc)
+ if (err != 0 && err != TPM_ERR_DISABLED && err != TPM_ERR_DEACTIVATED
+ && desc)
dev_err(&chip->dev, "A TPM error (%d) occurred %s\n", err,
desc);
if (err)

-----Original Message-----
From: Jerry Snitselaar <jsnitsel@...>
Sent: Tuesday, March 19, 2019 1:31 PM
To: Doug Fraser <doug.fraser@...>
Cc: James Bottomley <James.Bottomley@...>; openssl-tpm2-engine@groups.io; Kenneth Goldman <kgoldman@...>; Ibmtpm20tss-users@...
Subject: Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

On Tue Mar 19 19, Doug Fraser wrote:
Jerry,

We are on 4.14.77

I will look at cherry picking the tpm from 4.18
I'll take a stab at backporting the commit to 4.14 this afternoon. There are some minor differences back then, but it shouldn't too bad. Since it sounds like you are building the kernel, I can also send along a debugging patch that will spit out the values in the access and status registers when the status expect data check fails.

Regards,
Jerry


We are currently on Alpine 3.8 hoping to move to 3.9 (for other reasons) and also looking to move to 4.19 kernel.
This is going to take some time, but now I have a greater incentive to push on that.

Thank you all for your help in this.

Doug

Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Doug Fraser
 

Jerry,

Was there anything outside of drivers/char/tpm tree?

I diffed that whole tree 4.14 vs 4.18 and got a small number of diffs.

Doug

These five files...

diff --unified --recursive --minimal a/linux/drivers/char/tpm/Kconfig b/linux/drivers/char/tpm/Kconfig
diff --unified --recursive --minimal a/linux/drivers/char/tpm/tpm_crb.c b/linux/drivers/char/tpm/tpm_crb.c
diff --unified --recursive --minimal a/linux/drivers/char/tpm/tpm_i2c_nuvoton.c b/linux/drivers/char/tpm/tpm_i2c_nuvoton.c
diff --unified --recursive --minimal a/linux/drivers/char/tpm/tpm-interface.c b/linux/drivers/char/tpm/tpm-interface.c
diff --unified --recursive --minimal a/linux/drivers/char/tpm/xen-tpmfront.c b/linux/drivers/char/tpm/xen-tpmfront.c

With what looks to be the relevant changes in tpm-interface.c, with about a dozen lines spread across five sections of code.

diff --unified --recursive --minimal a/linux/drivers/char/tpm/tpm-interface.c b/linux/drivers/char/tpm/tpm-interface.c
--- a/linux/drivers/char/tpm/tpm-interface.c 2019-02-25 12:55:59.000000000 -0500
+++ b/linux/drivers/char/tpm/tpm-interface.c 2019-03-19 09:36:57.601582514 -0400
@@ -479,13 +479,15 @@

if (need_locality) {
rc = tpm_request_locality(chip, flags);
- if (rc < 0)
- goto out_no_locality;
+ if (rc < 0) {
+ need_locality = false;
+ goto out_locality;
+ }
}

rc = tpm_cmd_ready(chip, flags);
if (rc)
- goto out;
+ goto out_locality;

rc = tpm2_prepare_space(chip, space, ordinal, buf);
if (rc)
@@ -549,14 +551,13 @@
dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);

out:
- rc = tpm_go_idle(chip, flags);
- if (rc)
- goto out;
+ /* may fail but do not override previous error value in rc */
+ tpm_go_idle(chip, flags);

+out_locality:
if (need_locality)
tpm_relinquish_locality(chip, flags);

-out_no_locality:
if (chip->ops->clk_enable != NULL)
chip->ops->clk_enable(chip, false);

@@ -611,12 +612,13 @@
rc = be32_to_cpu(header->return_code);
if (rc != TPM2_RC_RETRY)
break;
- delay_msec *= 2;
+
if (delay_msec > TPM2_DURATION_LONG) {
dev_err(&chip->dev, "TPM is in retry loop\n");
break;
}
tpm_msleep(delay_msec);
+ delay_msec *= 2;
memcpy(buf, save, save_size);
}
return ret;
@@ -652,7 +654,8 @@
return len;

err = be32_to_cpu(header->return_code);
- if (err != 0 && desc)
+ if (err != 0 && err != TPM_ERR_DISABLED && err != TPM_ERR_DEACTIVATED
+ && desc)
dev_err(&chip->dev, "A TPM error (%d) occurred %s\n", err,
desc);
if (err)

-----Original Message-----
From: Jerry Snitselaar <jsnitsel@...>
Sent: Tuesday, March 19, 2019 1:31 PM
To: Doug Fraser <doug.fraser@...>
Cc: James Bottomley <James.Bottomley@...>; openssl-tpm2-engine@groups.io; Kenneth Goldman <kgoldman@...>; Ibmtpm20tss-users@...
Subject: Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

On Tue Mar 19 19, Doug Fraser wrote:
Jerry,

We are on 4.14.77

I will look at cherry picking the tpm from 4.18
I'll take a stab at backporting the commit to 4.14 this afternoon. There are some minor differences back then, but it shouldn't too bad. Since it sounds like you are building the kernel, I can also send along a debugging patch that will spit out the values in the access and status registers when the status expect data check fails.

Regards,
Jerry


We are currently on Alpine 3.8 hoping to move to 3.9 (for other reasons) and also looking to move to 4.19 kernel.
This is going to take some time, but now I have a greater incentive to push on that.

Thank you all for your help in this.

Doug

Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Jerry Snitselaar <jsnitsel@...>
 

On Tue Mar 19 19, Doug Fraser wrote:
Jerry,

We are on 4.14.77

I will look at cherry picking the tpm from 4.18
I'll take a stab at backporting the commit to 4.14 this
afternoon. There are some minor differences back then, but it
shouldn't too bad. Since it sounds like you are building the kernel, I
can also send along a debugging patch that will spit out the values in
the access and status registers when the status expect data check
fails.

Regards,
Jerry


We are currently on Alpine 3.8 hoping to move to 3.9 (for other reasons) and also looking to move to 4.19 kernel.
This is going to take some time, but now I have a greater incentive to push on that.

Thank you all for your help in this.

Doug

-----Original Message-----
From: Jerry Snitselaar <jsnitsel@...>
Sent: Tuesday, March 19, 2019 12:07 AM
To: James Bottomley <James.Bottomley@...>
Cc: openssl-tpm2-engine@groups.io; Doug Fraser <doug.fraser@...>; Kenneth Goldman <kgoldman@...>; Ibmtpm20tss-users@...
Subject: Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

On Mon Mar 18 19, James Bottomley wrote:
On Mon, 2019-03-18 at 21:05 +0000, Doug Fraser wrote:
Ken,

We are interfacing to the TPM via openssl with engine = tpm2, so at
that level, do we have direct access to session control?

The test server requests openssl to encrypt a block of data, then
passes it to the client. The client requests openssl to decrypt the
block, and checks its content. At that point, at the application
level, the transaction is complete.

Is there some other operation that the application has to request
from openssl to flush the related context?
No as I explained in the other email, the openssl_tpm2_engine mostly
keeps the TPM clear when it's not doing anything even if the key is
present and referenced from an openssl point of view.

I'll go back to something James mentioned in his reply, the failure
seems to be percolating up from SPI device layer.
Is there a serialization point within the SPI layer to marshal
commands into and out of the TPM?
Yes, it's chip->tpm_mutex which is taken in tpm_common_write via
tpm_try_get_ops.

I noticed a flag based mutex() with tpm_command(), but I haven't
spilled data from that yet.
Is that the serialization point for the interface?

I don't suspect signal integrity at the low clock rate we are
currently running, particularly in regard to the extreme long term
test that was run over the weekend.
well something caused the -EIO in tpm_try_transmit and that can only
come from the underlying driver. For the tis driver it means that
STS_DATA_EXPECT wasn't signalled as it should for transmitting data.

James
I wonder if this is problem I was dealing with last year where someone had a tpm getting into a state where they were in the send codepath and getting EIO, Debugging code added to where the expect data checks are showed the access register reading that the locality wasn't active, and that the status register was reading 0xff. Looking at ftrace output was odd, because it showed it going into release_locality, and then it would request_locality and see the locality as active, and not try to request the locality. So a patch went into 4.18 to change the release locality code in the tis driver to check that the access register shows the locality as no longer active before returning instead of returning immediately.

Doug, are you able to test a 4.18 or later kernel?

Regards,
Jerry



_______________________________________________
Ibmtpm20tss-users mailing list
Ibmtpm20tss-users@...
https://lists.sourceforge.net/lists/listinfo/ibmtpm20tss-users
This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient:(a) any dissemination or copying of this message is strictly prohibited; and (b) immediately notify the sender by return message and destroy any copies of this message in any form (electronic, paper or otherwise) that you have. The delivery of this message and its information is neither intended to be nor constitutes a disclosure or waiver of any trade secrets, intellectual property, attorney work product, or attorney-client communications.

Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

Doug Fraser
 

Jerry,

We are on 4.14.77

I will look at cherry picking the tpm from 4.18

We are currently on Alpine 3.8 hoping to move to 3.9 (for other reasons) and also looking to move to 4.19 kernel.
This is going to take some time, but now I have a greater incentive to push on that.

Thank you all for your help in this.

Doug

-----Original Message-----
From: Jerry Snitselaar <jsnitsel@...>
Sent: Tuesday, March 19, 2019 12:07 AM
To: James Bottomley <James.Bottomley@...>
Cc: openssl-tpm2-engine@groups.io; Doug Fraser <doug.fraser@...>; Kenneth Goldman <kgoldman@...>; Ibmtpm20tss-users@...
Subject: Re: [Ibmtpm20tss-users] [openssl-tpm2-engine] tpm sessions

On Mon Mar 18 19, James Bottomley wrote:
On Mon, 2019-03-18 at 21:05 +0000, Doug Fraser wrote:
Ken,

We are interfacing to the TPM via openssl with engine = tpm2, so at
that level, do we have direct access to session control?

The test server requests openssl to encrypt a block of data, then
passes it to the client. The client requests openssl to decrypt the
block, and checks its content. At that point, at the application
level, the transaction is complete.

Is there some other operation that the application has to request
from openssl to flush the related context?
No as I explained in the other email, the openssl_tpm2_engine mostly
keeps the TPM clear when it's not doing anything even if the key is
present and referenced from an openssl point of view.

I'll go back to something James mentioned in his reply, the failure
seems to be percolating up from SPI device layer.
Is there a serialization point within the SPI layer to marshal
commands into and out of the TPM?
Yes, it's chip->tpm_mutex which is taken in tpm_common_write via
tpm_try_get_ops.

I noticed a flag based mutex() with tpm_command(), but I haven't
spilled data from that yet.
Is that the serialization point for the interface?

I don't suspect signal integrity at the low clock rate we are
currently running, particularly in regard to the extreme long term
test that was run over the weekend.
well something caused the -EIO in tpm_try_transmit and that can only
come from the underlying driver. For the tis driver it means that
STS_DATA_EXPECT wasn't signalled as it should for transmitting data.

James
I wonder if this is problem I was dealing with last year where someone had a tpm getting into a state where they were in the send codepath and getting EIO, Debugging code added to where the expect data checks are showed the access register reading that the locality wasn't active, and that the status register was reading 0xff. Looking at ftrace output was odd, because it showed it going into release_locality, and then it would request_locality and see the locality as active, and not try to request the locality. So a patch went into 4.18 to change the release locality code in the tis driver to check that the access register shows the locality as no longer active before returning instead of returning immediately.

Doug, are you able to test a 4.18 or later kernel?

Regards,
Jerry



_______________________________________________
Ibmtpm20tss-users mailing list
Ibmtpm20tss-users@...
https://lists.sourceforge.net/lists/listinfo/ibmtpm20tss-users
This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient:(a) any dissemination or copying of this message is strictly prohibited; and (b) immediately notify the sender by return message and destroy any copies of this message in any form (electronic, paper or otherwise) that you have. The delivery of this message and its information is neither intended to be nor constitutes a disclosure or waiver of any trade secrets, intellectual property, attorney work product, or attorney-client communications.