From: Philippe Antoine <contact@catenacyber.fr>
Date: Mon, 7 Dec 2020 13:41:00 +0000 (+0100)
Subject: proto: introduce signature protocol, as extension to flow protocol
X-Git-Tag: suricata-6.0.3~20
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9daa22a021863508d497033939a353b61606de8a;p=thirdparty%2Fsuricata.git

proto: introduce signature protocol, as extension to flow protocol

AppProtoEquals function allows to check if a flow protocol
matches a signature protocol

This allows HTTP1 keywords on HTTP2 traffic
---

diff --git a/src/app-layer-detect-proto.c b/src/app-layer-detect-proto.c
index f2562d016a..1ac5495036 100644
--- a/src/app-layer-detect-proto.c
+++ b/src/app-layer-detect-proto.c
@@ -2081,12 +2081,10 @@ AppProto AppLayerProtoDetectGetProtoByName(const char *alproto_name)
     SCEnter();
 
     AppProto a;
+    AppProto b = StringToAppProto(alproto_name);
     for (a = 0; a < ALPROTO_MAX; a++) {
-        if (alpd_ctx.alproto_names[a] != NULL &&
-            strlen(alpd_ctx.alproto_names[a]) == strlen(alproto_name) &&
-            (SCMemcmp(alpd_ctx.alproto_names[a], alproto_name, strlen(alproto_name)) == 0))
-        {
-            SCReturnCT(a, "AppProto");
+        if (alpd_ctx.alproto_names[a] != NULL && AppProtoEquals(b, a)) {
+            SCReturnCT(b, "AppProto");
         }
     }
 
diff --git a/src/app-layer-protos.h b/src/app-layer-protos.h
index 278c3e4389..ff383c1514 100644
--- a/src/app-layer-protos.h
+++ b/src/app-layer-protos.h
@@ -77,6 +77,16 @@ static inline bool AppProtoIsValid(AppProto a)
     return ((a > ALPROTO_UNKNOWN && a < ALPROTO_FAILED));
 }
 
+// wether a signature AppProto matches a flow (or signature) AppProto
+static inline bool AppProtoEquals(AppProto sigproto, AppProto alproto)
+{
+    if (alproto == ALPROTO_HTTP2 && sigproto == ALPROTO_HTTP) {
+        // TODO config option
+        return true;
+    }
+    return (sigproto == alproto);
+}
+
 /**
  * \brief Maps the ALPROTO_*, to its string equivalent.
  *
diff --git a/src/detect-engine-prefilter-common.h b/src/detect-engine-prefilter-common.h
index 2ae1098a05..315bb270f6 100644
--- a/src/detect-engine-prefilter-common.h
+++ b/src/detect-engine-prefilter-common.h
@@ -79,7 +79,7 @@ PrefilterPacketHeaderExtraMatch(const PrefilterPacketHeaderCtx *ctx,
         case PREFILTER_EXTRA_MATCH_UNUSED:
             break;
         case PREFILTER_EXTRA_MATCH_ALPROTO:
-            if (p->flow == NULL || p->flow->alproto != ctx->value)
+            if (p->flow == NULL || !AppProtoEquals(ctx->value, p->flow->alproto))
                 return FALSE;
             break;
         case PREFILTER_EXTRA_MATCH_SRCPORT:
diff --git a/src/detect-engine.c b/src/detect-engine.c
index 3a41868fb5..cc06e73a59 100644
--- a/src/detect-engine.c
+++ b/src/detect-engine.c
@@ -551,7 +551,7 @@ int DetectEngineAppInspectionEngine2Signature(DetectEngineCtx *de_ctx, Signature
 
         if (t->alproto == ALPROTO_UNKNOWN) {
             /* special case, inspect engine applies to all protocols */
-        } else if (s->alproto != ALPROTO_UNKNOWN && s->alproto != t->alproto)
+        } else if (s->alproto != ALPROTO_UNKNOWN && !AppProtoEquals(s->alproto, t->alproto))
             goto next;
 
         if (s->flags & SIG_FLAG_TOSERVER && !(s->flags & SIG_FLAG_TOCLIENT)) {
diff --git a/src/detect-filestore.c b/src/detect-filestore.c
index 9b6c5a1a49..b35565cd45 100644
--- a/src/detect-filestore.c
+++ b/src/detect-filestore.c
@@ -517,4 +517,4 @@ void DetectFilestoreRegisterTests(void)
 {
     UtRegisterTest("DetectFilestoreTest01", DetectFilestoreTest01);
 }
-#endif /* UNITTESTS */
\ No newline at end of file
+#endif /* UNITTESTS */
diff --git a/src/detect-lua.c b/src/detect-lua.c
index 2bed7cf277..4960486b17 100644
--- a/src/detect-lua.c
+++ b/src/detect-lua.c
@@ -1017,7 +1017,7 @@ static int DetectLuaSetup (DetectEngineCtx *de_ctx, Signature *s, const char *st
         goto error;
 
     if (lua->alproto != ALPROTO_UNKNOWN) {
-        if (s->alproto != ALPROTO_UNKNOWN && lua->alproto != s->alproto) {
+        if (s->alproto != ALPROTO_UNKNOWN && !AppProtoEquals(s->alproto, lua->alproto)) {
             goto error;
         }
         s->alproto = lua->alproto;
diff --git a/src/detect-parse.c b/src/detect-parse.c
index 5464835a7d..08f651a72e 100644
--- a/src/detect-parse.c
+++ b/src/detect-parse.c
@@ -163,7 +163,7 @@ int DetectEngineContentModifierBufferSetup(DetectEngineCtx *de_ctx,
                    sigmatch_table[sm_type].name);
         goto end;
     }
-    if (s->alproto != ALPROTO_UNKNOWN && s->alproto != alproto) {
+    if (s->alproto != ALPROTO_UNKNOWN && !AppProtoEquals(s->alproto, alproto)) {
         SCLogError(SC_ERR_CONFLICTING_RULE_KEYWORDS, "rule contains conflicting "
                    "alprotos set");
         goto end;
@@ -1491,16 +1491,15 @@ int DetectSignatureSetAppProto(Signature *s, AppProto alproto)
         return -1;
     }
 
-    if (s->alproto != ALPROTO_UNKNOWN && s->alproto != alproto &&
-            // allow to keep HTTP2 with HTTP1 keywords
-            !(s->alproto == ALPROTO_HTTP2 && alproto == ALPROTO_HTTP)) {
+    if (s->alproto != ALPROTO_UNKNOWN && !AppProtoEquals(alproto, s->alproto)) {
         SCLogError(SC_ERR_CONFLICTING_RULE_KEYWORDS,
             "can't set rule app proto to %s: already set to %s",
             AppProtoToString(alproto), AppProtoToString(s->alproto));
         return -1;
     }
 
-    if (!(s->alproto == ALPROTO_HTTP2 && alproto == ALPROTO_HTTP)) {
+    // allow to keep HTTP2 as s->alproto with HTTP1 alproto keywords
+    if (!AppProtoEquals(alproto, s->alproto)) {
         s->alproto = alproto;
     }
     s->flags |= SIG_FLAG_APPLAYER;
@@ -1691,7 +1690,8 @@ static int SigValidate(DetectEngineCtx *de_ctx, Signature *s)
         if (s->init_data->smlists[x]) {
             const DetectEngineAppInspectionEngine *app = de_ctx->app_inspect_engines;
             for ( ; app != NULL; app = app->next) {
-                if (app->sm_list == x && ((s->alproto == app->alproto) || s->alproto == 0)) {
+                if (app->sm_list == x &&
+                        (AppProtoEquals(s->alproto, app->alproto) || s->alproto == 0)) {
                     SCLogDebug("engine %s dir %d alproto %d",
                             DetectBufferTypeGetNameById(de_ctx, app->sm_list),
                             app->dir, app->alproto);
diff --git a/src/detect-pcre.c b/src/detect-pcre.c
index 9bff6d044d..7b73a5251d 100644
--- a/src/detect-pcre.c
+++ b/src/detect-pcre.c
@@ -904,8 +904,7 @@ static int DetectPcreSetup (DetectEngineCtx *de_ctx, Signature *s, const char *r
                 if (alproto != ALPROTO_UNKNOWN) {
                     /* see if the proto doesn't conflict
                      * with what we already have. */
-                    if (s->alproto != ALPROTO_UNKNOWN &&
-                            alproto != s->alproto) {
+                    if (s->alproto != ALPROTO_UNKNOWN && !AppProtoEquals(s->alproto, alproto)) {
                         goto error;
                     }
                     if (DetectSignatureSetAppProto(s, alproto) < 0)
@@ -3656,4 +3655,4 @@ static void DetectPcreRegisterTests(void)
     UtRegisterTest("DetectPcreParseCaptureTest", DetectPcreParseCaptureTest);
 
 }
-#endif /* UNITTESTS */
\ No newline at end of file
+#endif /* UNITTESTS */
diff --git a/src/detect.c b/src/detect.c
index bd06176325..d842f586a5 100644
--- a/src/detect.c
+++ b/src/detect.c
@@ -361,7 +361,8 @@ DetectPrefilterBuildNonPrefilterList(DetectEngineThreadCtx *det_ctx,
          * so build the non_mpm array only for match candidates */
         const SignatureMask rule_mask = det_ctx->non_pf_store_ptr[x].mask;
         const uint8_t rule_alproto = det_ctx->non_pf_store_ptr[x].alproto;
-        if ((rule_mask & mask) == rule_mask && (rule_alproto == 0 || rule_alproto == alproto)) {
+        if ((rule_mask & mask) == rule_mask &&
+                (rule_alproto == 0 || AppProtoEquals(rule_alproto, alproto))) {
             det_ctx->non_pf_id_array[det_ctx->non_pf_id_cnt++] = det_ctx->non_pf_store_ptr[x].id;
         }
     }
@@ -777,7 +778,7 @@ static inline void DetectRulePacketRules(
 
         /* if the sig has alproto and the session as well they should match */
         if (likely(sflags & SIG_FLAG_APPLAYER)) {
-            if (s->alproto != ALPROTO_UNKNOWN && s->alproto != scratch->alproto) {
+            if (s->alproto != ALPROTO_UNKNOWN && !AppProtoEquals(s->alproto, scratch->alproto)) {
                 if (s->alproto == ALPROTO_DCERPC) {
                     if (scratch->alproto != ALPROTO_SMB) {
                         SCLogDebug("DCERPC sig, alproto not SMB");
@@ -1090,7 +1091,7 @@ static bool DetectRunTxInspectRule(ThreadVars *tv,
             return false;
         }
         /* stream mpm and negated mpm sigs can end up here with wrong proto */
-        if (!(f->alproto == s->alproto || s->alproto == ALPROTO_UNKNOWN)) {
+        if (!(AppProtoEquals(s->alproto, f->alproto) || s->alproto == ALPROTO_UNKNOWN)) {
             TRACE_SID_TXS(s->id, tx, "alproto mismatch");
             return false;
         }