]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
dns: support back to back requests without a response
authorJason Ish <ish@unx.ca>
Tue, 25 Oct 2016 06:13:07 +0000 (00:13 -0600)
committerJason Ish <ish@unx.ca>
Wed, 26 Oct 2016 15:49:19 +0000 (09:49 -0600)
Address the issue where a DNS response would not be logged when
the traffic is like:
- Request 1
- Request 2
- Response 1
- Response 2
which can happen on dual stack machines where the request for A
and AAAA are sent out at the same time on the same UDP "session".

A "window" is used to set the maximum number of outstanding
responses before considering the olders lost.

src/app-layer-dns-common.c
src/app-layer-dns-common.h
src/app-layer-dns-tcp.c
src/app-layer-dns-udp.c

index 8c5bc02affa0850bb0a99003b6e3ba4635798d02..31ed2c26852cdc46f5e484575d19c9286074526a 100644 (file)
@@ -392,6 +392,9 @@ DNSTransaction *DNSTransactionFindByTxId(const DNSState *dns_state, const uint16
         TAILQ_FOREACH(tx, &dns_state->tx_list, next) {
             if (tx->tx_id == tx_id) {
                 return tx;
+            } else if ((dns_state->transaction_max - tx->tx_num) >
+                (dns_state->window - 1U)) {
+                tx->reply_lost = 1;
             }
         }
     }
@@ -554,19 +557,11 @@ void DNSStoreQueryInState(DNSState *dns_state, const uint8_t *fqdn, const uint16
         return;
     }
 
-    /* see if the last tx is unreplied */
-    if (dns_state->curr != tx && dns_state->curr != NULL &&
-        dns_state->curr->replied == 0)
-    {
-        dns_state->curr->reply_lost = 1;
-        dns_state->unreplied_cnt++;
-
-        /* check flood limit */
-        if (dns_config.request_flood != 0 &&
-                dns_state->unreplied_cnt > dns_config.request_flood) {
-            DNSSetEvent(dns_state, DNS_DECODER_EVENT_FLOODED);
-            dns_state->givenup = 1;
-        }
+    /* check flood limit */
+    if (dns_config.request_flood != 0 &&
+        dns_state->unreplied_cnt > dns_config.request_flood) {
+        DNSSetEvent(dns_state, DNS_DECODER_EVENT_FLOODED);
+        dns_state->givenup = 1;
     }
 
     if (tx == NULL) {
@@ -579,6 +574,7 @@ void DNSStoreQueryInState(DNSState *dns_state, const uint8_t *fqdn, const uint16
         dns_state->curr = tx;
         tx->tx_num = dns_state->transaction_max;
         SCLogDebug("new tx %u with internal id %u", tx->tx_id, tx->tx_num);
+        dns_state->unreplied_cnt++;
     }
 
     if (DNSCheckMemcap((sizeof(DNSQueryEntry) + fqdn_len), dns_state) < 0)
@@ -645,9 +641,6 @@ void DNSStoreAnswerInState(DNSState *dns_state, const int rtype, const uint8_t *
 
     /* mark tx is as replied so we can log it */
     tx->replied = 1;
-
-    /* reset unreplied counter */
-    dns_state->unreplied_cnt = 0;
 }
 
 /** \internal
index 184fb26408d736cddb63323bac752d1f939c7841..bc20907f6f48464d1e86db5c9b9de50189570287 100644 (file)
@@ -218,6 +218,13 @@ typedef struct DNSState_ {
                                                  state-memcap settings */
     uint64_t tx_with_detect_state_cnt;
 
+    struct timeval last_req;      /**< Timestamp of last request. */
+    struct timeval last_resp;     /**< Timestamp of last response. */
+
+    uint16_t window;              /**< Window of allowed unreplied
+                                   * requests. Set by the maximum
+                                   * number of subsequent requests
+                                   * without a response. */
     uint16_t events;
     uint16_t givenup;
 
index 73637c5770d07ffa07f9b9cc61113153c2311112..d22ab923f54d11ec0031e891c57c1477132c00e1 100644 (file)
@@ -196,6 +196,14 @@ static int DNSRequestParseData(Flow *f, DNSState *dns_state, const uint8_t *inpu
 
     //PrintRawDataFp(stdout, (uint8_t*)data, input_len - (data - input));
 
+    if (dns_state != NULL) {
+        if (timercmp(&dns_state->last_req, &dns_state->last_resp, >=)) {
+            if (dns_state->window <= dns_state->unreplied_cnt) {
+                dns_state->window++;
+            }
+        }
+    }
+
     for (q = 0; q < ntohs(dns_header->questions); q++) {
         uint8_t fqdn[DNS_MAX_SIZE];
         uint16_t fqdn_offset = 0;
@@ -355,6 +363,10 @@ next_record:
             goto bad_data;
     }
 
+    if (dns_state != NULL && f != NULL) {
+        dns_state->last_req = f->lastts;
+    }
+
     SCReturnInt(1);
 insufficient_data:
     SCReturnInt(-1);
@@ -377,6 +389,8 @@ static int DNSReponseParseData(Flow *f, DNSState *dns_state, const uint8_t *inpu
     if (!found) {
         SCLogDebug("DNS_DECODER_EVENT_UNSOLLICITED_RESPONSE");
         DNSSetEvent(dns_state, DNS_DECODER_EVENT_UNSOLLICITED_RESPONSE);
+    } else if (dns_state->unreplied_cnt > 0) {
+        dns_state->unreplied_cnt--;
     }
 
     uint16_t q;
@@ -567,6 +581,11 @@ next_record:
         if (r < 0)
             goto bad_data;
     }
+
+    if (dns_state != NULL && f != NULL) {
+        dns_state->last_req = f->lastts;
+    }
+
     SCReturnInt(1);
 insufficient_data:
     SCReturnInt(-1);
index f2636df48ab6b6dd03e4ce94a5446a6f05669c0c..e34c4b14607612f64a59b2912fe1c247ee5110fb 100644 (file)
@@ -81,6 +81,14 @@ static int DNSUDPRequestParse(Flow *f, void *dstate,
     if (DNSValidateRequestHeader(dns_state, dns_header) < 0)
         goto bad_data;
 
+    if (dns_state != NULL) {
+        if (timercmp(&dns_state->last_req, &dns_state->last_resp, >=)) {
+            if (dns_state->window <= dns_state->unreplied_cnt) {
+                dns_state->window++;
+            }
+        }
+    }
+
     uint16_t q;
     const uint8_t *data = input + sizeof(DNSHeader);
     for (q = 0; q < ntohs(dns_header->questions); q++) {
@@ -151,6 +159,10 @@ static int DNSUDPRequestParse(Flow *f, void *dstate,
         }
     }
 
+    if (dns_state != NULL && f != NULL) {
+        dns_state->last_req = f->lastts;
+    }
+
     SCReturnInt(1);
 bad_data:
 insufficient_data:
@@ -196,6 +208,8 @@ static int DNSUDPResponseParse(Flow *f, void *dstate,
     if (!found) {
         SCLogDebug("DNS_DECODER_EVENT_UNSOLLICITED_RESPONSE");
         DNSSetEvent(dns_state, DNS_DECODER_EVENT_UNSOLLICITED_RESPONSE);
+    } else if (dns_state->unreplied_cnt > 0) {
+        dns_state->unreplied_cnt--;
     }
 
     if (DNSValidateResponseHeader(dns_state, dns_header) < 0)
@@ -300,6 +314,9 @@ static int DNSUDPResponseParse(Flow *f, void *dstate,
 
         tx->replied = 1;
     }
+    if (dns_state != NULL && f != NULL) {
+        dns_state->last_resp = f->lastts;
+    }
     SCReturnInt(1);
 
 bad_data:
@@ -627,6 +644,233 @@ end:
     return (result);
 }
 
+/**
+ * \test Test subsequent requests before response.
+ *
+ * This test sends 2 DNS requests on the same state then sends the response
+ * to the first request checking that it is seen and associated with the
+ * transaction.
+ */
+static int DNSUDPParserTestDelayedResponse(void)
+{
+    /* DNS request:
+     * - Flags: 0x0100 Standard query
+     * - A www.google.com
+     */
+    uint8_t req[] = {
+        0x00, 0x01, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x03, 0x77, 0x77, 0x77,
+        0x06, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x03,
+        0x63, 0x6f, 0x6d, 0x00, 0x00, 0x01, 0x00, 0x01,
+    };
+    size_t reqlen = sizeof(req);
+
+    /* DNS response:
+     * - Flags: 0x8180 Standard query response, no error
+     * - www.google.com A 24.244.4.56
+     * - www.google.com A 24.244.4.54
+     * - www.google.com A 24.244.4.57
+     * - www.google.com A 24.244.4.55
+     * - www.google.com A 24.244.4.52
+     * - www.google.com A 24.244.4.53
+     * - www.google.com A 24.244.4.58
+     * - www.google.com A 24.244.4.59
+     */
+    uint8_t res[] = {
+        0x00, 0x01, 0x81, 0x80, 0x00, 0x01, 0x00, 0x08,
+        0x00, 0x00, 0x00, 0x00, 0x03, 0x77, 0x77, 0x77,
+        0x06, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x03,
+        0x63, 0x6f, 0x6d, 0x00, 0x00, 0x01, 0x00, 0x01,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x38,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x39,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x34,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x35,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x36,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x3b,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x37,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x3a
+    };
+    size_t reslen = sizeof(res);
+
+    DNSState *state = DNSStateAlloc();
+    FAIL_IF_NULL(state);
+    Flow *f = UTHBuildFlow(AF_INET, "1.1.1.1", "2.2.2.2", 1024, 53);
+    FAIL_IF_NULL(f);
+    f->proto = IPPROTO_UDP;
+    f->alproto = ALPROTO_DNS;
+    f->alstate = state;
+
+    /* Send to requests with an incrementing tx id. */
+    FAIL_IF_NOT(DNSUDPRequestParse(f, f->alstate, NULL, req, reqlen, NULL));
+    req[1] = 0x02;
+    FAIL_IF_NOT(DNSUDPRequestParse(f, f->alstate, NULL, req, reqlen, NULL));
+
+    /* Send response to the first request. */
+    FAIL_IF_NOT(DNSUDPResponseParse(f, f->alstate, NULL, res, reslen, NULL));
+    DNSTransaction *tx = TAILQ_FIRST(&state->tx_list);
+    FAIL_IF_NULL(tx);
+    FAIL_IF_NOT(tx->replied);
+
+    /* Also free's state. */
+    UTHFreeFlow(f);
+
+    PASS;
+}
+
+/**
+ * \test Test entering the flood/givenup state.
+ */
+static int DNSUDPParserTestFlood(void)
+{
+    /* DNS request:
+     * - Flags: 0x0100 Standard query
+     * - A www.google.com
+     */
+    uint8_t req[] = {
+        0x00, 0x01, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x03, 0x77, 0x77, 0x77,
+        0x06, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x03,
+        0x63, 0x6f, 0x6d, 0x00, 0x00, 0x01, 0x00, 0x01,
+    };
+    size_t reqlen = sizeof(req);
+
+    DNSState *state = DNSStateAlloc();
+    FAIL_IF_NULL(state);
+    Flow *f = UTHBuildFlow(AF_INET, "1.1.1.1", "2.2.2.2", 1024, 53);
+    FAIL_IF_NULL(f);
+    f->proto = IPPROTO_UDP;
+    f->alproto = ALPROTO_DNS;
+    f->alstate = state;
+
+    uint16_t txid;
+    for (txid = 1; txid <= DNS_CONFIG_DEFAULT_REQUEST_FLOOD + 1; txid++) {
+        req[0] = (txid >> 8) & 0xff;
+        req[1] = txid & 0xff;
+        FAIL_IF_NOT(DNSUDPRequestParse(f, f->alstate, NULL, req, reqlen, NULL));
+        FAIL_IF(state->givenup);
+    }
+
+    /* With one more request we should enter a flooded state. */
+    txid++;
+    req[0] = (txid >> 8) & 0xff;
+    req[1] = txid & 0xff;
+    FAIL_IF_NOT(DNSUDPRequestParse(f, f->alstate, NULL, req, reqlen, NULL));
+    FAIL_IF(!state->givenup);
+
+    /* Also free's state. */
+    UTHFreeFlow(f);
+
+    PASS;
+}
+
+static int DNSUDPParserTestLostResponse(void)
+{
+    /* DNS request:
+     * - Flags: 0x0100 Standard query
+     * - A www.google.com
+     */
+    uint8_t req[] = {
+        0x00, 0x01, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x03, 0x77, 0x77, 0x77,
+        0x06, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x03,
+        0x63, 0x6f, 0x6d, 0x00, 0x00, 0x01, 0x00, 0x01,
+    };
+    size_t reqlen = sizeof(req);
+
+    uint8_t res[] = {
+        0x00, 0x01, 0x81, 0x80, 0x00, 0x01, 0x00, 0x08,
+        0x00, 0x00, 0x00, 0x00, 0x03, 0x77, 0x77, 0x77,
+        0x06, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x03,
+        0x63, 0x6f, 0x6d, 0x00, 0x00, 0x01, 0x00, 0x01,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x38,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x39,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x34,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x35,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x36,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x3b,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x37,
+        0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+        0x01, 0x08, 0x00, 0x04, 0x18, 0xf4, 0x04, 0x3a
+    };
+    size_t reslen = sizeof(res);
+
+    DNSTransaction *tx;
+    DNSState *state = DNSStateAlloc();
+    FAIL_IF_NULL(state);
+    Flow *f = UTHBuildFlow(AF_INET, "1.1.1.1", "2.2.2.2", 1024, 53);
+    FAIL_IF_NULL(f);
+    f->proto = IPPROTO_UDP;
+    f->alproto = ALPROTO_DNS;
+    f->alstate = state;
+
+    /* First request. */
+    req[1] = 0x01;
+    FAIL_IF_NOT(DNSUDPRequestParse(f, f->alstate, NULL, req, reqlen, NULL));
+    FAIL_IF_NOT(state->transaction_max == 1);
+    FAIL_IF_NOT(state->unreplied_cnt == 1);
+    FAIL_IF_NOT(state->window == 1);
+
+    /* Second request. */
+    req[1] = 0x02;
+    FAIL_IF_NOT(DNSUDPRequestParse(f, f->alstate, NULL, req, reqlen, NULL));
+    FAIL_IF_NOT(state->transaction_max == 2);
+    FAIL_IF_NOT(state->unreplied_cnt == 2);
+    FAIL_IF_NOT(state->window == 2);
+
+    /* Third request. */
+    req[1] = 0x03;
+    FAIL_IF_NOT(DNSUDPRequestParse(f, f->alstate, NULL, req, reqlen, NULL));
+    FAIL_IF_NOT(state->transaction_max == 3);
+    FAIL_IF_NOT(state->unreplied_cnt == 3);
+    FAIL_IF_NOT(state->window == 3);
+
+    /* Now respond to the second. */
+    res[1] = 0x02;
+    FAIL_IF_NOT(DNSUDPResponseParse(f, f->alstate, NULL, res, reslen, NULL));
+    FAIL_IF_NOT(state->unreplied_cnt == 2);
+    FAIL_IF_NOT(state->window == 3);
+    tx = TAILQ_FIRST(&state->tx_list);
+    FAIL_IF_NULL(tx);
+    FAIL_IF(tx->replied);
+    FAIL_IF(tx->reply_lost);
+
+    /* Send a 4th request. */
+    req[1] = 0x04;
+    FAIL_IF_NOT(DNSUDPRequestParse(f, f->alstate, NULL, req, reqlen, NULL));
+    FAIL_IF_NOT(state->unreplied_cnt == 3);
+    FAIL_IF(state->window != 3);
+    FAIL_IF_NOT(state->transaction_max == 4);
+
+    /* Response to the third request. */
+    res[1] = 0x03;
+    FAIL_IF_NOT(DNSUDPResponseParse(f, f->alstate, NULL, res, reslen, NULL));
+    FAIL_IF_NOT(state->unreplied_cnt == 2);
+    FAIL_IF_NOT(state->window == 3);
+    tx = TAILQ_FIRST(&state->tx_list);
+    FAIL_IF_NULL(tx);
+    FAIL_IF(tx->replied);
+    FAIL_IF(!tx->reply_lost);
+
+    /* Also free's state. */
+    UTHFreeFlow(f);
+
+    PASS;
+}
 
 void DNSUDPParserRegisterTests(void)
 {
@@ -635,5 +879,10 @@ void DNSUDPParserRegisterTests(void)
     UtRegisterTest("DNSUDPParserTest03", DNSUDPParserTest03);
     UtRegisterTest("DNSUDPParserTest04", DNSUDPParserTest04);
     UtRegisterTest("DNSUDPParserTest05", DNSUDPParserTest05);
+    UtRegisterTest("DNSUDPParserTestFlood", DNSUDPParserTestFlood);
+    UtRegisterTest("DNSUDPParserTestDelayedResponse",
+        DNSUDPParserTestDelayedResponse);
+    UtRegisterTest("DNSUDPParserTestLostResponse",
+        DNSUDPParserTestLostResponse);
 }
 #endif