mbox series

[SRU,B,0/5] netfilter: nf_conncount: fix for LP#1811094

Message ID 20190110033603.31647-1-mfo@canonical.com
Headers show
Series netfilter: nf_conncount: fix for LP#1811094 | expand

Message

Mauricio Faria de Oliveira Jan. 10, 2019, 3:35 a.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1811094

[Impact] 

 * The iptables connection count/limit rules can be breached 
   with multithreaded network driver/server/client (common)
   due to a race in the conncount/connlimit code.

 * For example:

   # iptables -A INPUT -p tcp -m tcp --syn --dport 7777 \
     -m connlimit --connlimit-above 2000 --connlimit-mask 0 \
     -j DROP

 * The fix is a backport from an upstream commit that resolves
   the problem (plus dependencies for a cleaner backport) that
   address the race condition:

   commit b36e4523d4d5 ("netfilter: nf_conncount: fix garbage
   collection confirm race").

[Test Case]

 * Server-side: (relevant kernel side)
   (limit TCP port 7777 to only 2000 connections)

   # iptables -A INPUT -p tcp -m tcp --syn --dport 7777 \
     -m connlimit --connlimit-above 2000 --connlimit-mask 0 \
     -j DROP

   # ulimit -SHn 65000   # increase number of open files
   # ruby server.rb      # multi-threaded server

 * Client-side:

   # ulimit -SHn 65000
   # ruby client.rb <server ip> <port> <target # connections> <# threads> 
   <test output>

 * Results with Original kernel:
   (client achieves target of 6000 connections > limit of 2000 connections)

   # ruby client.rb 10.230.56.100 7777 6000 3 
   1
   2
   3
   <...>
   6000
   Target reached. Thread finishing
   6001
   Target reached. Thread finishing
   6002
   Target reached. Thread finishing
   Threads done. 6002 connections
   press enter to exit

 * Results with Modified kernel:
   (client is limited to 2000 connections, and times out afterward)

   # ruby client.rb 10.230.56.100 7777 6000 3
   1
   2
   3
   <...>
   2000
   <... blocks for a few minutes ...>
   failed to create connection: Connection timed out - connect(2) for "10.230.56.100" port 7777
   failed to create connection: Connection timed out - connect(2) for "10.230.56.100" port 7777
   failed to create connection: Connection timed out - connect(2) for "10.230.56.100" port 7777
   Threads done. 2000 connections
   press enter to exit

 * Test cases possibly available upon request,
   depending on original author's permission.

[Regression Potential] 

 * The patchset has been reviewed by a netfilter maintainer [1] in
   stable mailing list, and was considered OK for 4.14, and that's
   essentially the same backport for 4.15 and 4.4. 

 * The changes are limited to netfilter conncount/connlimit (names
   change between older/newer kernel versions).

[Other Info]
 
 * The backport for 4.14 [2] is applied as of 4.14.92.

[1] https://www.spinics.net/lists/stable/msg276883.html
[2] https://www.spinics.net/lists/stable/msg276910.html

Florian Westphal (2):
  netfilter: nf_conncount: fix garbage collection confirm race
  netfilter: nf_conncount: don't skip eviction when age is negative

Mauricio Faria de Oliveira (1):
  UBUNTU: SAUCE: netfilter: xt_connlimit: remove the 'addr' parameter in
    add_hlist()

Pablo Neira Ayuso (1):
  netfilter: nf_conncount: expose connection list interface

Yi-Hung Wei (1):
  netfilter: nf_conncount: Fix garbage collection with zones

 include/net/netfilter/nf_conntrack_count.h | 15 ++++
 net/netfilter/xt_connlimit.c               | 96 +++++++++++++++++-----
 2 files changed, 91 insertions(+), 20 deletions(-)
 create mode 100644 include/net/netfilter/nf_conntrack_count.h

Comments

Khalid Elmously Jan. 10, 2019, 6:04 a.m. UTC | #1
On 2019-01-10 01:35:58 , Mauricio Faria de Oliveira wrote:
> BugLink: https://bugs.launchpad.net/bugs/1811094
> 
> [Impact] 
> 
>  * The iptables connection count/limit rules can be breached 
>    with multithreaded network driver/server/client (common)
>    due to a race in the conncount/connlimit code.
> 
>  * For example:
> 
>    # iptables -A INPUT -p tcp -m tcp --syn --dport 7777 \
>      -m connlimit --connlimit-above 2000 --connlimit-mask 0 \
>      -j DROP
> 
>  * The fix is a backport from an upstream commit that resolves
>    the problem (plus dependencies for a cleaner backport) that
>    address the race condition:
> 
>    commit b36e4523d4d5 ("netfilter: nf_conncount: fix garbage
>    collection confirm race").
> 
> [Test Case]
> 
>  * Server-side: (relevant kernel side)
>    (limit TCP port 7777 to only 2000 connections)
> 
>    # iptables -A INPUT -p tcp -m tcp --syn --dport 7777 \
>      -m connlimit --connlimit-above 2000 --connlimit-mask 0 \
>      -j DROP
> 
>    # ulimit -SHn 65000   # increase number of open files
>    # ruby server.rb      # multi-threaded server
> 
>  * Client-side:
> 
>    # ulimit -SHn 65000
>    # ruby client.rb <server ip> <port> <target # connections> <# threads> 
>    <test output>
> 
>  * Results with Original kernel:
>    (client achieves target of 6000 connections > limit of 2000 connections)
> 
>    # ruby client.rb 10.230.56.100 7777 6000 3 
>    1
>    2
>    3
>    <...>
>    6000
>    Target reached. Thread finishing
>    6001
>    Target reached. Thread finishing
>    6002
>    Target reached. Thread finishing
>    Threads done. 6002 connections
>    press enter to exit
> 
>  * Results with Modified kernel:
>    (client is limited to 2000 connections, and times out afterward)
> 
>    # ruby client.rb 10.230.56.100 7777 6000 3
>    1
>    2
>    3
>    <...>
>    2000
>    <... blocks for a few minutes ...>
>    failed to create connection: Connection timed out - connect(2) for "10.230.56.100" port 7777
>    failed to create connection: Connection timed out - connect(2) for "10.230.56.100" port 7777
>    failed to create connection: Connection timed out - connect(2) for "10.230.56.100" port 7777
>    Threads done. 2000 connections
>    press enter to exit
> 
>  * Test cases possibly available upon request,
>    depending on original author's permission.
> 
> [Regression Potential] 
> 
>  * The patchset has been reviewed by a netfilter maintainer [1] in
>    stable mailing list, and was considered OK for 4.14, and that's
>    essentially the same backport for 4.15 and 4.4. 
> 
>  * The changes are limited to netfilter conncount/connlimit (names
>    change between older/newer kernel versions).
> 
> [Other Info]
>  
>  * The backport for 4.14 [2] is applied as of 4.14.92.
> 
> [1] https://www.spinics.net/lists/stable/msg276883.html
> [2] https://www.spinics.net/lists/stable/msg276910.html
> 
> Florian Westphal (2):
>   netfilter: nf_conncount: fix garbage collection confirm race
>   netfilter: nf_conncount: don't skip eviction when age is negative
> 
> Mauricio Faria de Oliveira (1):
>   UBUNTU: SAUCE: netfilter: xt_connlimit: remove the 'addr' parameter in
>     add_hlist()
> 
> Pablo Neira Ayuso (1):
>   netfilter: nf_conncount: expose connection list interface
> 
> Yi-Hung Wei (1):
>   netfilter: nf_conncount: Fix garbage collection with zones
> 
>  include/net/netfilter/nf_conntrack_count.h | 15 ++++
>  net/netfilter/xt_connlimit.c               | 96 +++++++++++++++++-----
>  2 files changed, 91 insertions(+), 20 deletions(-)
>  create mode 100644 include/net/netfilter/nf_conntrack_count.h
> 
Acked-by: Khalid Elmously <khalid.elmously@canonical.com>
Stefan Bader Jan. 10, 2019, 2:37 p.m. UTC | #2
On 10.01.19 04:35, Mauricio Faria de Oliveira wrote:
> BugLink: https://bugs.launchpad.net/bugs/1811094
> 
> [Impact] 
> 
>  * The iptables connection count/limit rules can be breached 
>    with multithreaded network driver/server/client (common)
>    due to a race in the conncount/connlimit code.
> 
>  * For example:
> 
>    # iptables -A INPUT -p tcp -m tcp --syn --dport 7777 \
>      -m connlimit --connlimit-above 2000 --connlimit-mask 0 \
>      -j DROP
> 
>  * The fix is a backport from an upstream commit that resolves
>    the problem (plus dependencies for a cleaner backport) that
>    address the race condition:
> 
>    commit b36e4523d4d5 ("netfilter: nf_conncount: fix garbage
>    collection confirm race").
> 
> [Test Case]
> 
>  * Server-side: (relevant kernel side)
>    (limit TCP port 7777 to only 2000 connections)
> 
>    # iptables -A INPUT -p tcp -m tcp --syn --dport 7777 \
>      -m connlimit --connlimit-above 2000 --connlimit-mask 0 \
>      -j DROP
> 
>    # ulimit -SHn 65000   # increase number of open files
>    # ruby server.rb      # multi-threaded server
> 
>  * Client-side:
> 
>    # ulimit -SHn 65000
>    # ruby client.rb <server ip> <port> <target # connections> <# threads> 
>    <test output>
> 
>  * Results with Original kernel:
>    (client achieves target of 6000 connections > limit of 2000 connections)
> 
>    # ruby client.rb 10.230.56.100 7777 6000 3 
>    1
>    2
>    3
>    <...>
>    6000
>    Target reached. Thread finishing
>    6001
>    Target reached. Thread finishing
>    6002
>    Target reached. Thread finishing
>    Threads done. 6002 connections
>    press enter to exit
> 
>  * Results with Modified kernel:
>    (client is limited to 2000 connections, and times out afterward)
> 
>    # ruby client.rb 10.230.56.100 7777 6000 3
>    1
>    2
>    3
>    <...>
>    2000
>    <... blocks for a few minutes ...>
>    failed to create connection: Connection timed out - connect(2) for "10.230.56.100" port 7777
>    failed to create connection: Connection timed out - connect(2) for "10.230.56.100" port 7777
>    failed to create connection: Connection timed out - connect(2) for "10.230.56.100" port 7777
>    Threads done. 2000 connections
>    press enter to exit
> 
>  * Test cases possibly available upon request,
>    depending on original author's permission.
> 
> [Regression Potential] 
> 
>  * The patchset has been reviewed by a netfilter maintainer [1] in
>    stable mailing list, and was considered OK for 4.14, and that's
>    essentially the same backport for 4.15 and 4.4. 
> 
>  * The changes are limited to netfilter conncount/connlimit (names
>    change between older/newer kernel versions).
> 
> [Other Info]
>  
>  * The backport for 4.14 [2] is applied as of 4.14.92.
> 
> [1] https://www.spinics.net/lists/stable/msg276883.html
> [2] https://www.spinics.net/lists/stable/msg276910.html
> 
> Florian Westphal (2):
>   netfilter: nf_conncount: fix garbage collection confirm race
>   netfilter: nf_conncount: don't skip eviction when age is negative
> 
> Mauricio Faria de Oliveira (1):
>   UBUNTU: SAUCE: netfilter: xt_connlimit: remove the 'addr' parameter in
>     add_hlist()
> 
> Pablo Neira Ayuso (1):
>   netfilter: nf_conncount: expose connection list interface
> 
> Yi-Hung Wei (1):
>   netfilter: nf_conncount: Fix garbage collection with zones
> 
>  include/net/netfilter/nf_conntrack_count.h | 15 ++++
>  net/netfilter/xt_connlimit.c               | 96 +++++++++++++++++-----
>  2 files changed, 91 insertions(+), 20 deletions(-)
>  create mode 100644 include/net/netfilter/nf_conntrack_count.h
> 
For the Bionic set with the updated description as proposed in the reply (no
need to re-send)

Acked-by: Stefan Bader <stefan.bader@canonical.com>
Kleber Sacilotto de Souza Jan. 10, 2019, 3:27 p.m. UTC | #3
On 1/10/19 4:35 AM, Mauricio Faria de Oliveira wrote:
> BugLink: https://bugs.launchpad.net/bugs/1811094
>
> [Impact] 
>
>  * The iptables connection count/limit rules can be breached 
>    with multithreaded network driver/server/client (common)
>    due to a race in the conncount/connlimit code.
>
>  * For example:
>
>    # iptables -A INPUT -p tcp -m tcp --syn --dport 7777 \
>      -m connlimit --connlimit-above 2000 --connlimit-mask 0 \
>      -j DROP
>
>  * The fix is a backport from an upstream commit that resolves
>    the problem (plus dependencies for a cleaner backport) that
>    address the race condition:
>
>    commit b36e4523d4d5 ("netfilter: nf_conncount: fix garbage
>    collection confirm race").
>
> [Test Case]
>
>  * Server-side: (relevant kernel side)
>    (limit TCP port 7777 to only 2000 connections)
>
>    # iptables -A INPUT -p tcp -m tcp --syn --dport 7777 \
>      -m connlimit --connlimit-above 2000 --connlimit-mask 0 \
>      -j DROP
>
>    # ulimit -SHn 65000   # increase number of open files
>    # ruby server.rb      # multi-threaded server
>
>  * Client-side:
>
>    # ulimit -SHn 65000
>    # ruby client.rb <server ip> <port> <target # connections> <# threads> 
>    <test output>
>
>  * Results with Original kernel:
>    (client achieves target of 6000 connections > limit of 2000 connections)
>
>    # ruby client.rb 10.230.56.100 7777 6000 3 
>    1
>    2
>    3
>    <...>
>    6000
>    Target reached. Thread finishing
>    6001
>    Target reached. Thread finishing
>    6002
>    Target reached. Thread finishing
>    Threads done. 6002 connections
>    press enter to exit
>
>  * Results with Modified kernel:
>    (client is limited to 2000 connections, and times out afterward)
>
>    # ruby client.rb 10.230.56.100 7777 6000 3
>    1
>    2
>    3
>    <...>
>    2000
>    <... blocks for a few minutes ...>
>    failed to create connection: Connection timed out - connect(2) for "10.230.56.100" port 7777
>    failed to create connection: Connection timed out - connect(2) for "10.230.56.100" port 7777
>    failed to create connection: Connection timed out - connect(2) for "10.230.56.100" port 7777
>    Threads done. 2000 connections
>    press enter to exit
>
>  * Test cases possibly available upon request,
>    depending on original author's permission.
>
> [Regression Potential] 
>
>  * The patchset has been reviewed by a netfilter maintainer [1] in
>    stable mailing list, and was considered OK for 4.14, and that's
>    essentially the same backport for 4.15 and 4.4. 
>
>  * The changes are limited to netfilter conncount/connlimit (names
>    change between older/newer kernel versions).
>
> [Other Info]
>  
>  * The backport for 4.14 [2] is applied as of 4.14.92.
>
> [1] https://www.spinics.net/lists/stable/msg276883.html
> [2] https://www.spinics.net/lists/stable/msg276910.html
>
> Florian Westphal (2):
>   netfilter: nf_conncount: fix garbage collection confirm race
>   netfilter: nf_conncount: don't skip eviction when age is negative
>
> Mauricio Faria de Oliveira (1):
>   UBUNTU: SAUCE: netfilter: xt_connlimit: remove the 'addr' parameter in
>     add_hlist()
>
> Pablo Neira Ayuso (1):
>   netfilter: nf_conncount: expose connection list interface
>
> Yi-Hung Wei (1):
>   netfilter: nf_conncount: Fix garbage collection with zones
>
>  include/net/netfilter/nf_conntrack_count.h | 15 ++++
>  net/netfilter/xt_connlimit.c               | 96 +++++++++++++++++-----
>  2 files changed, 91 insertions(+), 20 deletions(-)
>  create mode 100644 include/net/netfilter/nf_conntrack_count.h
>
Applied to bionic/master-next branch, with the proposed commit message
for patch 1/5.

Thanks,
Kleber