| Message ID | 20240528210823.28798-1-jarkko@kernel.org |
|---|---|
| Headers | show |
| Series | KEYS: asymmetric: tpm2_key_{rsa,ecdsa} | expand |
On Wed May 29, 2024 at 12:42 AM EEST, Jarkko Sakkinen wrote: > On Wed May 29, 2024 at 12:08 AM EEST, Jarkko Sakkinen wrote: > > + /* Encode the ASN.1 signature: */ > > +#define TPM2_KEY_ECDSA_SIG_SIZE (2 + 2 * (2 + SHA256_DIGEST_SIZE) + r_0 + s_0) > > + pr_info("sig_size=%d\n", TPM2_KEY_ECDSA_SIG_SIZE); > > + ptr[0] = 0x30; /* SEQUENCE */ > > + ptr[1] = TPM2_KEY_ECDSA_SIG_SIZE - 2; > > +#define TPM2_KEY_ECDSA_SIG_R_TAG 2 > > +#define TPM2_KEY_ECDSA_SIG_R_SIZE 3 > > +#define TPM2_KEY_ECDSA_SIG_R_BODY 4 > > + ptr[TPM2_KEY_ECDSA_SIG_R_TAG] = 0x02; /* INTEGER */ > > + ptr[TPM2_KEY_ECDSA_SIG_R_SIZE] = SHA256_DIGEST_SIZE + r_0; > > + ptr[TPM2_KEY_ECDSA_SIG_R_BODY] = 0x00; /* maybe dummy write */ > > + memcpy(&ptr[TPM2_KEY_ECDSA_SIG_R_BODY + r_0], r, SHA256_DIGEST_SIZE); > > +#define TPM2_KEY_ECDSA_SIG_S_TAG (4 + r_0 + SHA256_DIGEST_SIZE) > > +#define TPM2_KEY_ECDSA_SIG_S_SIZE (5 + r_0 + SHA256_DIGEST_SIZE) > > +#define TPM2_KEY_ECDSA_SIG_S_BODY (6 + r_0 + SHA256_DIGEST_SIZE) > > + ptr[TPM2_KEY_ECDSA_SIG_S_TAG] = 0x02; /* INTEGER */ > > + ptr[TPM2_KEY_ECDSA_SIG_S_SIZE] = SHA256_DIGEST_SIZE + s_0; > > + ptr[TPM2_KEY_ECDSA_SIG_S_BODY] = 0x00; /* maybe dummy write */ > > + memcpy(&ptr[TPM2_KEY_ECDSA_SIG_S_BODY + s_0], s, SHA256_DIGEST_SIZE); > > + ret = TPM2_KEY_ECDSA_SIG_SIZE; > > Stefan, so this how I realized the signature encoding, thanks to > your earlier remarks [1]! I found out based on that a few glitches > and ended up with this better structured ECDSA signature encoder, > so thank you for doing that. 1. SHA384 would fit without any significant changes. 2. SHA512 will require a single additional byte, i.e. prefix byte 0x81 for the sequence (not for coeffients). 3. SM3 similarly is also trivial to add. Both will be also easy to add later on. I would not enlarge this patch set from what it is now. So I think this is pretty well along the lines of: https://datatracker.ietf.org/doc/draft-woodhouse-cert-best-practice/ BR, Jarkko
On 5/28/24 17:08, Jarkko Sakkinen wrote: > ASN.1 template is required for TPM2 asymmetric keys, as it needs to be > piggy-packed with the input data before applying TPM2_RSA_Decrypt. This piggy-backed > patch prepares crypto subsystem for the addition of those keys. > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> > --- > crypto/rsa-pkcs1pad.c | 16 ++++++++++------ > include/crypto/rsa-pkcs1pad.h | 20 ++++++++++++++++++++ > 2 files changed, 30 insertions(+), 6 deletions(-) > create mode 100644 include/crypto/rsa-pkcs1pad.h > > diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c > index cd501195f34a..00b6c14f861c 100644 > --- a/crypto/rsa-pkcs1pad.c > +++ b/crypto/rsa-pkcs1pad.c > @@ -7,6 +7,7 @@ > > #include <crypto/algapi.h> > #include <crypto/akcipher.h> > +#include <crypto/rsa-pkcs1pad.h> > #include <crypto/internal/akcipher.h> > #include <crypto/internal/rsa.h> > #include <linux/err.h> > @@ -79,11 +80,7 @@ static const u8 rsa_digest_info_sha3_512[] = { > 0x05, 0x00, 0x04, 0x40 > }; > > -static const struct rsa_asn1_template { > - const char *name; > - const u8 *data; > - size_t size; > -} rsa_asn1_templates[] = { > +static const struct rsa_asn1_template rsa_asn1_templates[] = { > #define _(X) { #X, rsa_digest_info_##X, sizeof(rsa_digest_info_##X) } > _(md5), > _(sha1), > @@ -101,7 +98,13 @@ static const struct rsa_asn1_template { > { NULL } > }; > > -static const struct rsa_asn1_template *rsa_lookup_asn1(const char *name) > +/** > + * rsa_lookup_asn1() - Lookup the ASN.1 digest info given the hash > + * name: hash algorithm name > + * > + * Returns the ASN.1 digest info on success, and NULL on failure. > + */ > +const struct rsa_asn1_template *rsa_lookup_asn1(const char *name) > { > const struct rsa_asn1_template *p; > > @@ -110,6 +113,7 @@ static const struct rsa_asn1_template *rsa_lookup_asn1(const char *name) > return p; > return NULL; > } > +EXPORT_SYMBOL_GPL(rsa_lookup_asn1); > > struct pkcs1pad_ctx { > struct crypto_akcipher *child; > diff --git a/include/crypto/rsa-pkcs1pad.h b/include/crypto/rsa-pkcs1pad.h > new file mode 100644 > index 000000000000..32c7453ff644 > --- /dev/null > +++ b/include/crypto/rsa-pkcs1pad.h > @@ -0,0 +1,20 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * RSA padding templates. > + */ > + > +#ifndef _CRYPTO_RSA_PKCS1PAD_H > +#define _CRYPTO_RSA_PKCS1PAD_H > + > +/* > + * Hash algorithm name to ASN.1 template mapping. > + */ > +struct rsa_asn1_template { > + const char *name; > + const u8 *data; > + size_t size; > +}; > + > +const struct rsa_asn1_template *rsa_lookup_asn1(const char *name); > + > +#endif /* _CRYPTO_RSA_PKCS1PAD_H */
On Wed May 29, 2024 at 2:15 AM EEST, Stefan Berger wrote: > > + ptr[TPM2_KEY_ECDSA_SIG_R_TAG] = 0x02; /* INTEGER */ > > + ptr[TPM2_KEY_ECDSA_SIG_R_SIZE] = SHA256_DIGEST_SIZE + r_0; > > The size of the signature has nothing to do with the size of the hash. > SHA256_DIGEST_SIZE (32) happens to match the number of bytes of a > coordinate of prime256v1 / NIST p256 but should fail when you use > secp521r1 / NIST p521 since then r or s may then be 66 or 67 bytes (if > most sign. bit is set) long. First remark did not go unnoticed, so thanks for both. There was not just much to comment on it :-) I could just replace the constant with a (range checked) variable read from the response and overall structure woud be the same. This will also mean that in the case of P521 also prefix byte (0x81) is required but just for the sequence I think, not for the integers. Finally, I need to implement p521 smoke test for testing this patch set. One big letdown that I only now have consciously realized, is that TCG does not have p256k1 in their algorithm repository. It is the basis for quite a few blockchain technologies. I wonder why... BR, Jarkko
On Wed May 29, 2024 at 2:20 AM EEST, Stefan Berger wrote: > > > On 5/28/24 17:08, Jarkko Sakkinen wrote: > > ASN.1 template is required for TPM2 asymmetric keys, as it needs to be > > piggy-packed with the input data before applying TPM2_RSA_Decrypt. This > > piggy-backed Right! I consciously wrote it that way, i.e. have used wrong spelling up to this day :-) Thanks for the review! This is not likely to change that much. Would not tag any other patches tho before up to p521 have been tested... BR, Jarkko