From 6bf00ab2893f415d8cbca645cea8aef0ed5a2f66 Mon Sep 17 00:00:00 2001
From: Eric Leblond <eric@regit.org>
Date: Fri, 21 Apr 2017 19:42:04 +0200
Subject: [PATCH] output-json-alert: conditionaly output metadata

Metadata of the signature can now conditionaly put in the alert
events. This will allow user to get more context about the events
generated by the alert.

detect-metadata: conditional parsing

Only parses metadata if an output module will use the information.
Patch also adds a unittest to check metadata is not parsed if not
asked to.

output-json-alert: optional output keys as array

Update rule metadata configuration to have an option to output
value as array. Also adds an option to log only a series of keys
as array. This is useful in the case of some ruleset where from
instance the `tag` key is used multiple time.

(Jason Ish) rule metadata: always log as lists

After review of rule metadata, we can't make assumptions
on what should be a list or not. So log everything as a list.
---
 src/detect-engine.c     | 17 ++++++++++++
 src/detect-engine.h     |  4 +++
 src/detect-metadata.c   | 40 +++++++++++++++++++++++++++--
 src/output-json-alert.c | 57 ++++++++++++++++++++++++++++++++++++++---
 src/output-json-alert.h |  3 ++-
 src/output-json-drop.c  |  4 +--
 suricata.yaml.in        |  2 ++
 7 files changed, 119 insertions(+), 8 deletions(-)

diff --git a/src/detect-engine.c b/src/detect-engine.c
index 2b0bddee11..16ac8d832e 100644
--- a/src/detect-engine.c
+++ b/src/detect-engine.c
@@ -3055,6 +3055,23 @@ int DetectEngineMTApply(void)
     return 0;
 }
 
+static int g_parse_metadata = 0;
+
+void DetectEngineSetParseMetadata(void)
+{
+    g_parse_metadata = 1;
+}
+
+void DetectEngineUnsetParseMetadata(void)
+{
+    g_parse_metadata = 0;
+}
+
+int DetectEngineMustParseMetadata(void)
+{
+    return g_parse_metadata;
+}
+
 const char *DetectSigmatchListEnumToString(enum DetectSigmatchListEnum type)
 {
     switch (type) {
diff --git a/src/detect-engine.h b/src/detect-engine.h
index 9b17d07230..dd6ed63dac 100644
--- a/src/detect-engine.h
+++ b/src/detect-engine.h
@@ -117,4 +117,8 @@ void DetectAppLayerInspectEngineRegister(const char *name,
 int DetectEngineAppInspectionEngine2Signature(Signature *s);
 void DetectEngineAppInspectionEngineSignatureFree(Signature *s);
 
+void DetectEngineSetParseMetadata(void);
+void DetectEngineUnsetParseMetadata(void);
+int DetectEngineMustParseMetadata(void);
+
 #endif /* __DETECT_ENGINE_H__ */
diff --git a/src/detect-metadata.c b/src/detect-metadata.c
index 5ee6dba0b2..aa273dcc9c 100644
--- a/src/detect-metadata.c
+++ b/src/detect-metadata.c
@@ -172,7 +172,9 @@ error:
 
 static int DetectMetadataSetup(DetectEngineCtx *de_ctx, Signature *s, const char *rawstr)
 {
-    DetectMetadataParse(s, rawstr);
+    if (DetectEngineMustParseMetadata()) {
+        DetectMetadataParse(s, rawstr);
+    }
 
     return 0;
 }
@@ -182,14 +184,47 @@ static int DetectMetadataSetup(DetectEngineCtx *de_ctx, Signature *s, const char
 static int DetectMetadataParseTest01(void)
 {
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
-    DetectMetadata *dm;
     FAIL_IF_NULL(de_ctx);
+    int prev_state = 0;
+
+    if (DetectEngineMustParseMetadata()) {
+        prev_state = 1;
+        DetectEngineUnsetParseMetadata();
+    }
+    Signature *sig = DetectEngineAppendSig(de_ctx,
+                                           "alert tcp any any -> any any "
+                                           "(metadata: toto 1; sid:1; rev:1;)");
+    if (prev_state == 1) {
+        DetectEngineSetParseMetadata();
+    }
+    FAIL_IF_NULL(sig);
+    FAIL_IF(sig->metadata); 
+
+    DetectEngineCtxFree(de_ctx);
+    PASS;
+}
+
+static int DetectMetadataParseTest02(void)
+{
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    DetectMetadata *dm;
+    int prev_state = 1;
+
+    if (! DetectEngineMustParseMetadata()) {
+        prev_state = 0;
+        DetectEngineSetParseMetadata();
+    }
 
+    de_ctx = DetectEngineCtxInit();
+    FAIL_IF_NULL(de_ctx);
     Signature *sig = DetectEngineAppendSig(de_ctx,
                                            "alert tcp any any -> any any "
                                            "(metadata: toto 1; "
                                            "metadata: titi 2, jaivu gros_minet;"
                                            "sid:1; rev:1;)");
+    if (prev_state == 0) {
+        DetectEngineUnsetParseMetadata();
+    }
     FAIL_IF_NULL(sig);
     FAIL_IF_NULL(sig->metadata); 
     FAIL_IF_NULL(sig->metadata->key); 
@@ -215,6 +250,7 @@ static void DetectMetadataRegisterTests(void)
 {
 #ifdef UNITTESTS
     UtRegisterTest("DetectMetadataParseTest01", DetectMetadataParseTest01);
+    UtRegisterTest("DetectMetadataParseTest02", DetectMetadataParseTest02);
 #endif /* UNITTESTS */
 }
 
diff --git a/src/output-json-alert.c b/src/output-json-alert.c
index 6b095c5fd5..030091c752 100644
--- a/src/output-json-alert.c
+++ b/src/output-json-alert.c
@@ -43,6 +43,7 @@
 #include "detect-engine.h"
 #include "detect-engine-mpm.h"
 #include "detect-reference.h"
+#include "detect-metadata.h"
 #include "app-layer-parser.h"
 #include "app-layer-dnp3.h"
 #include "app-layer-htp.h"
@@ -85,6 +86,7 @@
 #define LOG_JSON_FLOW              BIT_U16(5)
 #define LOG_JSON_HTTP_BODY         BIT_U16(6)
 #define LOG_JSON_HTTP_BODY_BASE64  BIT_U16(7)
+#define LOG_JSON_RULE_METADATA     BIT_U16(8)
 
 #define LOG_JSON_METADATA (LOG_JSON_APP_LAYER | LOG_JSON_FLOW)
 
@@ -226,9 +228,42 @@ static void AlertJsonSourceTarget(const Packet *p, const PacketAlert *pa,
     json_object_set_new(ajs, "target", tjs);
 }
 
+static void AlertJsonMetadata(AlertJsonOutputCtx *json_output_ctx, const PacketAlert *pa, json_t *ajs)
+{
+    if (pa->s->metadata) {
+        const DetectMetadata* kv = pa->s->metadata;
+        json_t *mjs = json_object();
+        if (unlikely(mjs == NULL)) {
+            return;
+        }
+        while (kv) {
+            json_t *jkey = json_object_get(mjs, kv->key);
+            if (jkey == NULL) {
+                jkey = json_array();
+                if (unlikely(jkey == NULL))
+                    break;
+                json_array_append_new(jkey, json_string(kv->value));
+                json_object_set_new(mjs, kv->key, jkey);
+            } else {
+                json_array_append_new(jkey, json_string(kv->value));
+            }
 
-void AlertJsonHeader(const Packet *p, const PacketAlert *pa, json_t *js)
+            kv = kv->next;
+        }
+
+        if (json_object_size(mjs) == 0) {
+            json_decref(mjs);
+        } else {
+            json_object_set_new(ajs, "metadata", mjs);
+        }
+    }
+}
+
+
+void AlertJsonHeader(void *ctx, const Packet *p, const PacketAlert *pa, json_t *js,
+                     uint16_t flags)
 {
+    AlertJsonOutputCtx *json_output_ctx = (AlertJsonOutputCtx *)ctx;
     const char *action = "allowed";
     /* use packet action if rate_filter modified the action */
     if (unlikely(pa->flags & PACKET_ALERT_RATE_FILTER_MODIFIED)) {
@@ -271,6 +306,10 @@ void AlertJsonHeader(const Packet *p, const PacketAlert *pa, json_t *js)
         AlertJsonSourceTarget(p, pa, js, ajs);
     }
 
+    if ((json_output_ctx != NULL) && (flags & LOG_JSON_RULE_METADATA)) {
+        AlertJsonMetadata(json_output_ctx, pa, ajs);
+    }
+
     /* alert */
     json_object_set_new(js, "alert", ajs);
 }
@@ -364,7 +403,7 @@ static int AlertJson(ThreadVars *tv, JsonAlertLogThread *aft, const Packet *p)
         MemBufferReset(aft->json_buffer);
 
         /* alert */
-        AlertJsonHeader(p, pa, js);
+        AlertJsonHeader(json_output_ctx, p, pa, js, json_output_ctx->flags);
 
         if (IS_TUNNEL_PKT(p)) {
             AlertJsonTunnel(p, js);
@@ -721,7 +760,6 @@ static void JsonAlertLogDeInitCtxSub(OutputCtx *output_ctx)
         if (xff_cfg != NULL) {
             SCFree(xff_cfg);
         }
-
         SCFree(json_output_ctx);
     }
     SCFree(output_ctx);
@@ -777,6 +815,19 @@ static void XffSetup(AlertJsonOutputCtx *json_output_ctx, ConfNode *conf)
         SetFlag(conf, "http-body-printable", LOG_JSON_HTTP_BODY, &flags);
         SetFlag(conf, "http-body", LOG_JSON_HTTP_BODY_BASE64, &flags);
 
+        ConfNode *rmetadata = ConfNodeLookupChild(conf, "rule-metadata");
+        if (rmetadata != NULL) {
+            int enabled = 0, ret;
+            ret = ConfGetChildValueBool(rmetadata, "enabled", &enabled);
+            if (ret && enabled) {
+                json_output_ctx->flags |= LOG_JSON_RULE_METADATA;
+            }
+        }
+
+        if (json_output_ctx->flags & LOG_JSON_RULE_METADATA) {
+            DetectEngineSetParseMetadata();
+        }
+
         const char *payload_buffer_value = ConfNodeLookupChildValue(conf, "payload-buffer-size");
 
         if (payload_buffer_value != NULL) {
diff --git a/src/output-json-alert.h b/src/output-json-alert.h
index 645b6f5c02..379ec8bb44 100644
--- a/src/output-json-alert.h
+++ b/src/output-json-alert.h
@@ -29,7 +29,8 @@
 
 void JsonAlertLogRegister(void);
 #ifdef HAVE_LIBJANSSON
-void AlertJsonHeader(const Packet *p, const PacketAlert *pa, json_t *js);
+void AlertJsonHeader(void *ctx, const Packet *p, const PacketAlert *pa, json_t *js,
+                     uint16_t flags);
 #endif /* HAVE_LIBJANSSON */
 
 #endif /* __OUTPUT_JSON_ALERT_H__ */
diff --git a/src/output-json-drop.c b/src/output-json-drop.c
index 3a91f39bd4..209d7aa013 100644
--- a/src/output-json-drop.c
+++ b/src/output-json-drop.c
@@ -163,14 +163,14 @@ static int DropLogJSON (JsonDropLogThread *aft, const Packet *p)
             if ((pa->action & (ACTION_REJECT|ACTION_REJECT_DST|ACTION_REJECT_BOTH)) ||
                ((pa->action & ACTION_DROP) && EngineModeIsIPS()))
             {
-                AlertJsonHeader(p, pa, js);
+                AlertJsonHeader(NULL, p, pa, js, 0);
                 logged = 1;
             }
         }
         if (logged == 0) {
             if (p->alerts.drop.action != 0) {
                 const PacketAlert *pa = &p->alerts.drop;
-                AlertJsonHeader(p, pa, js);
+                AlertJsonHeader(NULL, p, pa, js, 0);
             }
         }
     }
diff --git a/suricata.yaml.in b/suricata.yaml.in
index 0a8936b005..2b52fa3592 100644
--- a/suricata.yaml.in
+++ b/suricata.yaml.in
@@ -175,6 +175,8 @@ outputs:
             # packet: yes              # enable dumping of packet (without stream segments)
             # http-body: yes           # enable dumping of http body in Base64
             # http-body-printable: yes # enable dumping of http body in printable format
+            rule-metadata:             # dumping of key/value pairs defined by metadata keyword of rule
+              enabled: no              # set to yes to enable
 
             # Enable the logging of tagged packets for rules using the
             # "tag" keyword.