Skip to contentSkip to conversation

CERT/CC VU#464113, SYN plus RST/FIN

SECURITY Florian Weimer 3 participants 5 messages Oct 25, 2002

lore
Share & tools
lore
#0 lore raw

This patch prevents SYN+RST and SYN+FIN segments which arrive in the LISTEN state from initiating a three-way handshake.

I'm not sure if it is correct, but it's better than nothing (so far, I haven't seen any patch for this issue).

tcp_input.c +2 -0
@@ -3668,6 +3668,8 @@
 	case TCP_LISTEN:
 		if(th->ack)
 			return 1;
+		if(th->rst || th->fin)
+			goto discard;
 
 		if(th->syn) {
 			if(tp->af_specific->conn_request(sk, skb) < 0)
#1 lore raw
Quoted 5 lines · Florian Weimer

This patch prevents SYN+RST and SYN+FIN segments which arrive in the LISTEN state from initiating a three-way handshake.

I'm not sure if it is correct, but it's better than nothing (so far, I haven't seen any patch for this issue).

I would disagree with the th->fin check. We don't want to break stuff that is doing T/TCP initially. (Yes the advice people gave is badly wrong - SYN|ACK|FIN is legal for example and some stacks generate it)

The th->rst is clearly correct however

#2 lore raw
In reply to Florian Weimer
Quoted 10 lines · Florian Weimer

This patch prevents SYN+RST and SYN+FIN segments which arrive in the LISTEN state from initiating a three-way handshake.

I'm not sure if it is correct, but it's better than nothing (so far, I haven't seen any patch for this issue).

tcp_input.c line 3675

Quoted 8 lines
tcp_input.c +2 -0
@@ -3668,6 +3668,8 @@
 	case TCP_LISTEN:
 		if(th->ack)
 			return 1;
+		if(th->rst || th->fin)
+			goto discard;
 
 		if(th->syn) {
 			if(tp->af_specific->conn_request(sk, skb) < 0)

You mean to place the check below "if(th->syn)", don't you?

-alex

#3 lore raw
Quoted 9 lines · Alex Riesen
Quoted 9 lines
tcp_input.c +2 -0
@@ -3668,6 +3668,8 @@
 	case TCP_LISTEN:
 		if(th->ack)
 			return 1;
+		if(th->rst || th->fin)
+			goto discard;
 
Quoted 5 lines

tcp_input.c line 3675

Quoted 3 lines
tcp_input.c +0 -0
 		if(th->syn) {
 			if(tp->af_specific->conn_request(sk, skb) < 0)

You mean to place the check below "if(th->syn)", don't you?

No, of course not. :-) That's the whole point of the patch. A SYN is not a SYN if it comes together with a RST.

#4 lore raw
Quoted 10 lines · Florian Weimer
Quoted 7 lines

tcp_input.c line 3675

Quoted 5 lines
tcp_input.c +2 -0
+		if(th->rst || th->fin)
+			goto discard;
 
 		if(th->syn) {
 			if(tp->af_specific->conn_request(sk, skb) < 0)

You mean to place the check below "if(th->syn)", don't you?

No, of course not. :-) That's the whole point of the patch. A SYN is not a SYN if it comes together with a RST.

Oh, i see :)