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

[Victor Julien]

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