| Message ID | dcb40882817bb4c9396ca2dc209360ed7d4b9af9.1725972335.git.lukas@wunner.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Migrate to sig_alg and templatize ecdsa | expand |
On Tue Sep 10, 2024 at 5:30 PM EEST, Lukas Wunner wrote: > The RSASSA-PKCS1-v1_5 sign operation currently only checks that the > digest length is less than "key_size - hash_prefix->size - 11". > The verify operation merely checks that it's more than zero. > > Actually the precise digest length is known because the hash algorithm > is specified upon instance creation and the digest length is encoded > into the final byte of the hash algorithm's Full Hash Prefix. > > So check for the exact digest length rather than solely relying on > imprecise maximum/minimum checks. > > Keep the maximum length check for the sign operation as a safety net, > but drop the now unnecessary minimum check for the verify operation. > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > crypto/rsassa-pkcs1.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c > index 779c080fc013..8f42a5712806 100644 > --- a/crypto/rsassa-pkcs1.c > +++ b/crypto/rsassa-pkcs1.c > @@ -11,6 +11,7 @@ > #include <linux/scatterlist.h> > #include <crypto/akcipher.h> > #include <crypto/algapi.h> > +#include <crypto/hash.h> > #include <crypto/sig.h> > #include <crypto/internal/akcipher.h> > #include <crypto/internal/rsa.h> > @@ -118,6 +119,20 @@ static const struct hash_prefix *rsassa_pkcs1_find_hash_prefix(const char *name) > return NULL; > } > > +static unsigned int rsassa_pkcs1_hash_len(const struct hash_prefix *p) > +{ > + /* > + * The final byte of the Full Hash Prefix encodes the hash length. > + * > + * This needs to be revisited should hash algorithms with more than > + * 1016 bits (127 bytes * 8) ever be added. The length would then > + * be encoded into more than one byte by ASN.1. > + */ Maybe this could be moved outside the function. > + static_assert(HASH_MAX_DIGESTSIZE <= 127); > + > + return p->data[p->size - 1]; > +} > + > struct rsassa_pkcs1_ctx { > struct crypto_akcipher *child; > unsigned int key_size; > @@ -152,6 +167,9 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm, > if (dlen < ctx->key_size) > return -EOVERFLOW; > > + if (slen != rsassa_pkcs1_hash_len(hash_prefix)) > + return -EINVAL; > + > if (slen + hash_prefix->size > ctx->key_size - 11) > return -EOVERFLOW; > > @@ -217,7 +235,7 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm, > /* RFC 8017 sec 8.2.2 step 1 - length checking */ > if (!ctx->key_size || > slen != ctx->key_size || > - !dlen) > + dlen != rsassa_pkcs1_hash_len(hash_prefix)) > return -EINVAL; > > /* RFC 8017 sec 8.2.2 step 2 - RSA verification */ BR, Jarkko
diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c index 779c080fc013..8f42a5712806 100644 --- a/crypto/rsassa-pkcs1.c +++ b/crypto/rsassa-pkcs1.c @@ -11,6 +11,7 @@ #include <linux/scatterlist.h> #include <crypto/akcipher.h> #include <crypto/algapi.h> +#include <crypto/hash.h> #include <crypto/sig.h> #include <crypto/internal/akcipher.h> #include <crypto/internal/rsa.h> @@ -118,6 +119,20 @@ static const struct hash_prefix *rsassa_pkcs1_find_hash_prefix(const char *name) return NULL; } +static unsigned int rsassa_pkcs1_hash_len(const struct hash_prefix *p) +{ + /* + * The final byte of the Full Hash Prefix encodes the hash length. + * + * This needs to be revisited should hash algorithms with more than + * 1016 bits (127 bytes * 8) ever be added. The length would then + * be encoded into more than one byte by ASN.1. + */ + static_assert(HASH_MAX_DIGESTSIZE <= 127); + + return p->data[p->size - 1]; +} + struct rsassa_pkcs1_ctx { struct crypto_akcipher *child; unsigned int key_size; @@ -152,6 +167,9 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm, if (dlen < ctx->key_size) return -EOVERFLOW; + if (slen != rsassa_pkcs1_hash_len(hash_prefix)) + return -EINVAL; + if (slen + hash_prefix->size > ctx->key_size - 11) return -EOVERFLOW; @@ -217,7 +235,7 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm, /* RFC 8017 sec 8.2.2 step 1 - length checking */ if (!ctx->key_size || slen != ctx->key_size || - !dlen) + dlen != rsassa_pkcs1_hash_len(hash_prefix)) return -EINVAL; /* RFC 8017 sec 8.2.2 step 2 - RSA verification */
The RSASSA-PKCS1-v1_5 sign operation currently only checks that the digest length is less than "key_size - hash_prefix->size - 11". The verify operation merely checks that it's more than zero. Actually the precise digest length is known because the hash algorithm is specified upon instance creation and the digest length is encoded into the final byte of the hash algorithm's Full Hash Prefix. So check for the exact digest length rather than solely relying on imprecise maximum/minimum checks. Keep the maximum length check for the sign operation as a safety net, but drop the now unnecessary minimum check for the verify operation. Signed-off-by: Lukas Wunner <lukas@wunner.de> --- crypto/rsassa-pkcs1.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)