2002-10-25 08:54:32

by Florian Weimer

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

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)

2002-10-25 10:01:45

by Alan

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

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

2002-10-25 10:10:25

by Florian Weimer

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

Alex Riesen <[email protected]> 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.

2002-10-25 10:07:08

by Alex Riesen

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

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

2002-10-25 10:27:47

by Alex Riesen

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

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 :)