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@xxxxxxxxx>
> ---
> 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
