]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
dnp3: fixed wrong flow direction identification
authorIlya Bakhtin <ilya.bakhtin@gmail.com>
Tue, 25 Aug 2020 13:01:22 +0000 (15:01 +0200)
committerVictor Julien <victor@inliniac.net>
Wed, 7 Oct 2020 16:41:09 +0000 (18:41 +0200)
dnp3 is a communication between so-called master and outstation
in our terms master is a client and outstation is a server
DIR flag in dnp3 header is nonzero when a packet is from master
so if DIR is nonzero then packet is 'toserver'

src/app-layer-dnp3.c

index f5b42cc1afd69aad6d81408ce4913282cdb3c1f8..edf102c5e2d7acf95d8f88b6cd5e644beea17e86 100644 (file)
@@ -271,7 +271,17 @@ static uint16_t DNP3ProbingParser(Flow *f, uint8_t direction,
         const uint8_t *input, uint32_t len,
         uint8_t *rdir)
 {
-    DNP3LinkHeader *hdr = (DNP3LinkHeader *)input;
+    const DNP3LinkHeader* const hdr = (const DNP3LinkHeader*)input;
+    const bool toserver = (direction & STREAM_TOSERVER) != 0;
+
+    /* May be a banner. */
+    if (DNP3ContainsBanner(input, len)) {
+        SCLogDebug("Packet contains a DNP3 banner.");
+        if (toserver) {
+            *rdir = STREAM_TOCLIENT;
+        }
+        return ALPROTO_DNP3;
+    }
 
     /* Check that we have the minimum amount of bytes. */
     if (len < sizeof(DNP3LinkHeader)) {
@@ -279,12 +289,6 @@ static uint16_t DNP3ProbingParser(Flow *f, uint8_t direction,
         return ALPROTO_UNKNOWN;
     }
 
-    /* May be a banner. */
-    if (DNP3ContainsBanner(input, len)) {
-        SCLogDebug("Packet contains a DNP3 banner.");
-        goto end;
-    }
-
     /* Verify start value (from AN2013-004b). */
     if (!DNP3CheckStartBytes(hdr)) {
         SCLogDebug("Invalid start bytes.");
@@ -297,11 +301,9 @@ static uint16_t DNP3ProbingParser(Flow *f, uint8_t direction,
         return ALPROTO_FAILED;
     }
 
-end:
     // Test compatibility between direction and dnp3.ctl.direction
-    if ((DNP3_LINK_DIR(hdr->control) != 0) ^
-        ((direction & STREAM_TOCLIENT) != 0)) {
-        *rdir = 1;
+    if ((DNP3_LINK_DIR(hdr->control) != 0) != toserver) {
+        *rdir = toserver ? STREAM_TOCLIENT : STREAM_TOSERVER;
     }
     SCLogDebug("Detected DNP3.");
     return ALPROTO_DNP3;
@@ -2103,6 +2105,7 @@ static int DNP3ProbingParserTest(void)
     /* Send a banner. */
     char mybanner[] = "Welcome to DNP3 SCADA.";
     FAIL_IF(DNP3ProbingParser(NULL, STREAM_TOSERVER, (uint8_t *)mybanner, sizeof(mybanner) - 1, &rdir) != ALPROTO_DNP3);
+    FAIL_IF(rdir != STREAM_TOCLIENT);
 
     PASS;
 }