From: Eric Leblond <eric@regit.org>
Date: Tue, 28 Jan 2014 15:54:51 +0000 (+0100)
Subject: tls: fix negated match
X-Git-Tag: suricata-2.0rc1~48
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c2fcf329f09c6e0d16cebb5906244c4ecc8ba30f;p=thirdparty%2Fsuricata.git

tls: fix negated match

A negated match is matching if the tested field is NULL. But as it
is not set, nor negated nor normal test must match.

Without this patch, a rule like:
 alert tls any any -> any any (msg:"negated match"; tls.subject:!"CN=home.regit.org"; sid:1; rev:1;)
is alerting for all connections. Event if they are done on a certificate
with matching subject. This was due to the fact that tls protocol
is discovered before the handshake is complete. Thus the condition
on tls is true with a NULL tls.subject. And code was returning a
positive match in the case of a NULL subject and a signature with
a negated match.
---

diff --git a/src/detect-tls.c b/src/detect-tls.c
index 6b7fad46b7..4110f0c02a 100644
--- a/src/detect-tls.c
+++ b/src/detect-tls.c
@@ -212,12 +212,6 @@ static int DetectTlsSubjectMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx,
     int ret = 0;
     FLOWLOCK_RDLOCK(f);
 
-    if (tls_data->flags & DETECT_CONTENT_NEGATED) {
-        ret = 1;
-    } else {
-        ret = 0;
-    }
-
     SSLStateConnp *connp = NULL;
     if (flags & STREAM_TOSERVER) {
         connp = &ssl_state->client_connp;
@@ -235,7 +229,15 @@ static int DetectTlsSubjectMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx,
             } else {
                 ret = 1;
             }
+        } else {
+            if (tls_data->flags & DETECT_CONTENT_NEGATED) {
+                ret = 1;
+            } else {
+                ret = 0;
+            }
         }
+    } else {
+        ret = 0;
     }
 
     FLOWLOCK_UNLOCK(f);
@@ -418,12 +420,6 @@ static int DetectTlsIssuerDNMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx
     int ret = 0;
     FLOWLOCK_RDLOCK(f);
 
-    if (tls_data->flags & DETECT_CONTENT_NEGATED) {
-        ret = 1;
-    } else {
-        ret = 0;
-    }
-
     SSLStateConnp *connp = NULL;
     if (flags & STREAM_TOSERVER) {
         connp = &ssl_state->client_connp;
@@ -441,7 +437,15 @@ static int DetectTlsIssuerDNMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx
             } else {
                 ret = 1;
             }
+        } else {
+            if (tls_data->flags & DETECT_CONTENT_NEGATED) {
+                ret = 1;
+            } else {
+                ret = 0;
+            }
         }
+    } else {
+        ret = 0;
     }
 
     FLOWLOCK_UNLOCK(f);
@@ -694,11 +698,6 @@ static int DetectTlsFingerprintMatch (ThreadVars *t, DetectEngineThreadCtx *det_
     int ret = 0;
     FLOWLOCK_RDLOCK(f);
 
-    if (tls_data->flags & DETECT_CONTENT_NEGATED) {
-        ret = 1;
-    } else {
-        ret = 0;
-    }
     if (ssl_state->server_connp.cert0_fingerprint != NULL) {
         SCLogDebug("TLS: Fingerprint is [%s], looking for [%s]\n",
                    ssl_state->server_connp.cert0_fingerprint,
@@ -713,7 +712,15 @@ static int DetectTlsFingerprintMatch (ThreadVars *t, DetectEngineThreadCtx *det_
                 ret = 1;
 
             }
+        } else {
+            if (tls_data->flags & DETECT_CONTENT_NEGATED) {
+                ret = 1;
+            } else {
+                ret = 0;
+            }
         }
+    } else {
+        ret = 0;
     }
 
     FLOWLOCK_UNLOCK(f);