| Message ID | 20190124172146.q4acuud6z35wevm4@davejwatson-mba.dhcp.thefacebook.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next,1/7] net: tls: Save iv in tls_rec for async crypto requests | expand |
On Thu, 24 Jan 2019 17:20:12 +0000, Dave Watson wrote: > aead_request_set_crypt takes an iv pointer, and we change the iv > soon after setting it. Some async crypto algorithms don't save the iv, > so we need to save it in the tls_rec for async requests. > > Found by hardcoding x64 aesni to use async crypto manager (to test the async > codepath), however I don't think this combination can happen in the wild. > Presumably other hardware offloads will need this fix, but there have been > no user reports. > > Fixes: a42055e8d2c30 ("Add support for async encryption of records...") > Signed-off-by: Dave Watson <davejwatson@fb.com> This one should target net, no?
On 01/24/19 02:14 PM, Jakub Kicinski wrote: > On Thu, 24 Jan 2019 17:20:12 +0000, Dave Watson wrote: > > aead_request_set_crypt takes an iv pointer, and we change the iv > > soon after setting it. Some async crypto algorithms don't save the iv, > > so we need to save it in the tls_rec for async requests. > > > > Found by hardcoding x64 aesni to use async crypto manager (to test the async > > codepath), however I don't think this combination can happen in the wild. > > Presumably other hardware offloads will need this fix, but there have been > > no user reports. > > > > Fixes: a42055e8d2c30 ("Add support for async encryption of records...") > > Signed-off-by: Dave Watson <davejwatson@fb.com> > > This one should target net, no? Later patches touch the same code, so would need to be in both to avoid merge conflicts.
From: Dave Watson <davejwatson@fb.com> Date: Thu, 24 Jan 2019 22:34:29 +0000 > Later patches touch the same code, so would need to be in both to > avoid merge conflicts. Dave, here is the way you should handle this. Submit this patch for net. Let me know that something you want to submit for net-next depends upon it. I will high-prio my merge to Linus to that I can fast forward net and then merge it into net-next. I will let you know this has happened. Then you can submit the net-next parts on top. Thanks.
diff --git a/include/net/tls.h b/include/net/tls.h index 90bf52db573e..4592606e136a 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -120,6 +120,8 @@ struct tls_rec { struct scatterlist sg_aead_out[2]; char aad_space[TLS_AAD_SPACE_SIZE]; + u8 iv_data[TLS_CIPHER_AES_GCM_128_IV_SIZE + + TLS_CIPHER_AES_GCM_128_SALT_SIZE]; struct aead_request aead_req; u8 aead_req_ctx[]; }; diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 86b9527c4826..0382a4674bb3 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -447,6 +447,8 @@ static int tls_do_encryption(struct sock *sk, struct scatterlist *sge = sk_msg_elem(msg_en, start); int rc; + memcpy(rec->iv_data, tls_ctx->tx.iv, sizeof(rec->iv_data)); + sge->offset += tls_ctx->tx.prepend_size; sge->length -= tls_ctx->tx.prepend_size; @@ -456,7 +458,7 @@ static int tls_do_encryption(struct sock *sk, aead_request_set_ad(aead_req, TLS_AAD_SPACE_SIZE); aead_request_set_crypt(aead_req, rec->sg_aead_in, rec->sg_aead_out, - data_len, tls_ctx->tx.iv); + data_len, rec->iv_data); aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG, tls_encrypt_done, sk);
aead_request_set_crypt takes an iv pointer, and we change the iv soon after setting it. Some async crypto algorithms don't save the iv, so we need to save it in the tls_rec for async requests. Found by hardcoding x64 aesni to use async crypto manager (to test the async codepath), however I don't think this combination can happen in the wild. Presumably other hardware offloads will need this fix, but there have been no user reports. Fixes: a42055e8d2c30 ("Add support for async encryption of records...") Signed-off-by: Dave Watson <davejwatson@fb.com> --- include/net/tls.h | 2 ++ net/tls/tls_sw.c | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-)