From: Victor Julien <victor@inliniac.net>
Date: Fri, 6 May 2016 10:43:11 +0000 (+0200)
Subject: tcp: fix alignment issues with tcp timestamps
X-Git-Tag: suricata-3.1RC1~191
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=76c8c077c59e4f83e5f0558d23639649612322e7;p=thirdparty%2Fsuricata.git

tcp: fix alignment issues with tcp timestamps
---

diff --git a/src/decode-tcp.c b/src/decode-tcp.c
index 36a63d3c30..b3c73b5d89 100644
--- a/src/decode-tcp.c
+++ b/src/decode-tcp.c
@@ -124,10 +124,14 @@ static int DecodeTCPOptions(Packet *p, uint8_t *pkt, uint16_t len)
                     if (tcp_opts[tcp_opt_cnt].len != TCP_OPT_TS_LEN) {
                         ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN);
                     } else {
-                        if (p->tcpvars.ts.type != 0) {
+                        if (p->tcpvars.ts_set) {
                             ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE);
                         } else {
-                            SET_OPTS(p->tcpvars.ts, tcp_opts[tcp_opt_cnt]);
+                            uint32_t values[2];
+                            memcpy(&values, tcp_opts[tcp_opt_cnt].data, sizeof(values));
+                            p->tcpvars.ts_val = ntohl(values[0]);
+                            p->tcpvars.ts_ecr = ntohl(values[1]);
+                            p->tcpvars.ts_set = TRUE;
                         }
                     }
                     break;
diff --git a/src/decode-tcp.h b/src/decode-tcp.h
index 955800e17a..72ed817974 100644
--- a/src/decode-tcp.h
+++ b/src/decode-tcp.h
@@ -76,20 +76,16 @@
 #define TCP_GET_RAW_WINDOW(tcph)             ntohs((tcph)->th_win)
 #define TCP_GET_RAW_URG_POINTER(tcph)        ntohs((tcph)->th_urp)
 
-/** macro for getting the first timestamp from the packet. Timestamp is in host
- *  order and either returned from the cache or from the packet directly. */
-#define TCP_GET_TSVAL(p) \
-    (uint32_t)ntohl((*(uint32_t *)(p)->tcpvars.ts.data))
+/** macro for getting the first timestamp from the packet in host order */
+#define TCP_GET_TSVAL(p)                    ((p)->tcpvars.ts_val)
 
-/** macro for getting the second timestamp from the packet. Timestamp is in
- *  host order and either returned from the cache or from the packet directly. */
-#define TCP_GET_TSECR(p) \
-    (uint32_t)ntohl((*(uint32_t *)((p)->tcpvars.ts.data+4)))
+/** macro for getting the second timestamp from the packet in host order. */
+#define TCP_GET_TSECR(p)                    ((p)->tcpvars.ts_ecr)
 
 #define TCP_HAS_WSCALE(p)                   ((p)->tcpvars.ws.type == TCP_OPT_WS)
 #define TCP_HAS_SACK(p)                     ((p)->tcpvars.sack.type == TCP_OPT_SACK)
 #define TCP_HAS_SACKOK(p)                   ((p)->tcpvars.sackok.type == TCP_OPT_SACKOK)
-#define TCP_HAS_TS(p)                       ((p)->tcpvars.ts.type == TCP_OPT_TS)
+#define TCP_HAS_TS(p)                       ((p)->tcpvars.ts_set == TRUE)
 #define TCP_HAS_MSS(p)                      ((p)->tcpvars.mss.type == TCP_OPT_MSS)
 
 /** macro for getting the wscale from the packet. */
@@ -146,7 +142,9 @@ typedef struct TCPHdr_
 typedef struct TCPVars_
 {
     /* commonly used and needed opts */
-    TCPOpt ts;
+    _Bool ts_set;
+    uint32_t ts_val;    /* host-order */
+    uint32_t ts_ecr;    /* host-order */
     TCPOpt sack;
     TCPOpt sackok;
     TCPOpt ws;
@@ -156,7 +154,9 @@ typedef struct TCPVars_
 #define CLEAR_TCP_PACKET(p) { \
     (p)->tcph = NULL; \
     (p)->level4_comp_csum = -1; \
-    (p)->tcpvars.ts.type = 0; \
+    (p)->tcpvars.ts_set = FALSE; \
+    (p)->tcpvars.ts_val = 0; \
+    (p)->tcpvars.ts_ecr = 0; \
     (p)->tcpvars.sack.type = 0; \
     (p)->tcpvars.sackok.type = 0; \
     (p)->tcpvars.ws.type = 0; \
diff --git a/src/stream-tcp.c b/src/stream-tcp.c
index f17a761369..875a9d6f3e 100644
--- a/src/stream-tcp.c
+++ b/src/stream-tcp.c
@@ -6436,14 +6436,12 @@ end:
 static int StreamTcpTest07 (void)
 {
     Packet *p = SCMalloc(SIZE_OF_PACKET);
-    if (unlikely(p == NULL))
-        return 0;
+    FAIL_IF(unlikely(p == NULL));
     Flow f;
     ThreadVars tv;
     StreamTcpThread stt;
     TCPHdr tcph;
     uint8_t payload[1] = {0x42};
-    uint32_t data[2];
     PacketQueue pq;
 
     memset(p, 0, SIZE_OF_PACKET);
@@ -6455,7 +6453,6 @@ static int StreamTcpTest07 (void)
 
     FLOW_INITIALIZE(&f);
     p->flow = &f;
-    int ret = 0;
 
     StreamTcpInitConfig(TRUE);
     stream_config.midstream = TRUE;
@@ -6470,45 +6467,33 @@ static int StreamTcpTest07 (void)
     tcph.th_flags = TH_ACK|TH_PUSH;
     p->tcph = &tcph;
 
-    data[0] = htonl(10);
-    data[1] = htonl(11);
-
-    p->tcpvars.ts.type = TCP_OPT_TS;
-    p->tcpvars.ts.len = 10;
-    p->tcpvars.ts.data = (uint8_t *)data;
+    p->tcpvars.ts_set = TRUE;
+    p->tcpvars.ts_val = 10;
+    p->tcpvars.ts_ecr = 11;
 
     p->payload = payload;
     p->payload_len = 1;
 
     SCMutexLock(&f.m);
-    if (StreamTcpPacket(&tv, p, &stt, &pq) == -1)
-        goto end;
+    FAIL_IF(StreamTcpPacket(&tv, p, &stt, &pq) == -1);
 
     p->tcph->th_seq = htonl(11);
     p->tcph->th_ack = htonl(23);
     p->tcph->th_flags = TH_ACK|TH_PUSH;
     p->flowflags = FLOW_PKT_TOSERVER;
 
-    data[0] = htonl(2);
-    p->tcpvars.ts.data = (uint8_t *)data;
+    p->tcpvars.ts_val = 2;
 
-    if (StreamTcpPacket(&tv, p, &stt, &pq) == -1) {
-        if (((TcpSession *) (p->flow->protoctx))->client.next_seq != 11) {
-            printf("the timestamp values are client %"PRIu32" server %" PRIu32""
-                    " seq %" PRIu32 "\n", TCP_GET_TSVAL(p), TCP_GET_TSECR(p),
-                    ((TcpSession *) (p->flow->protoctx))->client.next_seq);
-            goto end;
-        }
+    FAIL_IF(StreamTcpPacket(&tv, p, &stt, &pq) != -1);
 
-        StreamTcpSessionClear(p->flow->protoctx);
-        ret = 1;
-    }
-end:
+    FAIL_IF (((TcpSession *) (p->flow->protoctx))->client.next_seq != 11);
+
+    StreamTcpSessionClear(p->flow->protoctx);
     StreamTcpFreeConfig(TRUE);
     SCMutexUnlock(&f.m);
     SCFree(p);
     FLOW_DESTROY(&f);
-    return ret;
+    PASS;
 }
 
 /**
@@ -6520,16 +6505,13 @@ end:
 
 static int StreamTcpTest08 (void)
 {
-
     Packet *p = SCMalloc(SIZE_OF_PACKET);
-    if (unlikely(p == NULL))
-    return 0;
+    FAIL_IF(unlikely(p == NULL));
     Flow f;
     ThreadVars tv;
     StreamTcpThread stt;
     TCPHdr tcph;
     uint8_t payload[1] = {0x42};
-    uint32_t data[2];
 
     memset(p, 0, SIZE_OF_PACKET);
     PacketQueue pq;
@@ -6541,7 +6523,6 @@ static int StreamTcpTest08 (void)
 
     FLOW_INITIALIZE(&f);
     p->flow = &f;
-    int ret = 0;
 
     StreamTcpInitConfig(TRUE);
     stream_config.midstream = TRUE;
@@ -6556,47 +6537,34 @@ static int StreamTcpTest08 (void)
     tcph.th_flags = TH_ACK|TH_PUSH;
     p->tcph = &tcph;
 
-    data[0] = htonl(10);
-    data[1] = htonl(11);
-
-    p->tcpvars.ts.type = TCP_OPT_TS;
-    p->tcpvars.ts.len = 10;
-    p->tcpvars.ts.data = (uint8_t *)data;
+    p->tcpvars.ts_set = TRUE;
+    p->tcpvars.ts_val = 10;
+    p->tcpvars.ts_ecr = 11;
 
     p->payload = payload;
     p->payload_len = 1;
 
     SCMutexLock(&f.m);
-    if (StreamTcpPacket(&tv, p, &stt, &pq) == -1)
-        goto end;
+    FAIL_IF(StreamTcpPacket(&tv, p, &stt, &pq) == -1);
 
     p->tcph->th_seq = htonl(11);
     p->tcph->th_ack = htonl(20);
     p->tcph->th_flags = TH_ACK|TH_PUSH;
     p->flowflags = FLOW_PKT_TOSERVER;
 
-    data[0] = htonl(12);
-    p->tcpvars.ts.data = (uint8_t *)data;
+    p->tcpvars.ts_val = 12;
 
-    if (StreamTcpPacket(&tv, p, &stt, &pq) == -1)
-        goto end;
+    FAIL_IF(StreamTcpPacket(&tv, p, &stt, &pq) == -1);
 
-    if (((TcpSession *) (p->flow->protoctx))->client.next_seq != 12) {
-        printf("the timestamp values are client %"PRIu32" server %" PRIu32 " "
-                "seq %" PRIu32 "\n", TCP_GET_TSVAL(p), TCP_GET_TSECR(p),
-                ((TcpSession *) (p->flow->protoctx))->client.next_seq);
-        goto end;
-    }
+    FAIL_IF(((TcpSession *) (p->flow->protoctx))->client.next_seq != 12);
 
     StreamTcpSessionClear(p->flow->protoctx);
 
-    ret = 1;
-end:
     StreamTcpFreeConfig(TRUE);
     SCMutexUnlock(&f.m);
     SCFree(p);
     FLOW_DESTROY(&f);
-    return ret;
+    PASS;
 }
 
 /**