mbox series

[nf-next,0/3] netfilter: conntrack: clash resolution for reverse collisions

Message ID 20240910093821.4871-1-fw@strlen.de
Headers show
Series netfilter: conntrack: clash resolution for reverse collisions | expand

Message

Florian Westphal Sept. 10, 2024, 9:38 a.m. UTC
This series resolves an esoteric scenario.

Given two tasks sending UDP packets to one another, NAT engine
can falsely detect a port collision if it happens to pick up
a reply packet as 'new' rather than 'reply'.

First patch adds extra code to detect this and suppress port
reallocation in this case.

Second patch extends clash resolution logic to detect such
a reverse clash (clashing conntrack is reply to existing entry).

Patch 3 adds a test case.

Since this has existed forever and hasn't been reported in two
decades I'm submitting this for -next.

Florian Westphal (3):
  netfilter: nf_nat: don't try nat source port reallocation for reverse
    dir clash
  netfilter: conntrack: add clash resolution for reverse collisions
  selftests: netfilter: add reverse-clash resolution test case

 net/netfilter/nf_conntrack_core.c             |  56 +++++++-
 net/netfilter/nf_nat_core.c                   | 120 ++++++++++++++++-
 .../testing/selftests/net/netfilter/Makefile  |   2 +
 .../net/netfilter/conntrack_reverse_clash.c   | 125 ++++++++++++++++++
 .../net/netfilter/conntrack_reverse_clash.sh  |  51 +++++++
 5 files changed, 347 insertions(+), 7 deletions(-)
 create mode 100644 tools/testing/selftests/net/netfilter/conntrack_reverse_clash.c
 create mode 100755 tools/testing/selftests/net/netfilter/conntrack_reverse_clash.sh

Comments

Pablo Neira Ayuso Sept. 15, 2024, 9:11 p.m. UTC | #1
Hi Florian,

On Tue, Sep 10, 2024 at 11:38:13AM +0200, Florian Westphal wrote:
> This series resolves an esoteric scenario.
> 
> Given two tasks sending UDP packets to one another, NAT engine
> can falsely detect a port collision if it happens to pick up
> a reply packet as 'new' rather than 'reply'.
> 
> First patch adds extra code to detect this and suppress port
> reallocation in this case.
> 
> Second patch extends clash resolution logic to detect such
> a reverse clash (clashing conntrack is reply to existing entry).
> 
> Patch 3 adds a test case.
> 
> Since this has existed forever and hasn't been reported in two
> decades I'm submitting this for -next.

-next is now closed, my plan is to place this series in nf.git for the
next PR.

nf-next will remain open in this cycle so hopefully we can merge your
updates to reduce memory footprint in the next -rc.

I cannot go any faster.
Florian Westphal Sept. 16, 2024, 8:38 a.m. UTC | #2
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Tue, Sep 10, 2024 at 11:38:13AM +0200, Florian Westphal wrote:
> > This series resolves an esoteric scenario.
> > 
> > Given two tasks sending UDP packets to one another, NAT engine
> > can falsely detect a port collision if it happens to pick up
> > a reply packet as 'new' rather than 'reply'.
> > 
> > First patch adds extra code to detect this and suppress port
> > reallocation in this case.
> > 
> > Second patch extends clash resolution logic to detect such
> > a reverse clash (clashing conntrack is reply to existing entry).
> > 
> > Patch 3 adds a test case.
> > 
> > Since this has existed forever and hasn't been reported in two
> > decades I'm submitting this for -next.
> 
> -next is now closed, my plan is to place this series in nf.git for the
> next PR.

Thats fine, I placed this in -next because I thought it was not a real
bug that warrents a change this close to release.

> nf-next will remain open in this cycle so hopefully we can merge your
> updates to reduce memory footprint in the next -rc.

Great, that works for me.

> I cannot go any faster.

Its fine, don't worry.