From: Victor Julien <vjulien@oisf.net>
Date: Wed, 21 Sep 2022 17:56:45 +0000 (+0200)
Subject: tls: handle incomplete header sooner
X-Git-Tag: suricata-6.0.10~34
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f0d9cc5a5e9d803598d4095e7c0cf52dfb2acc6d;p=thirdparty%2Fsuricata.git

tls: handle incomplete header sooner

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)
---

diff --git a/src/app-layer-ssl.c b/src/app-layer-ssl.c
index 4cec0e4f45..435411e4a3 100644
--- a/src/app-layer-ssl.c
+++ b/src/app-layer-ssl.c
@@ -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);