aiden
/
suricata
mirror of https://github.com/OISF/suricata
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

stream: fix bad last_ack update leading to gaps

Browse Source 
A bad last_ack update where it would be set beyond next_seq could
lead to rejection of valid segments and thus stream gaps.

Update tests to reflect new last_ack/next_seq behaviour.
pull/1397/head
Victor Julien 11 years ago
parent 4e177bc9d6
commit 8e83d0073e
1 changed files with 92 additions and 20 deletions
Show Stats Download Patch File Download Diff File

112
src/stream-tcp.c
Unescape Escape View File

@ -753,18 +753,25 @@ uint32_t StreamTcpGetStreamSize(TcpStream *stream)
} }
/** /**
* \brief macro to update last_ack only if the new value is higher * \brief macro to update last_ack only if the new value is higher and
* the ack value isn't beyond the next_seq
* *
* \param ssn session * \param ssn session
* \param stream stream to update * \param stream stream to update
* \param ack ACK value to test and set * \param ack ACK value to test and set
*/ */
#define StreamTcpUpdateLastAck(ssn, stream, ack) { \ #define StreamTcpUpdateLastAck(ssn, stream, ack) { \
if (SEQ_GT((ack), (stream)->last_ack)) { \ if (SEQ_GT((ack), (stream)->last_ack) && \
(SEQ_LEQ((ack),(stream)->next_seq) || \
((ssn)->state >= TCP_FIN_WAIT1 && SEQ_EQ((ack),((stream)->next_seq + 1))))) \
{ \
(stream)->last_ack = (ack); \ (stream)->last_ack = (ack); \
SCLogDebug("ssn %p: last_ack set to %"PRIu32, (ssn), (stream)->last_ack); \ SCLogDebug("ssn %p: last_ack set to %"PRIu32, (ssn), (stream)->last_ack); \
StreamTcpSackPruneList((stream)); \ StreamTcpSackPruneList((stream)); \
} \ } else { \
SCLogDebug("ssn %p: no update: ack %u, last_ack %"PRIu32", next_seq %u (state %u)", \
(ssn), (ack), (stream)->last_ack, (stream)->next_seq, (ssn)->state); \
}\
} }
/** /**
@ -2036,6 +2043,7 @@ static int HandleEstablishedPacketToServer(ThreadVars *tv, TcpSession *ssn, Pack
/* Check if the ACK value is sane and inside the window limit */ /* Check if the ACK value is sane and inside the window limit */
StreamTcpUpdateLastAck(ssn, &ssn->server, TCP_GET_ACK(p)); StreamTcpUpdateLastAck(ssn, &ssn->server, TCP_GET_ACK(p));
SCLogDebug("ack %u last_ack %u next_seq %u", TCP_GET_ACK(p), ssn->server.last_ack, ssn->server.next_seq);
if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
StreamTcpHandleTimestamp(ssn, p); StreamTcpHandleTimestamp(ssn, p);
@ -9910,12 +9918,11 @@ static int StreamTcpTest38 (void)
Flow f; Flow f;
ThreadVars tv; ThreadVars tv;
StreamTcpThread stt; StreamTcpThread stt;
uint8_t payload[4]; uint8_t payload[128];
TCPHdr tcph; TCPHdr tcph;
TcpReassemblyThreadCtx ra_ctx;
PacketQueue pq; PacketQueue pq;
memset(&ra_ctx, 0, sizeof(TcpReassemblyThreadCtx)); TcpReassemblyThreadCtx *ra_ctx = StreamTcpReassembleInitThreadCtx(NULL);
memset (&f, 0, sizeof(Flow)); memset (&f, 0, sizeof(Flow));
memset(&tv, 0, sizeof (ThreadVars)); memset(&tv, 0, sizeof (ThreadVars));
memset(&stt, 0, sizeof (StreamTcpThread)); memset(&stt, 0, sizeof (StreamTcpThread));
@ -9933,7 +9940,7 @@ static int StreamTcpTest38 (void)
tcph.th_flags = TH_SYN; tcph.th_flags = TH_SYN;
p->tcph = &tcph; p->tcph = &tcph;
p->flowflags = FLOW_PKT_TOSERVER; p->flowflags = FLOW_PKT_TOSERVER;
stt.ra_ctx = &ra_ctx; stt.ra_ctx = ra_ctx;
StreamTcpInitConfig(TRUE); StreamTcpInitConfig(TRUE);
SCMutexLock(&f.m); SCMutexLock(&f.m);
@ -9983,7 +9990,27 @@ static int StreamTcpTest38 (void)
goto end; goto end;
} }
p->tcph->th_ack = htonl(2984); p->tcph->th_ack = htonl(1);
p->tcph->th_seq = htonl(1);
p->tcph->th_flags = TH_PUSH | TH_ACK;
p->flowflags = FLOW_PKT_TOCLIENT;
StreamTcpCreateTestPacket(payload, 0x41, 127, 128); /*AAA*/
p->payload = payload;
p->payload_len = 127;
if (StreamTcpPacket(&tv, p, &stt, &pq) == -1) {
printf("failed in processing packet in StreamTcpPacket\n");
goto end;
}
if (((TcpSession *)(p->flow->protoctx))->server.next_seq != 128) {
printf("the server.next_seq should be 128, but it is %"PRIu32"\n",
((TcpSession *)(p->flow->protoctx))->server.next_seq);
goto end;
}
p->tcph->th_ack = htonl(256); // in window, but beyond next_seq
p->tcph->th_seq = htonl(5); p->tcph->th_seq = htonl(5);
p->tcph->th_flags = TH_PUSH | TH_ACK; p->tcph->th_flags = TH_PUSH | TH_ACK;
p->flowflags = FLOW_PKT_TOSERVER; p->flowflags = FLOW_PKT_TOSERVER;
@ -9997,10 +10024,32 @@ static int StreamTcpTest38 (void)
goto end; goto end;
} }
/* last_ack value should be 1, not 256, as the previous sent ACK value
is inside window, but beyond next_seq */
if (((TcpSession *)(p->flow->protoctx))->server.last_ack != 1) {
printf("the server.last_ack should be 1, but it is %"PRIu32"\n",
((TcpSession *)(p->flow->protoctx))->server.last_ack);
goto end;
}
p->tcph->th_ack = htonl(128);
p->tcph->th_seq = htonl(8);
p->tcph->th_flags = TH_PUSH | TH_ACK;
p->flowflags = FLOW_PKT_TOSERVER;
StreamTcpCreateTestPacket(payload, 0x41, 3, 4); /*AAA*/
p->payload = payload;
p->payload_len = 3;
if (StreamTcpPacket(&tv, p, &stt, &pq) == -1) {
printf("failed in processing packet in StreamTcpPacket\n");
goto end;
}
/* last_ack value should be 2984 as the previous sent ACK value is inside /* last_ack value should be 2984 as the previous sent ACK value is inside
window */ window */
if (((TcpSession *)(p->flow->protoctx))->server.last_ack != 2984) { if (((TcpSession *)(p->flow->protoctx))->server.last_ack != 128) {
printf("the server.last_ack should be 2984, but it is %"PRIu32"\n", printf("the server.last_ack should be 128, but it is %"PRIu32"\n",
((TcpSession *)(p->flow->protoctx))->server.last_ack); ((TcpSession *)(p->flow->protoctx))->server.last_ack);
goto end; goto end;
} }
@ -10013,6 +10062,8 @@ end:
SCMutexUnlock(&f.m); SCMutexUnlock(&f.m);
SCFree(p); SCFree(p);
FLOW_DESTROY(&f); FLOW_DESTROY(&f);
if (stt.ra_ctx != NULL)
StreamTcpReassembleFreeThreadCtx(stt.ra_ctx);
return ret; return ret;
} }
@ -10030,10 +10081,9 @@ static int StreamTcpTest39 (void)
StreamTcpThread stt; StreamTcpThread stt;
uint8_t payload[4]; uint8_t payload[4];
TCPHdr tcph; TCPHdr tcph;
TcpReassemblyThreadCtx ra_ctx;
PacketQueue pq; PacketQueue pq;
memset(&ra_ctx, 0, sizeof(TcpReassemblyThreadCtx)); TcpReassemblyThreadCtx *ra_ctx = StreamTcpReassembleInitThreadCtx(NULL);
memset (&f, 0, sizeof(Flow)); memset (&f, 0, sizeof(Flow));
memset(&tv, 0, sizeof (ThreadVars)); memset(&tv, 0, sizeof (ThreadVars));
memset(&stt, 0, sizeof (StreamTcpThread)); memset(&stt, 0, sizeof (StreamTcpThread));
@ -10052,7 +10102,7 @@ static int StreamTcpTest39 (void)
p->tcph = &tcph; p->tcph = &tcph;
p->flowflags = FLOW_PKT_TOSERVER; p->flowflags = FLOW_PKT_TOSERVER;
int ret = 0; int ret = 0;
stt.ra_ctx = &ra_ctx; stt.ra_ctx = ra_ctx;
StreamTcpInitConfig(TRUE); StreamTcpInitConfig(TRUE);
@ -10081,7 +10131,27 @@ static int StreamTcpTest39 (void)
goto end; goto end;
} }
p->tcph->th_ack = htonl(2984); p->tcph->th_ack = htonl(1);
p->tcph->th_seq = htonl(1);
p->tcph->th_flags = TH_PUSH | TH_ACK;
p->flowflags = FLOW_PKT_TOCLIENT;
StreamTcpCreateTestPacket(payload, 0x41, 3, 4); /*AAA*/
p->payload = payload;
p->payload_len = 3;
if (StreamTcpPacket(&tv, p, &stt, &pq) == -1) {
printf("failed in processing packet in StreamTcpPacket\n");
goto end;
}
if (((TcpSession *)(p->flow->protoctx))->server.next_seq != 4) {
printf("the server.next_seq should be 4, but it is %"PRIu32"\n",
((TcpSession *)(p->flow->protoctx))->server.next_seq);
goto end;
}
p->tcph->th_ack = htonl(4);
p->tcph->th_seq = htonl(2); p->tcph->th_seq = htonl(2);
p->tcph->th_flags = TH_PUSH | TH_ACK; p->tcph->th_flags = TH_PUSH | TH_ACK;
p->flowflags = FLOW_PKT_TOSERVER; p->flowflags = FLOW_PKT_TOSERVER;
@ -10095,15 +10165,15 @@ static int StreamTcpTest39 (void)
goto end; goto end;
} }
/* last_ack value should be 2984 as the previous sent ACK value is inside /* last_ack value should be 4 as the previous sent ACK value is inside
window */ window */
if (((TcpSession *)(p->flow->protoctx))->server.last_ack != 2984) { if (((TcpSession *)(p->flow->protoctx))->server.last_ack != 4) {
printf("the server.last_ack should be 2984, but it is %"PRIu32"\n", printf("the server.last_ack should be 4, but it is %"PRIu32"\n",
((TcpSession *)(p->flow->protoctx))->server.last_ack); ((TcpSession *)(p->flow->protoctx))->server.last_ack);
goto end; goto end;
} }
p->tcph->th_seq = htonl(2984); p->tcph->th_seq = htonl(4);
p->tcph->th_ack = htonl(5); p->tcph->th_ack = htonl(5);
p->tcph->th_flags = TH_PUSH | TH_ACK; p->tcph->th_flags = TH_PUSH | TH_ACK;
p->flowflags = FLOW_PKT_TOCLIENT; p->flowflags = FLOW_PKT_TOCLIENT;
@ -10119,8 +10189,8 @@ static int StreamTcpTest39 (void)
/* next_seq value should be 2987 as the previous sent ACK value is inside /* next_seq value should be 2987 as the previous sent ACK value is inside
window */ window */
if (((TcpSession *)(p->flow->protoctx))->server.next_seq != 2987) { if (((TcpSession *)(p->flow->protoctx))->server.next_seq != 7) {
printf("the server.next_seq should be 2987, but it is %"PRIu32"\n", printf("the server.next_seq should be 7, but it is %"PRIu32"\n",
((TcpSession *)(p->flow->protoctx))->server.next_seq); ((TcpSession *)(p->flow->protoctx))->server.next_seq);
goto end; goto end;
} }
@ -10133,6 +10203,8 @@ end:
SCMutexUnlock(&f.m); SCMutexUnlock(&f.m);
SCFree(p); SCFree(p);
FLOW_DESTROY(&f); FLOW_DESTROY(&f);
if (stt.ra_ctx != NULL)
StreamTcpReassembleFreeThreadCtx(stt.ra_ctx);
return ret; return ret;
} }

Write Preview
Loading…
Cancel
Save