[PATCH net] tls: device: keep tls_sw_fallback() from encrypting its own output

[Rishikesh Jethwani]

A socket offloaded through a bond has every skb validated twice, once
at the master and again at the slave. tls_validate_xmit_skb() passes
the master-level call through for that reason (netif_is_bond_master()),
but tls_validate_xmit_skb_sw(), the validator tls_device_down()
installs, calls tls_sw_fallback() unconditionally, so it encrypts the
master-level ciphertext a second time with the same rcd_sn and IV. This
cancels the CTR keystream and exposes plaintext on the wire. For a
complete record encrypted twice in one skb, the second pass also
generates a valid GCM tag, so the peer accepts corrupted data without
an authentication failure.

Guard on the content rather than the device. Offload plaintext always
carries the decrypted bit (tls_push_data() and
tls_device_write_space() send with MSG_SENDPAGE_DECRYPTED), so
tls_sw_fallback() can pass through any skb without it: such a skb needs
no offload encryption. complete_skb() now clears the bit that
skb_copy_header() copied from the plaintext original, so the fallback's
own output passes through any later validation pass untouched. This
also does not rely on every path validating a skb at both the master
and the slave.

Tested on two hosts back to back over a mlx5 port, the only slave of
a bond0 (balance-xor, layer3+4), so each transmit is validated at the
master and again at the slave. Run from each host as sender, a TLS 1.2
AES-GCM-128 TX socket kept open across an administrative bounce of the
slave leaked every later record as plaintext before the fix; after it
each record is encrypted exactly once and the kTLS peer reads the
stream intact.

Fixes: c55dcdd435aa ("net/tls: Fix use-after-free after the TLS device goes down and up")
Cc: stable@xxxxxxxxxxxxxxx # needs commit fc6d80eb5044 ("tcp: prevent collapsing skbs across boundary in rtx queue")
Signed-off-by: Rishikesh Jethwani <rjethwani@xxxxxxxxxxxxxxx>
---
 net/tls/tls_device_fallback.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c
index 3b7d0ab2bcf1..24fba12febd1 100644
--- a/net/tls/tls_device_fallback.c
+++ b/net/tls/tls_device_fallback.c
@@ -192,6 +192,14 @@ static void complete_skb(struct sk_buff *nskb, struct sk_buff *skb, int headln)
 	skb->destructor = NULL;
 	skb->sk = NULL;
 
+	/* nskb carries ciphertext now, but skb_copy_header() inherited the
+	 * plaintext original's decrypted bit. Clear it so the bit keeps meaning
+	 * "still-plaintext, needs an encryptor": a requeued nskb would otherwise
+	 * be re-validated and encrypted a second time on any re-entry into
+	 * tls_sw_fallback().
+	 */
+	nskb->decrypted = 0;
+
 	update_chksum(nskb, headln);
 
 	/* sock_efree means skb must gone through skb_orphan_partial() */
@@ -371,6 +379,17 @@ static struct sk_buff *tls_sw_fallback(struct sock *sk, struct sk_buff *skb)
 	s32 sync_size = 0;
 	u64 rcd_sn;
 
+	/* Only skbs carrying the decrypted bit hold plaintext that awaits
+	 * offload encryption; anything else needs no work here. That
+	 * includes this function's own output (complete_skb() clears the
+	 * bit skb_copy_header() inherited), which is seen again when a skb
+	 * is validated a second time, e.g. a bond master handing it to a
+	 * slave via bond_dev_queue_xmit(). fill_sg_in() would map it back
+	 * onto its own record by tcp_seq, reusing rcd_sn and IV.
+	 */
+	if (!skb_is_decrypted(skb))
+		return skb;
+
 	/* worst case is:
 	 * MAX_SKB_FRAGS in tls_record_info
 	 * MAX_SKB_FRAGS + 1 in SKB head and frags.
-- 
2.50.1