diff --git a/doc/userguide/rules/dnp3-keywords.rst b/doc/userguide/rules/dnp3-keywords.rst index 64b14cd3d2..665539e2d9 100644 --- a/doc/userguide/rules/dnp3-keywords.rst +++ b/doc/userguide/rules/dnp3-keywords.rst @@ -152,3 +152,36 @@ Example :: dnp3_data; content:"|c3 06|"; + +DNP3 link-layer events +---------------------- + +Besides the link-layer anomaly events ``dnp3.bad_link_crc``, +``dnp3.len_too_small`` and ``dnp3.bad_transport_crc``, two events describe +how the parser recovered from a link-layer error: + +dnp3.bad_start_bytes + Octets that are not a DNP3 start sequence (``0x05 0x64``) were skipped. + Raised only with ``app-layer.protocols.dnp3.resync-on-bad-start`` set to + ``yes``; otherwise such octets end DNP3 inspection of the flow. With the + option set, protocol detection also looks up to 256 octets into the flow + for a start sequence with a valid header CRC, and after a bad transport + CRC (``dnp3.bad_transport_crc``) the parser searches for the next start + sequence from the octet after the failed frame's start, instead of + skipping the frame by its ``LENGTH``, so frames embedded in a corrupted + frame are parsed, as a resynchronizing endpoint would. + +dnp3.after_resync + The first well-formed frame parsed after the parser recovered from a + link-layer error on the same flow direction. A DNP3 endpoint that stops on link errors (for example + ``dnp3-rs`` with ``LinkErrorMode::Close``) would have dropped the session + before this frame; one that resynchronizes (``opendnp3``, ``dnp3-rs`` + ``Discard``) processes it. Which one the device does is a device setting + that is not visible on the wire, so this event marks where processing + starts to depend on it. It is raised once per link-layer error, on the + first frame that follows it, not on every later frame of the session. + +Example:: + + alert dnp3 any any -> any any (msg:"DNP3 frame behind a link error"; \ + app-layer-event:dnp3.after_resync; sid:1; rev:1;) diff --git a/rules/dnp3-events.rules b/rules/dnp3-events.rules index 983a5ae5f0..e9d4476c86 100644 --- a/rules/dnp3-events.rules +++ b/rules/dnp3-events.rules @@ -40,3 +40,8 @@ alert dnp3 any any -> any any (msg:"SURICATA DNP3 Too many objects"; \ alert dnp3 any any -> any any (msg:"SURICATA DNP3 Too long reassembly"; \ app-layer-event:dnp3.too_long_reassembly; \ classtype:protocol-command-decode; sid:2270007; rev:1;) + +alert dnp3 any any -> any any (msg:"SURICATA DNP3 Start bytes not 0x05 0x64"; \ + app-layer-event:dnp3.bad_start_bytes; classtype:protocol-command-decode; sid:2270010; rev:1;) +alert dnp3 any any -> any any (msg:"SURICATA DNP3 Frame parsed after a link-layer error"; \ + app-layer-event:dnp3.after_resync; classtype:protocol-command-decode; sid:2270011; rev:1;) diff --git a/src/app-layer-dnp3.c b/src/app-layer-dnp3.c index 9db71e9916..5524e2b263 100644 --- a/src/app-layer-dnp3.c +++ b/src/app-layer-dnp3.c @@ -45,6 +45,15 @@ #define DNP3_START_BYTE0 0x05 #define DNP3_START_BYTE1 0x64 +/* app-layer.protocols.dnp3.resync-on-bad-start: when yes, octets that are not + * a start sequence are skipped (like a bad CRC) instead of ending DNP3 + * inspection of the flow. Default: no. */ +static int dnp3_resync_on_bad_start = 0; + +/* With resync-on-bad-start, how far into the stream the probing parser + * searches for a start sequence with a valid header CRC. */ +#define DNP3_RESYNC_PROBE_DEPTH 256 + /* Minimum length for a DNP3 frame. */ #define DNP3_MIN_LEN 5 @@ -109,6 +118,8 @@ SCEnumCharMap dnp3_decoder_event_table[] = { { "TOO_MANY_POINTS", DNP3_DECODER_EVENT_TOO_MANY_POINTS }, { "TOO_MANY_OBJECTS", DNP3_DECODER_EVENT_TOO_MANY_OBJECTS }, { "TOO_LONG_REASSEMBLY", DNP3_DECODER_EVENT_TOO_LONG_REASS }, + { "BAD_START_BYTES", DNP3_DECODER_EVENT_BAD_START_BYTES }, + { "AFTER_RESYNC", DNP3_DECODER_EVENT_AFTER_RESYNC }, { NULL, -1 }, }; @@ -277,6 +288,27 @@ static int DNP3ContainsBanner(const uint8_t *input, uint32_t len) return BasicSearch(input, len, (uint8_t *)DNP3_BANNER, strlen(DNP3_BANNER)) != NULL; } +/** + * \brief Checks shared by the probing parser for a header with valid start + * bytes: minimum length, and direction of the flow. + */ +static uint16_t DNP3ProbingCheckHeader( + const DNP3LinkHeader *const hdr, const bool toserver, uint8_t *rdir) +{ + /* Verify minimum length. */ + if (hdr->len < DNP3_MIN_LEN) { + SCLogDebug("Packet too small to be a valid DNP3 fragment."); + return ALPROTO_FAILED; + } + + // Test compatibility between direction and dnp3.ctl.direction + if ((DNP3_LINK_DIR(hdr->control) != 0) != toserver) { + *rdir = toserver ? STREAM_TOCLIENT : STREAM_TOSERVER; + } + SCLogDebug("Detected DNP3."); + return ALPROTO_DNP3; +} + /** * \brief DNP3 probing parser. */ @@ -313,22 +345,27 @@ static uint16_t DNP3ProbingParser( /* Verify start value (from AN2013-004b). */ if (!DNP3CheckStartBytes(hdr)) { - SCLogDebug("Invalid start bytes."); - return ALPROTO_FAILED; - } - - /* Verify minimum length. */ - if (hdr->len < DNP3_MIN_LEN) { - SCLogDebug("Packet too small to be a valid DNP3 fragment."); - return ALPROTO_FAILED; + if (!dnp3_resync_on_bad_start) { + SCLogDebug("Invalid start bytes."); + return ALPROTO_FAILED; + } + /* Resynchronizing mode: the device may skip these octets, so look + * for a start sequence with a valid header CRC and length further + * in, and check it like a header at offset 0; wait for more data + * if none yet. */ + for (uint32_t i = 1; i + 1 < len; i++) { + if (input[i] != DNP3_START_BYTE0 || input[i + 1] != DNP3_START_BYTE1) + continue; + if (len - i < sizeof(DNP3LinkHeader)) + return ALPROTO_UNKNOWN; + const DNP3LinkHeader *const found = (const DNP3LinkHeader *)(input + i); + if (found->len >= DNP3_MIN_LEN && DNP3CheckLinkHeaderCRC(found)) + return DNP3ProbingCheckHeader(found, toserver, rdir); + } + return len < DNP3_RESYNC_PROBE_DEPTH ? ALPROTO_UNKNOWN : ALPROTO_FAILED; } - // Test compatibility between direction and dnp3.ctl.direction - if ((DNP3_LINK_DIR(hdr->control) != 0) != toserver) { - *rdir = toserver ? STREAM_TOCLIENT : STREAM_TOSERVER; - } - SCLogDebug("Detected DNP3."); - return ALPROTO_DNP3; + return DNP3ProbingCheckHeader(hdr, toserver, rdir); } /** @@ -1104,12 +1141,19 @@ static int DNP3HandleRequestLinkLayer( DNP3LinkHeader *header = (DNP3LinkHeader *)input; if (!DNP3CheckStartBytes(header)) { - /* Terminal error condition. */ - SCReturnInt(-1); + if (!dnp3_resync_on_bad_start) { + /* Terminal error condition. */ + SCReturnInt(-1); + } + DNP3SetEvent(dnp3, true, DNP3_DECODER_EVENT_BAD_START_BYTES); + dnp3->link_error_ts = true; + DNP3Resync(&input, &input_len, &processed); + continue; } if (!DNP3CheckLinkHeaderCRC(header)) { DNP3SetEvent(dnp3, true, DNP3_DECODER_EVENT_BAD_LINK_CRC); + dnp3->link_error_ts = true; DNP3Resync(&input, &input_len, &processed); continue; } @@ -1117,6 +1161,7 @@ static int DNP3HandleRequestLinkLayer( uint16_t frame_len = DNP3CalculateLinkLength(header->len); if (frame_len == 0) { DNP3SetEvent(dnp3, true, DNP3_DECODER_EVENT_LEN_TOO_SMALL); + dnp3->link_error_ts = true; DNP3Resync(&input, &input_len, &processed); continue; } @@ -1140,10 +1185,27 @@ static int DNP3HandleRequestLinkLayer( if (!DNP3CheckUserDataCRCs(input + sizeof(DNP3LinkHeader), frame_len - sizeof(DNP3LinkHeader))) { DNP3SetEvent(dnp3, true, DNP3_DECODER_EVENT_BAD_TRANSPORT_CRC); + dnp3->link_error_ts = true; + if (dnp3_resync_on_bad_start) { + /* Like the endpoints: search for the next start sequence from + * the next octet, rather than trusting the LENGTH of a frame + * whose body failed its checksums. */ + DNP3Resync(&input, &input_len, &processed); + continue; + } goto next; } DNP3HandleUserDataRequest(f, dnp3, input, frame_len); + if (dnp3->link_error_ts) { + /* The first well-formed frame behind a link-layer error: a + * stop-on-error receiver never sees it, a resynchronizing one + * processes it. Which the device does depends on a setting the + * wire does not carry. Flagged once per error, so that one + * transient error does not flag the rest of the session. */ + DNP3SetEvent(dnp3, true, DNP3_DECODER_EVENT_AFTER_RESYNC); + dnp3->link_error_ts = false; + } next: /* Advance the input buffer. */ @@ -1238,12 +1300,19 @@ static int DNP3HandleResponseLinkLayer( DNP3LinkHeader *header = (DNP3LinkHeader *)input; if (!DNP3CheckStartBytes(header)) { - /* Terminal error condition. */ - SCReturnInt(-1); + if (!dnp3_resync_on_bad_start) { + /* Terminal error condition. */ + SCReturnInt(-1); + } + DNP3SetEvent(dnp3, false, DNP3_DECODER_EVENT_BAD_START_BYTES); + dnp3->link_error_tc = true; + DNP3Resync(&input, &input_len, &processed); + continue; } if (!DNP3CheckLinkHeaderCRC(header)) { DNP3SetEvent(dnp3, false, DNP3_DECODER_EVENT_BAD_LINK_CRC); + dnp3->link_error_tc = true; DNP3Resync(&input, &input_len, &processed); continue; } @@ -1252,6 +1321,7 @@ static int DNP3HandleResponseLinkLayer( uint16_t frame_len = DNP3CalculateLinkLength(header->len); if (frame_len == 0) { DNP3SetEvent(dnp3, false, DNP3_DECODER_EVENT_LEN_TOO_SMALL); + dnp3->link_error_tc = true; DNP3Resync(&input, &input_len, &processed); continue; } @@ -1275,10 +1345,27 @@ static int DNP3HandleResponseLinkLayer( if (!DNP3CheckUserDataCRCs(input + sizeof(DNP3LinkHeader), frame_len - sizeof(DNP3LinkHeader))) { DNP3SetEvent(dnp3, false, DNP3_DECODER_EVENT_BAD_TRANSPORT_CRC); + dnp3->link_error_tc = true; + if (dnp3_resync_on_bad_start) { + /* Like the endpoints: search for the next start sequence from + * the next octet, rather than trusting the LENGTH of a frame + * whose body failed its checksums. */ + DNP3Resync(&input, &input_len, &processed); + continue; + } goto next; } DNP3HandleUserDataResponse(f, dnp3, input, frame_len); + if (dnp3->link_error_tc) { + /* The first well-formed frame behind a link-layer error: a + * stop-on-error receiver never sees it, a resynchronizing one + * processes it. Which the device does depends on a setting the + * wire does not carry. Flagged once per error, so that one + * transient error does not flag the rest of the session. */ + DNP3SetEvent(dnp3, false, DNP3_DECODER_EVENT_AFTER_RESYNC); + dnp3->link_error_tc = false; + } next: /* Advance the input buffer. */ @@ -1610,16 +1697,25 @@ void RegisterDNP3Parsers(void) const char *proto_name = "dnp3"; + /* Read before the probing parser is registered: in resynchronizing mode + * it may need more than one link header to find a start sequence. */ + int bval = 0; + if (SCConfGetBool("app-layer.protocols.dnp3.resync-on-bad-start", &bval)) { + dnp3_resync_on_bad_start = bval; + } + const uint16_t probe_depth = + dnp3_resync_on_bad_start ? DNP3_RESYNC_PROBE_DEPTH : sizeof(DNP3LinkHeader); + if (SCAppLayerProtoDetectConfProtoDetectionEnabledDefault("tcp", proto_name, false)) { AppLayerProtoDetectRegisterProtocol(ALPROTO_DNP3, proto_name); if (RunmodeIsUnittests()) { SCAppLayerProtoDetectPPRegister(IPPROTO_TCP, DNP3_DEFAULT_PORT, ALPROTO_DNP3, 0, - sizeof(DNP3LinkHeader), STREAM_TOSERVER, DNP3ProbingParser, DNP3ProbingParser); + probe_depth, STREAM_TOSERVER, DNP3ProbingParser, DNP3ProbingParser); } else { if (!SCAppLayerProtoDetectPPParseConfPorts("tcp", IPPROTO_TCP, proto_name, ALPROTO_DNP3, - 0, sizeof(DNP3LinkHeader), DNP3ProbingParser, DNP3ProbingParser)) { + 0, probe_depth, DNP3ProbingParser, DNP3ProbingParser)) { return; } } diff --git a/src/app-layer-dnp3.h b/src/app-layer-dnp3.h index 7970169c27..94d3905bc1 100644 --- a/src/app-layer-dnp3.h +++ b/src/app-layer-dnp3.h @@ -113,6 +113,8 @@ enum { DNP3_DECODER_EVENT_TOO_MANY_POINTS, DNP3_DECODER_EVENT_TOO_MANY_OBJECTS, DNP3_DECODER_EVENT_TOO_LONG_REASS, + DNP3_DECODER_EVENT_BAD_START_BYTES, + DNP3_DECODER_EVENT_AFTER_RESYNC, }; /** @@ -247,6 +249,12 @@ typedef struct DNP3State_ { uint16_t events; uint32_t unreplied; /**< Number of unreplied requests. */ uint8_t flooded; /**< Flag indicating flood. */ + /** A link-layer error occurred to server (ts) or to client (tc) and no + * frame has been accepted since in that direction. A stop-on-error + * endpoint would have dropped the session at the error; the next frame + * accepted is one only a resynchronizing endpoint processes. */ + bool link_error_ts; + bool link_error_tc; DNP3Buffer request_buffer; /**< Request buffer for buffering * incomplete request PDUs received diff --git a/suricata.yaml.in b/suricata.yaml.in index 49c1df696c..2cf9660633 100644 --- a/suricata.yaml.in +++ b/suricata.yaml.in @@ -1267,6 +1267,13 @@ app-layer: enabled: no detection-ports: dp: 20000 + # Recover from link-layer errors like a resynchronizing endpoint + # (opendnp3, dnp3-rs Discard): skip octets that are not a start + # sequence (0x05 0x64) instead of ending DNP3 inspection of the flow, + # and after a bad transport CRC search for the next start sequence + # from the next octet instead of skipping the frame by its LENGTH + # (so frames embedded in a corrupted frame are parsed). Default: no. + #resync-on-bad-start: no #max-tx: 32 #max-points: 16384 #max-objects: 2048