From: Mats Klepsland <mats.klepsland@gmail.com>
Date: Tue, 21 Aug 2018 06:21:21 +0000 (+0200)
Subject: app-layer-ssl: add better session id support
X-Git-Tag: suricata-4.1.0-rc2~98
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=21897a4d7aaed971477e32dc38ad5e3eb7f162e8;p=thirdparty%2Fsuricata.git

app-layer-ssl: add better session id support

Verify that the session id from both the client hello record and the
server hello record matches before marking the session as 'resumed'.
---

diff --git a/src/app-layer-ssl.c b/src/app-layer-ssl.c
index 6877993c39..16ec1a41d1 100644
--- a/src/app-layer-ssl.c
+++ b/src/app-layer-ssl.c
@@ -608,13 +608,31 @@ static inline int TLSDecodeHSHelloSessionID(SSLState *ssl_state,
     uint8_t session_id_length = *input;
     input += 1;
 
-    if (session_id_length != 0) {
-        ssl_state->flags |= SSL_AL_FLAG_SSL_CLIENT_SESSION_ID;
-    }
-
     if (!(HAS_SPACE(session_id_length)))
         goto invalid_length;
 
+    if (session_id_length != 0 && ssl_state->curr_connp->session_id == NULL) {
+        ssl_state->curr_connp->session_id = SCMalloc(session_id_length);
+
+        if (unlikely(ssl_state->curr_connp->session_id == NULL)) {
+            return -1;
+        }
+
+        memcpy(ssl_state->curr_connp->session_id, input, session_id_length);
+        ssl_state->curr_connp->session_id_length = session_id_length;
+
+        if ((ssl_state->current_flags & SSL_AL_FLAG_STATE_SERVER_HELLO) &&
+                ssl_state->client_connp.session_id != NULL &&
+                ssl_state->server_connp.session_id != NULL) {
+            if ((ssl_state->client_connp.session_id_length ==
+                    ssl_state->server_connp.session_id_length) &&
+                    (memcmp(ssl_state->server_connp.session_id,
+                    ssl_state->client_connp.session_id, session_id_length) == 0)) {
+                ssl_state->flags |= SSL_AL_FLAG_SESSION_RESUMED;
+            }
+        }
+    }
+
     input += session_id_length;
 
     return (input - initial_input);
@@ -1956,13 +1974,6 @@ static int SSLv3Decode(uint8_t direction, SSLState *ssl_state,
 
             if (direction) {
                 ssl_state->flags |= SSL_AL_FLAG_SERVER_CHANGE_CIPHER_SPEC;
-
-                int server_cert_seen = (ssl_state->server_connp.cert0_issuerdn != NULL &&
-                                        ssl_state->server_connp.cert0_subject != NULL);
-                if (!server_cert_seen && (ssl_state->flags & SSL_AL_FLAG_SSL_CLIENT_SESSION_ID) != 0) {
-                    ssl_state->flags |= SSL_AL_FLAG_SESSION_RESUMED;
-                }
-
             } else {
                 ssl_state->flags |= SSL_AL_FLAG_CLIENT_CHANGE_CIPHER_SPEC;
             }
@@ -2313,6 +2324,8 @@ static void SSLStateFree(void *p)
         SCFree(ssl_state->client_connp.cert0_fingerprint);
     if (ssl_state->client_connp.sni)
         SCFree(ssl_state->client_connp.sni);
+    if (ssl_state->client_connp.session_id)
+        SCFree(ssl_state->client_connp.session_id);
 
     if (ssl_state->server_connp.trec)
         SCFree(ssl_state->server_connp.trec);
@@ -2324,6 +2337,8 @@ static void SSLStateFree(void *p)
         SCFree(ssl_state->server_connp.cert0_fingerprint);
     if (ssl_state->server_connp.sni)
         SCFree(ssl_state->server_connp.sni);
+    if (ssl_state->server_connp.session_id)
+        SCFree(ssl_state->server_connp.session_id);
 
     if (ssl_state->ja3_str)
         Ja3BufferFree(&ssl_state->ja3_str);
@@ -5056,7 +5071,7 @@ static int SSLParserTest26(void)
     FAIL_IF_NULL(ssl_state);
 
     FAIL_IF((ssl_state->flags & SSL_AL_FLAG_STATE_CLIENT_HELLO) == 0);
-    FAIL_IF((ssl_state->flags & SSL_AL_FLAG_SSL_CLIENT_SESSION_ID) == 0);
+    FAIL_IF_NULL(ssl_state->client_connp.session_id);
 
     FLOWLOCK_WRLOCK(&f);
     r = AppLayerParserParse(NULL, alp_tctx, &f, ALPROTO_TLS, STREAM_TOCLIENT,
diff --git a/src/app-layer-ssl.h b/src/app-layer-ssl.h
index 882a8f2f23..398b1a84a8 100644
--- a/src/app-layer-ssl.h
+++ b/src/app-layer-ssl.h
@@ -153,7 +153,6 @@ typedef struct SSLStateConnp_ {
     /* the no of bytes processed in the currently parsed handshake */
     uint16_t hs_bytes_processed;
 
-    /* sslv2 client hello session id length */
     uint16_t session_id_length;
 
     char *cert0_subject;
@@ -166,6 +165,8 @@ typedef struct SSLStateConnp_ {
     /* ssl server name indication extension */
     char *sni;
 
+    char *session_id;
+
     TAILQ_HEAD(, SSLCertsChain_) certs;
 
     uint32_t cert_log_flag;