All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Avoid potentially erroneos RST check
@ 2021-04-28 13:11 Ali Abdallah
  2021-04-28 13:23 ` Florian Westphal
  2021-04-28 14:30 ` Pablo Neira Ayuso
  0 siblings, 2 replies; 6+ messages in thread
From: Ali Abdallah @ 2021-04-28 13:11 UTC (permalink / raw)
  To: netfilter-devel

In 'commit b303e7b80ff1 ("Reset the max ACK flag on SYN in ignore state")'
we reset the max ACK number to avoid dropping valid RST that is a
response to a SYN.

Unfortunately that might not be enough, an out of order ACK in origin
might reset it back, and we might end up again dropping valid RST.

This patch disables the RST check when we are not in established state
and  we receive an RST with SEQ=0 that is most likely a response to a
SYN we had let it go through.

Signed-off-by: Ali Abdallah <aabdallah@suse.de>
---
 net/netfilter/nf_conntrack_proto_tcp.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 83890a700ef8..fb1c389a97fe 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -1048,6 +1048,12 @@ int nf_conntrack_tcp_packet(struct nf_conn *ct,
 		if (ct->proto.tcp.seen[!dir].flags & IP_CT_TCP_FLAG_MAXACK_SET) {
 			u32 seq = ntohl(th->seq);
 
+			/* If we are not in established state, and an RST is
+			 * observed with SEQ=0, this is most likely an answer
+			 * to a SYN we had let go through above.
+			 */
+			if (seq == 0 && !nf_conntrack_tcp_established(ct))
+				break;
+
 			if (before(seq, ct->proto.tcp.seen[!dir].td_maxack) &&
 			    !tn->tcp_be_liberal) {
 				/* Invalid RST  */
-- 
2.26.2

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

* Re: [PATCH] Avoid potentially erroneos RST check
  2021-04-28 13:11 [PATCH] Avoid potentially erroneos RST check Ali Abdallah
@ 2021-04-28 13:23 ` Florian Westphal
  2021-04-28 14:30 ` Pablo Neira Ayuso
  1 sibling, 0 replies; 6+ messages in thread
From: Florian Westphal @ 2021-04-28 13:23 UTC (permalink / raw)
  To: Ali Abdallah; +Cc: netfilter-devel

Ali Abdallah <ali.abdallah@suse.com> wrote:
> In 'commit b303e7b80ff1 ("Reset the max ACK flag on SYN in ignore state")'
> we reset the max ACK number to avoid dropping valid RST that is a
> response to a SYN.
> 
> Unfortunately that might not be enough, an out of order ACK in origin
> might reset it back, and we might end up again dropping valid RST.
> 
> This patch disables the RST check when we are not in established state
> and  we receive an RST with SEQ=0 that is most likely a response to a
> SYN we had let it go through.
> 
> Signed-off-by: Ali Abdallah <aabdallah@suse.de>

Looks good, thanks!

Acked-by: Florian Westphal <fw@strlen.de>

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

* Re: [PATCH] Avoid potentially erroneos RST check
  2021-04-28 13:11 [PATCH] Avoid potentially erroneos RST check Ali Abdallah
  2021-04-28 13:23 ` Florian Westphal
@ 2021-04-28 14:30 ` Pablo Neira Ayuso
  2021-04-30  9:27   ` Ali Abdallah
  1 sibling, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2021-04-28 14:30 UTC (permalink / raw)
  To: Ali Abdallah; +Cc: netfilter-devel

Hi,

On Wed, Apr 28, 2021 at 03:11:47PM +0200, Ali Abdallah wrote:
> In 'commit b303e7b80ff1 ("Reset the max ACK flag on SYN in ignore state")'
> we reset the max ACK number to avoid dropping valid RST that is a
> response to a SYN.

I did not apply:

https://patchwork.ozlabs.org/project/netfilter-devel/patch/20210420122415.v2jtayiw3n4ds7t7@Fryzen495/

as you requested to send a v2.

Would it make sense to squash this patch and ("Reset the max ACK flag
on SYN in ignore state") in one single patch?

Thanks.

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

* Re: [PATCH] Avoid potentially erroneos RST check
  2021-04-28 14:30 ` Pablo Neira Ayuso
@ 2021-04-30  9:27   ` Ali Abdallah
  2021-05-03 21:07     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 6+ messages in thread
From: Ali Abdallah @ 2021-04-30  9:27 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On 28.04.2021 16:30, Pablo Neira Ayuso wrote:
> I did not apply:
> 
> https://patchwork.ozlabs.org/project/netfilter-devel/patch/20210420122415.v2jtayiw3n4ds7t7@Fryzen495/
> 
> as you requested to send a v2.
> 
> Would it make sense to squash this patch and ("Reset the max ACK flag
> on SYN in ignore state") in one single patch?
> 
> Thanks.

Yes, I will send a single patch then. Thanks.

-- 
Ali Abdallah | SUSE Linux L3 Engineer
GPG fingerprint: 51A0 F4A0 C8CF C98F 842E  A9A8 B945 56F8 1C85 D0D5


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

* Re: [PATCH] Avoid potentially erroneos RST check
  2021-04-30  9:27   ` Ali Abdallah
@ 2021-05-03 21:07     ` Pablo Neira Ayuso
  2021-05-04  6:55       ` Ali Abdallah
  0 siblings, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2021-05-03 21:07 UTC (permalink / raw)
  To: Ali Abdallah; +Cc: netfilter-devel

On Fri, Apr 30, 2021 at 11:27:29AM +0200, Ali Abdallah wrote:
> On 28.04.2021 16:30, Pablo Neira Ayuso wrote:
> > I did not apply:
> > 
> > https://patchwork.ozlabs.org/project/netfilter-devel/patch/20210420122415.v2jtayiw3n4ds7t7@Fryzen495/
> > 
> > as you requested to send a v2.
> > 
> > Would it make sense to squash this patch and ("Reset the max ACK flag
> > on SYN in ignore state") in one single patch?
> > 
> > Thanks.
> 
> Yes, I will send a single patch then. Thanks.

Thanks.

There are three patches in patchwork now (they come with no
versioning, not sure if one of these is replaced by another).

So which ones below should be consider to be applied upstream?

https://patchwork.ozlabs.org/project/netfilter-devel/patch/20210420122415.v2jtayiw3n4ds7t7@Fryzen495/
https://patchwork.ozlabs.org/project/netfilter-devel/patch/20210428130911.cteglt52r5if7ynp@Fryzen495/
https://patchwork.ozlabs.org/project/netfilter-devel/patch/20210430093601.zibczc4cjnwx3qwn@Fryzen495/

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

* Re: [PATCH] Avoid potentially erroneos RST check
  2021-05-03 21:07     ` Pablo Neira Ayuso
@ 2021-05-04  6:55       ` Ali Abdallah
  0 siblings, 0 replies; 6+ messages in thread
From: Ali Abdallah @ 2021-05-04  6:55 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On 03.05.2021 23:07, Pablo Neira Ayuso wrote:
> Thanks.
> 
> There are three patches in patchwork now (they come with no
> versioning, not sure if one of these is replaced by another).
> 
> So which ones below should be consider to be applied upstream?
> 
> https://patchwork.ozlabs.org/project/netfilter-devel/patch/20210420122415.v2jtayiw3n4ds7t7@Fryzen495/

Please discard the above, only kindly apply the following two commits:

> https://patchwork.ozlabs.org/project/netfilter-devel/patch/20210428130911.cteglt52r5if7ynp@Fryzen495/
> https://patchwork.ozlabs.org/project/netfilter-devel/patch/20210430093601.zibczc4cjnwx3qwn@Fryzen495/

-- 
Ali Abdallah | SUSE Linux L3 Engineer
GPG fingerprint: 51A0 F4A0 C8CF C98F 842E  A9A8 B945 56F8 1C85 D0D5


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

end of thread, other threads:[~2021-05-04  6:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-28 13:11 [PATCH] Avoid potentially erroneos RST check Ali Abdallah
2021-04-28 13:23 ` Florian Westphal
2021-04-28 14:30 ` Pablo Neira Ayuso
2021-04-30  9:27   ` Ali Abdallah
2021-05-03 21:07     ` Pablo Neira Ayuso
2021-05-04  6:55       ` Ali Abdallah

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.