[Oisf-devel] [MOVED to Oisf-devel] Re: [Oisf-users] Config options discrepancies between suricata.yaml and ConfNodeLookupChildValue() in the source.

[Nikolay Denev]

On Feb 16, 2012, at 6:46 PM, Victor Julien wrote:

> On 02/16/2012 05:44 PM, Nikolay Denev wrote:
>> 
>> On Feb 16, 2012, at 6:30 PM, Victor Julien wrote:
>> 
>>> On 02/16/2012 03:16 PM, Nikolay Denev wrote:
>>>> Hi,
>>>> 
>>>> I've noticed that there are some discrepancies regarding the config options that are in the default suricata.yaml file, that
>>>> I guess most people use as a starting point to modify for their needs. For example I've tried to set the pcap logging to use only 10 files and rotate them,
>>>> and I noticed that this didn't work. I've found that the source uses "ConfNodeLookupChildValue(conf, "max-files");" to get the number of files, but suricata.yaml
>>>> has "max_files", so this option is not parsed.
>>>> I see other similar mixups of dashes and underscores like : 
>>>> 
>>>> /usr/local/etc/suricata/suricata.yaml:      use_stream_depth: no #If set to "yes" packets seen after reaching stream inspection depth are ignored. "no" logs all packets
>>>> ./log-pcap.c:        use_stream_depth = ConfNodeLookupChildValue(conf, "use-stream-depth");
>>>> 
>>>> ./log-pcap.c:            s_dir = ConfNodeLookupChildValue(conf, "sguil-base-dir");
>>>> ./log-pcap.c:                s_dir = ConfNodeLookupChildValue(conf, "sguil_base_dir");
>>>> 
>>>> 
>>>> ts-format and ts_format…
>>>> 
>>>> I can try to prepare a patch for these when I have free time, if it's clear what should be the convention: underscore or dash.
>>> 
>>> Yeah it's inconsistent. The goal is to have the dash approach
>>> everywhere. Complicating things is that for every setting we convert, I
>>> want the old way to continue to work. So backwards compatibility, like
>>> with the sguil-base-dir.
>>> 
>>> Still interested in helping out? :)
>>> 
>>> -- 
>>> ---------------------------------------------
>>> Victor Julien
>>> http://www.inliniac.net/
>>> PGP: http://www.inliniac.net/victorjulien.asc
>>> ---------------------------------------------
>> 
>> Hmm :)
>> Does converting all underscores to dashes in the ConfNodeLookup* functions (emitting warning about the uncerscore versions being deprecated) and using only dashes internally makes sense?
>> This way even "sguil_base-dir" would work as a side effect, but I doubt it would break anything.
>> 
> 
> Thats actually a pretty good idea, as long as we only do it for the
> option name, not the value.
> 
> -- 
> ---------------------------------------------
> Victor Julien
> http://www.inliniac.net/
> PGP: http://www.inliniac.net/victorjulien.asc
> ---------------------------------------------
> 

I've hacked this ugly piece :)

--- suricata.orig/work/suricata-1.2.1/src/conf-yaml-loader.c	2012-01-20 17:17:30.000000000 +0200
+++ suricata/work/suricata-1.2.1/src/conf-yaml-loader.c	2012-02-16 21:21:21.000000000 +0200
@@ -104,7 +104,7 @@
                 if (state == CONF_KEY) {
                     if (parent->is_seq) {
                         if (parent->val == NULL) {
-                            parent->val = SCStrdup(value);
+                            parent->val = SCStrdup_mangle(value);
                         }
                     }
                     ConfNode *n0 = ConfNodeLookupChild(parent, value);
@@ -113,7 +113,7 @@
                     }
                     else {
                         node = ConfNodeNew();
-                        node->name = SCStrdup(value);
+                        node->name = SCStrdup_mangle(value);
                         TAILQ_INSERT_TAIL(&parent->head, node, next);
                     }
                     state = CONF_VAL;
diff -ur suricata.orig/work/suricata-1.2.1/src/util-mem.h suricata/work/suricata-1.2.1/src/util-mem.h
--- suricata.orig/work/suricata-1.2.1/src/util-mem.h	2012-01-20 17:17:30.000000000 +0200
+++ suricata/work/suricata-1.2.1/src/util-mem.h	2012-02-16 21:03:23.000000000 +0200
@@ -118,7 +118,11 @@
     (void*)ptrmem; \
 })
 
-#define SCStrdup(a) ({ \
+#define SCStrdup(a) _SCStrdup(a,0)
+
+#define SCStrdup_mangle(a) _SCStrdup(a,1)
+
+#define _SCStrdup(a,mangle) ({ \
     char *ptrmem = NULL; \
     extern size_t global_mem; \
     extern uint8_t print_mem_flag; \
@@ -134,6 +138,12 @@
         } \
     } \
     \
+    if ((mangle != 0)) { \
+        char *u; \
+        while (u = strchr(ptrmem, '_')) { \
+            *u = '-'; \
+        } \
+    } \
     global_mem += len; \
     if (print_mem_flag == 1) \
         SCLogInfo("SCStrdup return at %p of size %"PRIuMAX, \
@@ -196,7 +206,11 @@
     (void*)ptrmem; \
 })
 
-#define SCStrdup(a) ({ \
+#define SCStrdup(a) _SCStrdup(a,0)
+
+#define SCStrdup_mangle(a) _SCStrdup(a,1)
+
+#define _SCStrdup(a,mangle) ({ \
     char *ptrmem = NULL; \
     \
     ptrmem = strdup((a)); \
@@ -209,6 +223,12 @@
             exit(EXIT_FAILURE); \
         } \
     } \
+    if ((mangle != 0)) { \
+        char *u; \
+        while (u = strchr(ptrmem, '_')) { \
+            *u = '-'; \
+        } \
+    } \
     (void*)ptrmem; \
 })
 


Seems to work, except that I have to specially handle the address-group and port-group variables, as this currently
makes HOME_NET -> HOME-NET and the rules fail.