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.

diff --git a/src/app-layer-ssl.c b/src/app-layer-ssl.c
index 45dd9c2776bdbd3e0d3445c5f47d5be7c7ba16bc..9a1a30c3b18783456f5ed621c5ae22c68cfa083e 100644 (file)
--- a/src/app-layer-ssl.c
+++ b/src/app-layer-ssl.c
@@ -2218,6 +2218,7 @@ static struct SSLDecoderResult SSLv3Decode(uint8_t direction, SSLState *ssl_stat
 {
     uint32_t parsed = 0;
     uint32_t record_len; /* slice of input_len for the current record */
+    const bool first_call = (ssl_state->curr_connp->bytes_processed == 0);
 
     if (ssl_state->curr_connp->bytes_processed < SSLV3_RECORD_HDR_LEN) {
         const uint16_t prev_version = ssl_state->curr_connp->version;
@@ -2229,6 +2230,33 @@ 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);
+
+        /* first the hdr frame at our first chance */
+        if (first_call) {
+            AppLayerFrameNewByPointer(ssl_state->f, &stream_slice, input, SSLV3_RECORD_HDR_LEN,
+                    direction, TLS_FRAME_HDR);
+        }
+
+        /* 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);
+        }
+
+        /* pdu frame needs record length, so only create it when hdr fully parsed. */
+        AppLayerFrameNewByPointer(ssl_state->f, &stream_slice, input,
+                ssl_state->curr_connp->record_length + retval, direction, TLS_FRAME_PDU);
+        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) {
@@ -2243,17 +2271,6 @@ static struct SSLDecoderResult SSLv3Decode(uint8_t direction, SSLState *ssl_stat
                 break;
         }
 
-        SCLogDebug("%s input %p record_length %u", (direction == 0) ? "toserver" : "toclient",
-                input, ssl_state->curr_connp->record_length);
-        AppLayerFrameNewByPointer(ssl_state->f, &stream_slice, input,
-                ssl_state->curr_connp->record_length + retval, direction, TLS_FRAME_PDU);
-        AppLayerFrameNewByPointer(
-                ssl_state->f, &stream_slice, input, SSLV3_RECORD_HDR_LEN, direction, TLS_FRAME_HDR);
-        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) {
@@ -2294,6 +2311,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);