| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20220715052235.1452170-7-kuba@kernel.org>
Date: Thu, 14 Jul 2022 22:22:30 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: davem@...emloft.net
Cc: netdev@...r.kernel.org, edumazet@...gle.com, pabeni@...hat.com,
borisp@...dia.com, john.fastabend@...il.com, maximmi@...dia.com,
tariqt@...dia.com, vfedorenko@...ek.ru,
Jakub Kicinski <kuba@...nel.org>
Subject: [PATCH net-next v2 06/11] tls: rx: read the input skb from ctx->recv_pkt
Callers always pass ctx->recv_pkt into decrypt_skb_update(),
and it propagates it to its callees. This may give someone
the false impression that those functions can accept any valid
skb containing a TLS record. That's not the case, the record
sequence number is read from the context, and they can only
take the next record coming out of the strp.
Let the functions get the skb from the context instead of
passing it in. This will also make it cleaner to return
a different skb than ctx->recv_pkt as the decrypted one
later on.
Since we're touching the definition of decrypt_skb_update()
use this as an opportunity to rename it.
Signed-off-by: Jakub Kicinski <kuba@...nel.org>
---
net/tls/tls.h | 14 ++++++++------
net/tls/tls_device.c | 25 ++++++++++++++++---------
net/tls/tls_sw.c | 37 ++++++++++++++++++-------------------
3 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/net/tls/tls.h b/net/tls/tls.h
index e0ccc96a0850..44522b221717 100644
--- a/net/tls/tls.h
+++ b/net/tls/tls.h
@@ -118,8 +118,7 @@ void tls_device_write_space(struct sock *sk, struct tls_context *ctx);
int tls_process_cmsg(struct sock *sk, struct msghdr *msg,
unsigned char *record_type);
-int decrypt_skb(struct sock *sk, struct sk_buff *skb,
- struct scatterlist *sgout);
+int decrypt_skb(struct sock *sk, struct scatterlist *sgout);
int tls_sw_fallback_init(struct sock *sk,
struct tls_offload_context_tx *offload_ctx,
@@ -132,6 +131,11 @@ static inline struct tls_msg *tls_msg(struct sk_buff *skb)
return &scb->tls;
}
+static inline struct sk_buff *tls_strp_msg(struct tls_sw_context_rx *ctx)
+{
+ return ctx->recv_pkt;
+}
+
#ifdef CONFIG_TLS_DEVICE
int tls_device_init(void);
void tls_device_cleanup(void);
@@ -140,8 +144,7 @@ void tls_device_free_resources_tx(struct sock *sk);
int tls_set_device_offload_rx(struct sock *sk, struct tls_context *ctx);
void tls_device_offload_cleanup_rx(struct sock *sk);
void tls_device_rx_resync_new_rec(struct sock *sk, u32 rcd_len, u32 seq);
-int tls_device_decrypted(struct sock *sk, struct tls_context *tls_ctx,
- struct sk_buff *skb, struct strp_msg *rxm);
+int tls_device_decrypted(struct sock *sk, struct tls_context *tls_ctx);
#else
static inline int tls_device_init(void) { return 0; }
static inline void tls_device_cleanup(void) {}
@@ -165,8 +168,7 @@ static inline void
tls_device_rx_resync_new_rec(struct sock *sk, u32 rcd_len, u32 seq) {}
static inline int
-tls_device_decrypted(struct sock *sk, struct tls_context *tls_ctx,
- struct sk_buff *skb, struct strp_msg *rxm)
+tls_device_decrypted(struct sock *sk, struct tls_context *tls_ctx)
{
return 0;
}
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 6f9dff1c11f6..6abbe3c2520c 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -889,14 +889,19 @@ static void tls_device_core_ctrl_rx_resync(struct tls_context *tls_ctx,
}
}
-static int tls_device_reencrypt(struct sock *sk, struct sk_buff *skb)
+static int
+tls_device_reencrypt(struct sock *sk, struct tls_sw_context_rx *sw_ctx)
{
- struct strp_msg *rxm = strp_msg(skb);
- int err = 0, offset = rxm->offset, copy, nsg, data_len, pos;
- struct sk_buff *skb_iter, *unused;
+ int err = 0, offset, copy, nsg, data_len, pos;
+ struct sk_buff *skb, *skb_iter, *unused;
struct scatterlist sg[1];
+ struct strp_msg *rxm;
char *orig_buf, *buf;
+ skb = tls_strp_msg(sw_ctx);
+ rxm = strp_msg(skb);
+ offset = rxm->offset;
+
orig_buf = kmalloc(rxm->full_len + TLS_HEADER_SIZE +
TLS_CIPHER_AES_GCM_128_IV_SIZE, sk->sk_allocation);
if (!orig_buf)
@@ -919,7 +924,7 @@ static int tls_device_reencrypt(struct sock *sk, struct sk_buff *skb)
goto free_buf;
/* We are interested only in the decrypted data not the auth */
- err = decrypt_skb(sk, skb, sg);
+ err = decrypt_skb(sk, sg);
if (err != -EBADMSG)
goto free_buf;
else
@@ -974,10 +979,12 @@ static int tls_device_reencrypt(struct sock *sk, struct sk_buff *skb)
return err;
}
-int tls_device_decrypted(struct sock *sk, struct tls_context *tls_ctx,
- struct sk_buff *skb, struct strp_msg *rxm)
+int tls_device_decrypted(struct sock *sk, struct tls_context *tls_ctx)
{
struct tls_offload_context_rx *ctx = tls_offload_ctx_rx(tls_ctx);
+ struct tls_sw_context_rx *sw_ctx = tls_sw_ctx_rx(tls_ctx);
+ struct sk_buff *skb = tls_strp_msg(sw_ctx);
+ struct strp_msg *rxm = strp_msg(skb);
int is_decrypted = skb->decrypted;
int is_encrypted = !is_decrypted;
struct sk_buff *skb_iter;
@@ -1000,7 +1007,7 @@ int tls_device_decrypted(struct sock *sk, struct tls_context *tls_ctx,
* likely have initial fragments decrypted, and final ones not
* decrypted. We need to reencrypt that single SKB.
*/
- return tls_device_reencrypt(sk, skb);
+ return tls_device_reencrypt(sk, sw_ctx);
}
/* Return immediately if the record is either entirely plaintext or
@@ -1017,7 +1024,7 @@ int tls_device_decrypted(struct sock *sk, struct tls_context *tls_ctx,
}
ctx->resync_nh_reset = 1;
- return tls_device_reencrypt(sk, skb);
+ return tls_device_reencrypt(sk, sw_ctx);
}
static void tls_device_attach(struct tls_context *ctx, struct sock *sk,
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 5ef78e75c463..6205ad1a84c7 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1421,8 +1421,7 @@ static int tls_setup_from_iter(struct iov_iter *from,
* NULL, then the decryption happens inside skb buffers itself, i.e.
* zero-copy gets disabled and 'darg->zc' is updated.
*/
-static int tls_decrypt_sg(struct sock *sk, struct sk_buff *skb,
- struct iov_iter *out_iov,
+static int tls_decrypt_sg(struct sock *sk, struct iov_iter *out_iov,
struct scatterlist *out_sg,
struct tls_decrypt_arg *darg)
{
@@ -1430,6 +1429,7 @@ static int tls_decrypt_sg(struct sock *sk, struct sk_buff *skb,
struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
struct tls_prot_info *prot = &tls_ctx->prot_info;
int n_sgin, n_sgout, aead_size, err, pages = 0;
+ struct sk_buff *skb = tls_strp_msg(ctx);
struct strp_msg *rxm = strp_msg(skb);
struct tls_msg *tlm = tls_msg(skb);
struct aead_request *aead_req;
@@ -1567,14 +1567,14 @@ static int tls_decrypt_sg(struct sock *sk, struct sk_buff *skb,
static int
tls_decrypt_device(struct sock *sk, struct tls_context *tls_ctx,
- struct sk_buff *skb, struct tls_decrypt_arg *darg)
+ struct tls_decrypt_arg *darg)
{
int err;
if (tls_ctx->rx_conf != TLS_HW)
return 0;
- err = tls_device_decrypted(sk, tls_ctx, skb, strp_msg(skb));
+ err = tls_device_decrypted(sk, tls_ctx);
if (err <= 0)
return err;
@@ -1583,22 +1583,22 @@ tls_decrypt_device(struct sock *sk, struct tls_context *tls_ctx,
return 1;
}
-static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
- struct iov_iter *dest,
- struct tls_decrypt_arg *darg)
+static int tls_rx_one_record(struct sock *sk, struct iov_iter *dest,
+ struct tls_decrypt_arg *darg)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
+ struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
struct tls_prot_info *prot = &tls_ctx->prot_info;
- struct strp_msg *rxm = strp_msg(skb);
+ struct strp_msg *rxm;
int pad, err;
- err = tls_decrypt_device(sk, tls_ctx, skb, darg);
+ err = tls_decrypt_device(sk, tls_ctx, darg);
if (err < 0)
return err;
if (err)
goto decrypt_done;
- err = tls_decrypt_sg(sk, skb, dest, NULL, darg);
+ err = tls_decrypt_sg(sk, dest, NULL, darg);
if (err < 0) {
if (err == -EBADMSG)
TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSDECRYPTERROR);
@@ -1613,14 +1613,15 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
if (!darg->tail)
TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSRXNOPADVIOL);
TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSDECRYPTRETRY);
- return decrypt_skb_update(sk, skb, dest, darg);
+ return tls_rx_one_record(sk, dest, darg);
}
decrypt_done:
- pad = tls_padding_length(prot, skb, darg);
+ pad = tls_padding_length(prot, ctx->recv_pkt, darg);
if (pad < 0)
return pad;
+ rxm = strp_msg(ctx->recv_pkt);
rxm->full_len -= pad;
rxm->offset += prot->prepend_size;
rxm->full_len -= prot->overhead_size;
@@ -1630,12 +1631,11 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
return 0;
}
-int decrypt_skb(struct sock *sk, struct sk_buff *skb,
- struct scatterlist *sgout)
+int decrypt_skb(struct sock *sk, struct scatterlist *sgout)
{
struct tls_decrypt_arg darg = { .zc = true, };
- return tls_decrypt_sg(sk, skb, NULL, sgout, &darg);
+ return tls_decrypt_sg(sk, NULL, sgout, &darg);
}
static int tls_record_content_type(struct msghdr *msg, struct tls_msg *tlm,
@@ -1905,7 +1905,7 @@ int tls_sw_recvmsg(struct sock *sk,
else
darg.async = false;
- err = decrypt_skb_update(sk, skb, &msg->msg_iter, &darg);
+ err = tls_rx_one_record(sk, &msg->msg_iter, &darg);
if (err < 0) {
tls_err_abort(sk, -EBADMSG);
goto recv_end;
@@ -2058,14 +2058,13 @@ ssize_t tls_sw_splice_read(struct socket *sock, loff_t *ppos,
if (err <= 0)
goto splice_read_end;
- skb = ctx->recv_pkt;
-
- err = decrypt_skb_update(sk, skb, NULL, &darg);
+ err = tls_rx_one_record(sk, NULL, &darg);
if (err < 0) {
tls_err_abort(sk, -EBADMSG);
goto splice_read_end;
}
+ skb = ctx->recv_pkt;
tls_rx_rec_done(ctx);
}
--
2.36.1
Powered by blists - more mailing lists