Topics

[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

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

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.

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.