[SECURITY] CERT/CC VU#464113, SYN plus RST/FIN

[SECURITY] CERT/CC VU#464113, SYN plus RST/FIN

[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	2002/10/25 08:45:20	1.1
+++ tcp_input.c	2002/10/25 08:49:21
@@ -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)

Re: [SECURITY] CERT/CC VU#464113, SYN plus RST/FIN

[Alex Riesen]

On Fri, Oct 25, 2002 at 11:00:43AM +0200, Florian Weimer wrote:
> 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	2002/10/25 08:45:20	1.1
> +++ tcp_input.c	2002/10/25 08:49:21
> @@ -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

Re: [SECURITY] CERT/CC VU#464113, SYN plus RST/FIN

[Florian Weimer]

Alex Riesen <Alexander.Riesen@synopsys.com> writes:

>> --- tcp_input.c	2002/10/25 08:45:20	1.1
>> +++ tcp_input.c	2002/10/25 08:49:21
>> @@ -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?

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.

Re: [SECURITY] CERT/CC VU#464113, SYN plus RST/FIN

[Alan Cox]

On Fri, 2002-10-25 at 10:00, Florian Weimer wrote:
> 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

Re: [SECURITY] CERT/CC VU#464113, SYN plus RST/FIN

[Alex Riesen]

On Fri, Oct 25, 2002 at 12:16:37PM +0200, Florian Weimer wrote:
> >> +		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 :)