Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request use of SHA256 hash from OpenSSL #31

Open
AndreasFuchsTPM opened this issue Oct 17, 2018 · 9 comments
Open

Request use of SHA256 hash from OpenSSL #31

AndreasFuchsTPM opened this issue Oct 17, 2018 · 9 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@AndreasFuchsTPM
Copy link
Member

Implement an ameth which implements ASN1_PKEY_CTRL_DEFAULT_MD_NID, reporting SHA256 as mandatory.

Split out from #15

@AndreasFuchsTPM AndreasFuchsTPM added enhancement New feature or request good first issue Good for newcomers question Further information is requested and removed good first issue Good for newcomers labels Oct 17, 2018
@AndreasFuchsTPM
Copy link
Member Author

I have no clue on what an ameth is or where to register it.
This will need some investigation and (unless someone has good pointers) postponed until after 0.1 release.

@AndreasFuchsTPM
Copy link
Member Author

Notes from #15:

@AndreasFuchsSIT I'll let you ensure you have an ameth which implements ASN1_PKEY_CTRL_DEFAULT_MD_NID, reporting something like SHA1 or SHA256 as mandatory.

Arguably if you do that, it should be considered a bug in existing versions of OpenSSL that they then ask you to perform any other kind of signature. And in fact, that bug is fixed in my trees at
https://github.com/dwmw2/openssl/commits/pkey_md_1.1.1
https://github.com/dwmw2/openssl/commits/pkey_md_1.1.0
I'll let the PR for OpenSSL 1.1.2 settle first though, before I actually submit those for the stable branches (and I can do 1.0.2 too).

[...]

This is fixed in OpenSSL master now that openssl/openssl#7408 is merged.
PRs open for 1.0.2: openssl/openssl#7610 and 1.1.1: openssl/openssl#7609

This bug is all yours now :)

@AndreasFuchsTPM
Copy link
Member Author

AndreasFuchsTPM commented Nov 27, 2018

@dwmw2 Sorry, but OpenSSL is just beyond my comprehension...

So, at https://github.com/tpm2-software/tpm2-tss-engine/blob/master/src/tpm2-tss-engine-ecc.c#L328
I need to call const EVP_PKEY_ASN1_METHOD *EVP_PKEY_get0_asn1(const EVP_PKEY *pkey) to get the ameth ?
I need to call EVP_PKEY_asn1_set_ctrl(EVP_PKEY_ASN1_METHOD *ameth, (*pkey_ctrl)) to some pkey_ctrl for ECC keys of mine ?
That pkey_ctrl is a int (*pkey_ctrl) (EVP_PKEY *pkey, int op, long arg1, void *arg2)) that implements the op ASN1_PKEY_CTRL_DEFAULT_MD_NID ?
And that pkey_ctrl()'s ASN1_PKEY_CTRL_DEFAULT_MD_NID looks up the hash for the provided pkey's ecc key from the tpm2data and returns it via *(int *)arg2 = NID_sha1 and then returns 1 or 2 (which one makes it mandatory ?) ?

Just wanted to confirm before running the wrong direction...

@dwmw2
Copy link
Contributor

dwmw2 commented Nov 27, 2018

I confess I don't really know. What you suggest looks reasonable.

@AndreasFuchsTPM
Copy link
Member Author

https://mta.openssl.org/pipermail/openssl-users/2018-November/009334.html pointed out that digest values are merely truncated if the curve uses fewer bits. Implement this alongside the ameth ctrl for maximum compatibility.

@dwmw2
Copy link
Contributor

dwmw2 commented Aug 23, 2019

The truncation seems to be working, although shouldn't it be using EC_GROUP_order_bits() instead of hard-coding it for just a couple of curves? That's what I've submitted to the other engine, in https://groups.io/g/openssl-tpm2-engine/topic/patch_truncate_hashes_for/32999767

I suggested that OpenSSL should perhaps do the truncation for itself: openssl/openssl#9680

Note that if OpenSSL does that, then our trick of making up a hash type for the TPM to ignore based on the digest size is going to start failing. Perhaps we should just always say SHA256 regardless of the hash being used? Why does the TPM care anyway?

@AndreasFuchsTPM
Copy link
Member Author

The problem is within the TPM spec. The max size for input buffers on the TPM command is restricted by the maximum hash size of TPM supported hash algorithms. Kind of weird, but whatever.
Thus a "typical" TPM will not support 64 bytes input buffers since it hash no support for sha512, whilst a TPM simulator will support sha512 / 64 byte input buffers.

Would you mind sending a PR to here as well so it's not GPL tainted ? Thanks !

@dwmw2
Copy link
Contributor

dwmw2 commented Aug 26, 2019

How does #139 look?

@AndreasFuchsTPM
Copy link
Member Author

LGTM.
Still need to ameth for RSA though, so leaving this open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants