[PATCH nf] netfilter: nft_payload: restrict checksum offsets to known values
Florian Westphal <[email protected]>
| Newsgroups | gmane.comp.security.firewalls.netfilter.devel |
|---|---|
| Message-ID | <[email protected]> |
We need to prevent userspace from corrupting e.g. tcp->doff, because
many locations in conntrack and conntrack helpers rely on
nf_conntrack_in() having validated the packet headers.
nft_payload allows to alter headers later which invalidates this
assumption.
The 'Fixes' commit restricts writes to safe fields, but there is
another side channel: the checksum location.
Restrict this too. Reported via sashiko/gemini.
Fixes: 112e447d17f7 ("netfilter: validate L4 headers after userspace packet writes")
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Florian Westphal <[email protected]>
---
net/netfilter/nft_payload.c | 36 +++++++++++++++++++++++++++++-------
1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
index e315d35f73d4..70f70a65e327 100644
--- a/net/netfilter/nft_payload.c
+++ b/net/netfilter/nft_payload.c
@@ -1008,11 +1008,13 @@ static bool nft_payload_validate_inet_csum_offset(const struct nft_ctx *ctx,
if (priv->csum_flags) /* makes no sense, asks for "re-update" of L4 checksum */
return false;
- /* no further check here; offset can't be negative so bogus
- * offsets can corrupt L4 or payload but not l3 headers.
- * We already allow arbitrary l4/inner payload writes.
- */
- return true;
+ /* Validate csum_offset is one of the supported transport header checksums */
+ if (priv->csum_offset == offsetof(struct tcphdr, check) ||
+ priv->csum_offset == offsetof(struct udphdr, check) ||
+ priv->csum_offset == offsetof(struct icmp6hdr, icmp6_cksum))
+ return true;
+
+ return false;
case NFT_PAYLOAD_INNER_HEADER:
return true;
case NFT_PAYLOAD_TUN_HEADER:
@@ -1046,6 +1048,25 @@ static bool nft_payload_csum_nh_write_ok(const struct nft_payload_set *priv,
return false;
}
+static bool nft_payload_csum_th_write_ok(const struct nft_payload_set *priv,
+ const struct nft_pktinfo *pkt)
+{
+ if (!(pkt->flags & NFT_PKTINFO_L4PROTO))
+ return false;
+
+ switch (pkt->tprot) {
+ case IPPROTO_TCP:
+ return priv->csum_offset == offsetof(struct tcphdr, check);
+ case IPPROTO_UDP:
+ case IPPROTO_UDPLITE:
+ return priv->csum_offset == offsetof(struct udphdr, check);
+ case IPPROTO_ICMPV6:
+ return priv->csum_offset == offsetof(struct icmp6hdr, icmp6_cksum);
+ }
+
+ return false;
+}
+
static bool nft_payload_csum_write_ok(const struct nft_pktinfo *pkt,
const struct nft_payload_set *priv)
{
@@ -1055,9 +1076,10 @@ static bool nft_payload_csum_write_ok(const struct nft_pktinfo *pkt,
case NFT_PAYLOAD_NETWORK_HEADER:
return nft_payload_csum_nh_write_ok(priv, pkt);
case NFT_PAYLOAD_TRANSPORT_HEADER:
+ return nft_payload_csum_th_write_ok(priv, pkt);
case NFT_PAYLOAD_INNER_HEADER:
- /* neither offsets are validated, offsets cannot be
- * negative so real l3 headers cannot be mangled.
+ /* offset is not validated, offset cannot be
+ * negative so real l3/l4 headers cannot be mangled.
*/
return true;
case NFT_PAYLOAD_TUN_HEADER:
--
2.54.0