| Message ID | 819ed9cd21975ad4d6683d46f4147659ca043f8b.1725972335.git.lukas@wunner.de |
|---|---|
| State | Accepted |
| Commit | 778206d87103ba6d3e401b84d4472e96db7b7582 |
| Headers | show |
| Series | Migrate to sig_alg and templatize ecdsa | expand |
On Tue Sep 10, 2024 at 5:30 PM EEST, Lukas Wunner wrote: > When constructing the EMSA-PKCS1-v1_5 padding for the sign operation, > a buffer for the padding is allocated and the Full Hash Prefix is copied > into it. The padding is then passed to the RSA decrypt operation as an > sglist entry which is succeeded by a second sglist entry for the hash. > > Actually copying the hash prefix around is completely unnecessary. > It can simply be referenced from a third sglist entry which sits > in-between the padding and the digest. > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > crypto/rsassa-pkcs1.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c > index 8f42a5712806..b291ec0944a2 100644 > --- a/crypto/rsassa-pkcs1.c > +++ b/crypto/rsassa-pkcs1.c > @@ -153,7 +153,7 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm, > struct rsassa_pkcs1_ctx *ctx = crypto_sig_ctx(tfm); > unsigned int child_reqsize = crypto_akcipher_reqsize(ctx->child); > struct akcipher_request *child_req __free(kfree_sensitive) = NULL; > - struct scatterlist in_sg[2], out_sg; > + struct scatterlist in_sg[3], out_sg; > struct crypto_wait cwait; > unsigned int pad_len; > unsigned int ps_end; > @@ -173,24 +173,26 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm, > if (slen + hash_prefix->size > ctx->key_size - 11) > return -EOVERFLOW; > > - child_req = kmalloc(sizeof(*child_req) + child_reqsize + > - ctx->key_size - 1 - slen, GFP_KERNEL); > + pad_len = ctx->key_size - slen - hash_prefix->size - 1; > + > + child_req = kmalloc(sizeof(*child_req) + child_reqsize + pad_len, > + GFP_KERNEL); > if (!child_req) > return -ENOMEM; > > /* RFC 8017 sec 8.2.1 step 1 - EMSA-PKCS1-v1_5 encoding generation */ > in_buf = (u8 *)(child_req + 1) + child_reqsize; > - ps_end = ctx->key_size - hash_prefix->size - slen - 2; > + ps_end = pad_len - 1; > in_buf[0] = 0x01; > memset(in_buf + 1, 0xff, ps_end - 1); > in_buf[ps_end] = 0x00; > - memcpy(in_buf + ps_end + 1, hash_prefix->data, hash_prefix->size); > > /* RFC 8017 sec 8.2.1 step 2 - RSA signature */ > crypto_init_wait(&cwait); > - sg_init_table(in_sg, 2); > - sg_set_buf(&in_sg[0], in_buf, ctx->key_size - 1 - slen); > - sg_set_buf(&in_sg[1], src, slen); > + sg_init_table(in_sg, 3); > + sg_set_buf(&in_sg[0], in_buf, pad_len); > + sg_set_buf(&in_sg[1], hash_prefix->data, hash_prefix->size); > + sg_set_buf(&in_sg[2], src, slen); > sg_init_one(&out_sg, dst, dlen); > akcipher_request_set_tfm(child_req, ctx->child); > akcipher_request_set_crypt(child_req, in_sg, &out_sg, LGTM. Not giving any tags because given the size of the patch set this could change in content and/or order (in the series). Wondering if this could be as a clean up like more in the head of the series? BR, Jarkko
diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c index 8f42a5712806..b291ec0944a2 100644 --- a/crypto/rsassa-pkcs1.c +++ b/crypto/rsassa-pkcs1.c @@ -153,7 +153,7 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm, struct rsassa_pkcs1_ctx *ctx = crypto_sig_ctx(tfm); unsigned int child_reqsize = crypto_akcipher_reqsize(ctx->child); struct akcipher_request *child_req __free(kfree_sensitive) = NULL; - struct scatterlist in_sg[2], out_sg; + struct scatterlist in_sg[3], out_sg; struct crypto_wait cwait; unsigned int pad_len; unsigned int ps_end; @@ -173,24 +173,26 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm, if (slen + hash_prefix->size > ctx->key_size - 11) return -EOVERFLOW; - child_req = kmalloc(sizeof(*child_req) + child_reqsize + - ctx->key_size - 1 - slen, GFP_KERNEL); + pad_len = ctx->key_size - slen - hash_prefix->size - 1; + + child_req = kmalloc(sizeof(*child_req) + child_reqsize + pad_len, + GFP_KERNEL); if (!child_req) return -ENOMEM; /* RFC 8017 sec 8.2.1 step 1 - EMSA-PKCS1-v1_5 encoding generation */ in_buf = (u8 *)(child_req + 1) + child_reqsize; - ps_end = ctx->key_size - hash_prefix->size - slen - 2; + ps_end = pad_len - 1; in_buf[0] = 0x01; memset(in_buf + 1, 0xff, ps_end - 1); in_buf[ps_end] = 0x00; - memcpy(in_buf + ps_end + 1, hash_prefix->data, hash_prefix->size); /* RFC 8017 sec 8.2.1 step 2 - RSA signature */ crypto_init_wait(&cwait); - sg_init_table(in_sg, 2); - sg_set_buf(&in_sg[0], in_buf, ctx->key_size - 1 - slen); - sg_set_buf(&in_sg[1], src, slen); + sg_init_table(in_sg, 3); + sg_set_buf(&in_sg[0], in_buf, pad_len); + sg_set_buf(&in_sg[1], hash_prefix->data, hash_prefix->size); + sg_set_buf(&in_sg[2], src, slen); sg_init_one(&out_sg, dst, dlen); akcipher_request_set_tfm(child_req, ctx->child); akcipher_request_set_crypt(child_req, in_sg, &out_sg,
When constructing the EMSA-PKCS1-v1_5 padding for the sign operation, a buffer for the padding is allocated and the Full Hash Prefix is copied into it. The padding is then passed to the RSA decrypt operation as an sglist entry which is succeeded by a second sglist entry for the hash. Actually copying the hash prefix around is completely unnecessary. It can simply be referenced from a third sglist entry which sits in-between the padding and the digest. Signed-off-by: Lukas Wunner <lukas@wunner.de> --- crypto/rsassa-pkcs1.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)