]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
tls: handle incomplete header sooner
authorVictor Julien <vjulien@oisf.net>
Wed, 21 Sep 2022 17:56:45 +0000 (19:56 +0200)
committerVictor Julien <vjulien@oisf.net>
Fri, 13 Jan 2023 11:33:04 +0000 (12:33 +0100)
Make sure to exit the parser early on incomplete header data.

Additionally, make sure to not create duplicated tls frames in this
case.

Add a debug validation check for the header parser parsing too much
data, which should never happen.

(cherry picked from commit 1701a6b14c0fef6a374368a31c8a0d753b574b9c)

src/app-layer-ssl.c

index 4cec0e4f45bffe2128553235eac91b627ef31b34..435411e4a326de6f81b6dfcfad0a2bcd2f090636 100644 (file)
@@ -2202,6 +2202,24 @@ static struct SSLDecoderResult SSLv3Decode(uint8_t direction, SSLState *ssl_stat
             SSLSetEvent(ssl_state, TLS_DECODER_EVENT_INVALID_TLS_HEADER);
             return SSL_DECODER_ERROR(-1);
         }
+        parsed = retval;
+
+        SCLogDebug("%s input %p record_length %u", (direction == 0) ? "toserver" : "toclient",
+                input, ssl_state->curr_connp->record_length);
+
+        /* parser is streaming for the initial header, then switches to incomplete
+         * API: so if we don't have the hdr yet, return consumed bytes and wait
+         * until we are called again with new data. */
+        if (ssl_state->curr_connp->bytes_processed < SSLV3_RECORD_HDR_LEN) {
+            SCLogDebug(
+                    "incomplete header, return %u bytes consumed and wait for more data", parsed);
+            return SSL_DECODER_OK(parsed);
+        }
+
+        record_len = MIN(input_len - parsed, ssl_state->curr_connp->record_length);
+        SCLogDebug(
+                "record_len %u (input_len %u, parsed %u, ssl_state->curr_connp->record_length %u)",
+                record_len, input_len, parsed, ssl_state->curr_connp->record_length);
 
         bool unknown_record = false;
         switch (ssl_state->curr_connp->content_type) {
@@ -2216,11 +2234,6 @@ static struct SSLDecoderResult SSLv3Decode(uint8_t direction, SSLState *ssl_stat
                 break;
         }
 
-        parsed = retval;
-        record_len = MIN(input_len - parsed, ssl_state->curr_connp->record_length);
-        SCLogDebug("record_len %u (input_len %u, parsed %u, ssl_state->curr_connp->record_length %u)",
-                record_len, input_len, parsed, ssl_state->curr_connp->record_length);
-
         /* unknown record type. For TLS 1.0, 1.1 and 1.2 this is ok. For the rest it is fatal. Based
          * on Wireshark logic. */
         if (prev_version == TLS_VERSION_10 || prev_version == TLS_VERSION_11) {
@@ -2261,6 +2274,7 @@ static struct SSLDecoderResult SSLv3Decode(uint8_t direction, SSLState *ssl_stat
             SSLSetEvent(ssl_state, TLS_DECODER_EVENT_INVALID_RECORD_LENGTH);
             return SSL_DECODER_ERROR(-1);
         }
+        DEBUG_VALIDATE_BUG_ON(ssl_state->curr_connp->bytes_processed > SSLV3_RECORD_HDR_LEN);
     } else {
         ValidateRecordState(ssl_state->curr_connp);