netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] doc: document danger of applying REJECT to INVALID CTs
@ 2020-06-03 13:36 Jan Engelhardt
  2020-06-07 22:30 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Engelhardt @ 2020-06-03 13:36 UTC (permalink / raw)
  To: netfilter-devel; +Cc: jengelh

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
 extensions/libip6t_REJECT.man | 20 ++++++++++++++++++++
 extensions/libipt_REJECT.man  | 20 ++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/extensions/libip6t_REJECT.man b/extensions/libip6t_REJECT.man
index 0030a51f..3c42768e 100644
--- a/extensions/libip6t_REJECT.man
+++ b/extensions/libip6t_REJECT.man
@@ -30,3 +30,23 @@ TCP RST packet to be sent back.  This is mainly useful for blocking
 hosts (which won't accept your mail otherwise).
 \fBtcp\-reset\fP
 can only be used with kernel versions 2.6.14 or later.
+.PP
+\fIWarning:\fP You should not indiscriminately apply the REJECT target to
+packets whose connection state is classified as INVALID; instead, you should
+only DROP these.
+.PP
+Consider a source host transmitting a packet P, with P experiencing so much
+delay along its path that the source host issues a retransmission, P_2, with
+P_2 being successful in reaching its destination and advancing the connection
+state normally. It is conceivable that the late-arriving P may be considered
+not to be associated with any connection tracking entry. Generating a reject
+response for a packet so classed would then terminate the healthy connection.
+.PP
+So, instead of:
+.PP
+-A INPUT ... -j REJECT
+.PP
+do consider using:
+.PP
+-A INPUT ... -m conntrack --ctstate INVALID -j DROP
+-A INPUT ... -j REJECT
diff --git a/extensions/libipt_REJECT.man b/extensions/libipt_REJECT.man
index 8a360ce7..cc47aead 100644
--- a/extensions/libipt_REJECT.man
+++ b/extensions/libipt_REJECT.man
@@ -30,3 +30,23 @@ TCP RST packet to be sent back.  This is mainly useful for blocking
 hosts (which won't accept your mail otherwise).
 .IP
 (*) Using icmp\-admin\-prohibited with kernels that do not support it will result in a plain DROP instead of REJECT
+.PP
+\fIWarning:\fP You should not indiscriminately apply the REJECT target to
+packets whose connection state is classified as INVALID; instead, you should
+only DROP these.
+.PP
+Consider a source host transmitting a packet P, with P experiencing so much
+delay along its path that the source host issues a retransmission, P_2, with
+P_2 being successful in reaching its destination and advancing the connection
+state normally. It is conceivable that the late-arriving P may be considered
+not to be associated with any connection tracking entry. Generating a reject
+response for a packet so classed would then terminate the healthy connection.
+.PP
+So, instead of:
+.PP
+-A INPUT ... -j REJECT
+.PP
+do consider using:
+.PP
+-A INPUT ... -m conntrack --ctstate INVALID -j DROP
+-A INPUT ... -j REJECT
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] doc: document danger of applying REJECT to INVALID CTs
  2020-06-03 13:36 [PATCH] doc: document danger of applying REJECT to INVALID CTs Jan Engelhardt
@ 2020-06-07 22:30 ` Pablo Neira Ayuso
  0 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2020-06-07 22:30 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: netfilter-devel

Applied, thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] doc: document danger of applying REJECT to INVALID CTs
  2020-05-09 21:28   ` Maciej Żenczykowski
@ 2020-05-09 21:31     ` Maciej Żenczykowski
  0 siblings, 0 replies; 5+ messages in thread
From: Maciej Żenczykowski @ 2020-05-09 21:31 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Pablo Neira Ayuso, Florian Westphal, Linux NetDev,
	Netfilter Development Mailing List

Also maybe the example should be:

instead of just:
-A INPUT ... -j REJECT
do:
-A INPUT ... -m conntrack --ctstate INVALID -j DROP
-A INPUT ... -j REJECT

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] doc: document danger of applying REJECT to INVALID CTs
  2020-05-09 21:17 ` [PATCH] doc: document danger of applying REJECT to INVALID CTs Jan Engelhardt
@ 2020-05-09 21:28   ` Maciej Żenczykowski
  2020-05-09 21:31     ` Maciej Żenczykowski
  0 siblings, 1 reply; 5+ messages in thread
From: Maciej Żenczykowski @ 2020-05-09 21:28 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Pablo Neira Ayuso, Florian Westphal, Linux NetDev,
	Netfilter Development Mailing List

I *think* that your talk of 3 packets is not needed, ie. the initial
delayed packet doesn't have to be a retransmission.
It can be the first copy of that segment that gets massively delayed
and arrives late and causes problems,
by virtue of arriving after the retransmission already caused the
connection to move on.

Other than that this does seem perhaps a bit cleared than what I wrote.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] doc: document danger of applying REJECT to INVALID CTs
  2020-05-09 17:45 [PATCH] document danger of '-j REJECT'ing of '-m state INVALID' packets Maciej Żenczykowski
@ 2020-05-09 21:17 ` Jan Engelhardt
  2020-05-09 21:28   ` Maciej Żenczykowski
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Engelhardt @ 2020-05-09 21:17 UTC (permalink / raw)
  To: zenczykowski; +Cc: maze, pablo, fw, netdev, netfilter-devel

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---

Maciej's explanation on how INVALID+REJECT can lead to problems looks
convincing. I hereby present new manpage wording in the form of "if A, then B"
to better build the argument of avoiding REJECT. So the issue is not caused by
an _incoming_ TCP RST as the initial mail might have suggested,
but by RST generated by REJECT (--reject-with tcp-reset).

It is conceivable to me that a connection termination may occur with not only
TCP+RST, but also with TCP+ICMP and UDP+ICMP, so I trimmed any
protocol-specific wording too. Also trimmed is any mention of -j ACCEPT,
because rule order is not the point of the argument.


 extensions/libip6t_REJECT.man | 21 +++++++++++++++++++++
 extensions/libipt_REJECT.man  | 21 +++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/extensions/libip6t_REJECT.man b/extensions/libip6t_REJECT.man
index 0030a51f..38183dd7 100644
--- a/extensions/libip6t_REJECT.man
+++ b/extensions/libip6t_REJECT.man
@@ -30,3 +30,24 @@ TCP RST packet to be sent back.  This is mainly useful for blocking
 hosts (which won't accept your mail otherwise).
 \fBtcp\-reset\fP
 can only be used with kernel versions 2.6.14 or later.
+.PP
+\fIWarning:\fP You should not indiscrimnately apply the REJECT target to
+packets whose connection state is classified as INVALID; instead, you should
+only DROP these:
+.PP
+Consider a source host retransmitting an original packet P as P_2 for any
+reason, and P_2 getting routed via a different path (load balancing/policy
+routing, or anything of the kind). Additionally, let P_2 experience so much
+delay that the source host issues \fIanother\fP retransmission, P_3, with P_3
+being succesful in reaching its destination and advancing the connection state
+normally. The delayed P_2, when it eventually is processed, may be considered
+to be not associated with any connection tracking entry. Generating a reject
+packet for such a belated packet would then terminate the healthy connection.
+.PP
+So, instead of:
+.PP
+-A INPUT -m conntrack --ctstate INVALID -j REJECT
+.PP
+do consider using:
+.PP
+-A INPUT -m conntrack --ctstate INVALID -j DROP
diff --git a/extensions/libipt_REJECT.man b/extensions/libipt_REJECT.man
index 8a360ce7..9e80d7ea 100644
--- a/extensions/libipt_REJECT.man
+++ b/extensions/libipt_REJECT.man
@@ -30,3 +30,24 @@ TCP RST packet to be sent back.  This is mainly useful for blocking
 hosts (which won't accept your mail otherwise).
 .IP
 (*) Using icmp\-admin\-prohibited with kernels that do not support it will result in a plain DROP instead of REJECT
+.PP
+\fIWarning:\fP You should not indiscrimnately apply the REJECT target to
+packets whose connection state is classified as INVALID; instead, you should
+only DROP these:
+.PP
+Consider a source host retransmitting an original packet P as P_2 for any
+reason, and P_2 getting routed via a different path (load balancing/policy
+routing, or anything of the kind). Additionally, let P_2 experience so much
+delay that the source host issues \fIanother\fP retransmission, P_3, with P_3
+being succesful in reaching its destination and advancing the connection state
+normally. The delayed P_2, when it eventually is processed, may be considered
+to be not associated with any connection tracking entry. Generating a reject
+packet for such a belated packet would then terminate the healthy connection.
+.PP
+So, instead of:
+.PP
+-A INPUT -m conntrack --ctstate INVALID -j REJECT
+.PP
+do consider using:
+.PP
+-A INPUT -m conntrack --ctstate INVALID -j DROP
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-06-07 22:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 13:36 [PATCH] doc: document danger of applying REJECT to INVALID CTs Jan Engelhardt
2020-06-07 22:30 ` Pablo Neira Ayuso
  -- strict thread matches above, loose matches on Subject: below --
2020-05-09 17:45 [PATCH] document danger of '-j REJECT'ing of '-m state INVALID' packets Maciej Żenczykowski
2020-05-09 21:17 ` [PATCH] doc: document danger of applying REJECT to INVALID CTs Jan Engelhardt
2020-05-09 21:28   ` Maciej Żenczykowski
2020-05-09 21:31     ` Maciej Żenczykowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).