All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: Qi Tang <tpluszz77@gmail.com>
Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
	pablo@netfilter.org, herbert@gondor.apana.org.au,
	michael.bommarito@gmail.com, lyutoon@gmail.com
Subject: Re: [RFC] netfilter: disable payload mangling in userns
Date: Fri, 15 May 2026 14:48:05 +0200	[thread overview]
Message-ID: <agcWBZNugohelNp6@strlen.de> (raw)
In-Reply-To: <20260515114848.1105927-1-tpluszz77@gmail.com>

Qi Tang <tpluszz77@gmail.com> wrote:
> I agree with the userns block.  I'll keep pushing the five
> consumer-side bounds-check patches: root in init userns can
> still install the same payload-set rule and trigger the same
> OOB at the re-read sites, so the underlying bug fix is still
> worth landing.
> 
> None of the five sites overlap with the relax wishlist (saddr/
> daddr, transport, linklayer).  Same class showed up in an
> earlier patch:
>   https://lore.kernel.org/netdev/20260514035802.1540395-1-tpluszz77@gmail.com/
> 
> These five are unlikely to be all of them; we think the
> consumer side warrants a broader audit.  Thoughts?

I think we have to do both.

For nft_payload/nft_exthdr:
1. Writes from !initial_userns are rejected (rule insert fails).
2. Writes for initial userns get validated at rule add time:
  - netdev ingress is allowed to alter everything, I think
  this is early enough to not introduce oddities that can't come
  from wire / untrusted peer.
 - bridge is allowed to alter everything: AFAICS there shouldn't
 be a problem with this, same as with ingress.
 - inet (ipv4/ip6): Check base (offset is unsigned and relative
   to ll/network header/transport header/payload
   - Allow modifications past transport header.
   - Allow modifications of transport header
   - Allow network header modifications for a subset of valid
     offsets/lengths: saddr, daddr, checksum, tos, id, ttl / hl.
     Reject everything else.
   - Allow link-layer; but check at from packetpath that
     offset + len don't write past start of the network header.
     Else: no-op.

nfqueue is the bigger problem: userspace gives us a whole new
packet data blob, not a delta that we should apply at offset x).

I think we have to update nfqueue to do rudimentary header revalidation
and drop the packet on failure, e.g. at least making sure tot_len is not
past skb->len.

      reply	other threads:[~2026-05-15 12:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15 10:04 [RFC] netfilter: disable payload mangling in userns Florian Westphal
2026-05-15 11:48 ` Qi Tang
2026-05-15 12:48   ` Florian Westphal [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=agcWBZNugohelNp6@strlen.de \
    --to=fw@strlen.de \
    --cc=herbert@gondor.apana.org.au \
    --cc=lyutoon@gmail.com \
    --cc=michael.bommarito@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=tpluszz77@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.