[Oisf-devel] RFC: stream: handle extra different SYN/ACK

[Victor Julien]

This has now been pushed into master.

On 04/08/2013 06:27 PM, Victor Julien wrote:
> Handling of multiple different SYN/ACKs
> 
> When processing the TCP 3 way handshake (3whs), Suricata's TCP stream
> engine will closely follow the setup of a TCP connection to make sure
> the rest of the session can be tracked and reassembled properly.
> Retransmissions of SYN/ACKs are silently accepted, unless they are
> different somehow. If the SEQ or ACK values are different they are
> considered wrong and events are set. The stream events rules will match
> on this.
> 
> I ran into some cases where not the initial SYN/ACK was used by the
> client, but instead a later one. Suricata however, had accepted the
> initial SYN/ACK. The result was that every packet from that point was
> rejected by the stream engine. A 67 packet pcap resulting in 64 stream
> events.
> 
> If people have the stream events enabled _and_ pay attention to them, a
> noisy session like this should certainly get their attention. However,
> many people disable the stream events, or choose to ignore them, so a
> better solution is necessary.
> 
> 
> *Analysis*
> 
> In this case the curious thing is that the extra SYN/ACK(s) have
> different properties: the sequence number is different. As the SYN/ACKs
> sequence number is used as "initial sequence number" (ISN) in the "to
> client" direction, it's crucial to track it correctly. Failing to do so,
> Suricata will loose track of the stream, causing reassembly to fail.
> This could lead to missed alerts.
> 
> Whats happening on the wire:
> 
> TCP SSN 1:
>> SYN: SEQ 10
> < SYN/ACK 1: ACK 11, SEQ 100
> < SYN/ACK 2: ACK 11, SEQ 1000
>> ACK: SEQ 11, ACK 101
> 
> TCP SSN 2:
>> SYN: SEQ 10
> < SYN/ACK 1: ACK 11, SEQ 100
>> SYN/ACK 2: ACK 11, SEQ 1000
> < ACK: SEQ 11, ACK 1001
> 
> It's clear that in SSN 1 the client ACKs the first SYN/ACK while in SSN
> 2 the 2nd SYN/ACK is ACK'd. It's likely that the first SYN/ACK was lost
> before it reached the client. Suricata accepts the first though, and
> rejects any others that are not the same.
> 
> 
> *Solution*
> 
> The solution I've been working on is to delay judgement on the extra
> SYN/ACKs until Suricata sees the ACK that completes the 3whs. At that
> point Suricata knows what the client accepted, and which SYN/ACKs were
> either ignored, or never received.
> 
> Logic in pseudo code:
> 
> Normal SYN/ACK coming in:
> 
>     UpdateState(p);
>     ssn->state = TCP_SYN_RECV;
> 
> Extra SYN/ACK packets:
> 
>     if (p != ssn) {
>         QueueState(p);
> 
> On receiving the ACK that completes the 3whs:
> 
>     if (ssn->queue_len) {
>         q = QueueFindState(p);
>         if (q)
>             UpdateState(q);
>     }
>     UpdateState(p);
>     ssn->state = TCP_ESTABLISHED;
> 
> So when receiving the ACK, Suricata first searches for the proper
> SYN/ACK on the list. If it's not found, the ACK will be processed
> normally, which means it's checked against the original SYN/ACK. If
> Suricata did have a queued state, it will first apply it to the SSN.
> Then the ACK will be processed normally, so that is can complete the
> 3whs and move the state to ESTABLISHED.
> 
> 
> *Limitations*
> 
> Queuing these states takes some memory, and for this reason there is a
> limit to the number each SSN will accept. This is configurable through a
> new stream option:
> 
> stream:
>   max-synack-queued: 5
> 
> It defaults to 5. I've seen a few (valid) hits against a few terrabytes
> of traffic, so I think the default is reasonably safe. An event is being
> set if the limit is exceeded. It can be matched using a stream-event rule:
> 
>   alert tcp any any -> any any (msg:"SURICATA STREAM 3way handshake
> excessive different SYN/ACKs"; stream-event:3whs_synack_flood;
> sid:2210055; rev:1;)
> 
> 
> *Performance*
> 
> This functionality doesn't affect the regular "fast path" except for a
> small check to see if we have queued states. However, if the queue list
> is being used Suricata enters a slow path. Currently this involves an
> memory allocation per stored queue. It may be interesting to consider
> using pools here, although a single global pool might be ineffecient. In
> such a case a lock would have to be used and this might lead to
> contention, especially in a case where Suricata would be flooded. Per
> thread pools [https://redmine.openinfosecfoundation.org/issues/519,
> https://redmine.openinfosecfoundation.org/issues/520,
> https://redmine.openinfosecfoundation.org/issues/521] may be best here.
> 
> *IPS mode*
> 
> SYN/ACKs that exceed the limit are dropped if stream.inline is enabled
> as is the case with all packets that are considered to be bad in some way.
> 
> 
> *Code*
> 
> Please review:
> https://github.com/inliniac/suricata/commit/61315e92619e32565c7e439666acad8431972f9e
> 


-- 
---------------------------------------------
Victor Julien
http://www.inliniac.net/
PGP: http://www.inliniac.net/victorjulien.asc
---------------------------------------------