detect/config: clean up keyword value parsing

diff --git a/src/detect-config.c b/src/detect-config.c
index 64be0ae456b511e3f95d4c3594848447545d0a1a..38f2472836df554aae1a77281c7e3f76ed20ed5d 100644 (file)
--- a/src/detect-config.c
+++ b/src/detect-config.c
@@ -149,44 +149,23 @@ static int DetectConfigPostMatch(DetectEngineThreadCtx *det_ctx,
     SCReturnInt(1);
 }
 
-/**
- * \brief this function is used to parse filestore options
- * \brief into the current signature
- *
- * \param de_ctx pointer to the Detection Engine Context
- * \param s pointer to the Current Signature
- * \param str pointer to the user provided "filestore" option
- *
- * \retval 0 on Success
- * \retval -1 on Failure
- */
-static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char *str)
-{
-    SCEnter();
-
-    DetectConfigData *fd = NULL;
-    int res = 0;
-    size_t pcre2len;
-#if 0
-    /* filestore and bypass keywords can't work together */
-    if (s->flags & SIG_FLAG_BYPASS) {
-        SCLogError(
-                   "filestore can't work with bypass keyword");
-        return -1;
-    }
-#endif
-    pcre2_match_data *match = NULL;
-
-    if (str == NULL || strlen(str) == 0) {
-        SCLogError("config keywords need arguments");
-        goto error;
-    }
+struct ConfigStrings {
     char subsys[32];
     char state[32];
     char type[32];
     char typeval[32];
     char scope[32];
     char scopeval[32];
+};
+
+static int GetStrings(const char *str, struct ConfigStrings *p)
+{
+    pcre2_match_data *match = NULL;
+
+    if (str == NULL || strlen(str) == 0) {
+        SCLogError("config keywords need arguments");
+        return -1;
+    }
     SCLogDebug("str %s", str);
 
     int ret = DetectParsePcreExec(&parse_regex, &match, str, 0, 0);
@@ -194,112 +173,153 @@ static int DetectConfigSetup (DetectEngineCtx *de_ctx, Signature *s, const char
         SCLogError("config is rather picky at this time");
         goto error;
     }
-    pcre2len = sizeof(subsys);
-    res = pcre2_substring_copy_bynumber(match, 1, (PCRE2_UCHAR8 *)subsys, &pcre2len);
+    size_t pcre2len = sizeof(p->subsys);
+    int res = pcre2_substring_copy_bynumber(match, 1, (PCRE2_UCHAR8 *)p->subsys, &pcre2len);
     if (res < 0) {
-        SCLogError("pcre2_substring_copy_bynumber failed");
+        SCLogError("failed to copy subsys substring");
         goto error;
     }
 
-    if (strcmp(subsys, "logging") != 0) {
-        SCLogError("only 'logging' supported at this time");
+    pcre2len = sizeof(p->state);
+    res = pcre2_substring_copy_bynumber(match, 2, (PCRE2_UCHAR8 *)p->state, &pcre2len);
+    if (res < 0) {
+        SCLogError("failed to copy state substring");
         goto error;
     }
-    SCLogDebug("subsys %s", subsys);
 
-    pcre2len = sizeof(state);
-    res = pcre2_substring_copy_bynumber(match, 2, (PCRE2_UCHAR8 *)state, &pcre2len);
+    pcre2len = sizeof(p->type);
+    res = pcre2_substring_copy_bynumber(match, 3, (PCRE2_UCHAR8 *)p->type, &pcre2len);
     if (res < 0) {
-        SCLogError("pcre2_substring_copy_bynumber failed");
+        SCLogError("failed to copy type substring");
         goto error;
     }
 
-    if (strcmp(state, "disable") != 0) {
-        SCLogError("only 'disable' supported at this time");
+    pcre2len = sizeof(p->typeval);
+    res = pcre2_substring_copy_bynumber(match, 4, (PCRE2_UCHAR8 *)p->typeval, &pcre2len);
+    if (res < 0) {
+        SCLogError("failed to copy typeval substring");
         goto error;
     }
-    SCLogDebug("state %s", state);
 
-    pcre2len = sizeof(type);
-    res = pcre2_substring_copy_bynumber(match, 3, (PCRE2_UCHAR8 *)type, &pcre2len);
+    pcre2len = sizeof(p->scope);
+    res = pcre2_substring_copy_bynumber(match, 5, (PCRE2_UCHAR8 *)p->scope, &pcre2len);
     if (res < 0) {
-        SCLogError("pcre2_substring_copy_bynumber failed");
+        SCLogError("failed to copy scope substring");
         goto error;
     }
 
-    if (strcmp(type, "type") != 0) {
-        SCLogError("only 'type' supported at this time");
+    pcre2len = sizeof(p->scopeval);
+    res = pcre2_substring_copy_bynumber(match, 6, (PCRE2_UCHAR8 *)p->scopeval, &pcre2len);
+    if (res < 0) {
+        SCLogError("failed to copy scopeval substring");
         goto error;
     }
-    SCLogDebug("type %s", type);
 
-    pcre2len = sizeof(typeval);
-    res = pcre2_substring_copy_bynumber(match, 4, (PCRE2_UCHAR8 *)typeval, &pcre2len);
-    if (res < 0) {
-        SCLogError("pcre2_substring_copy_bynumber failed");
-        goto error;
+    pcre2_match_data_free(match);
+    return 0;
+error:
+    pcre2_match_data_free(match);
+    return -1;
+}
+
+static bool ParseValues(const struct ConfigStrings *c, enum ConfigType *type,
+        enum ConfigSubsys *subsys, enum ConfigScope *scope)
+{
+    SCLogDebug("subsys %s", c->subsys);
+    if (strcmp(c->subsys, "logging") == 0) {
+        *subsys = CONFIG_SUBSYS_LOGGING;
+    } else {
+        SCLogError("subsys %s not supported: only 'logging' supported at this time", c->subsys);
+        return false;
     }
 
-    if (!(strcmp(typeval, "tx") == 0 ||strcmp(typeval, "flow") == 0)) {
-        SCLogError("only 'tx' and 'flow' supported at this time");
-        goto error;
+    SCLogDebug("state %s", c->state);
+    if (strcmp(c->state, "disable") != 0) {
+        SCLogError("only 'disable' supported at this time");
+        return false;
     }
-    SCLogDebug("typeval %s", typeval);
 
-    pcre2len = sizeof(scope);
-    res = pcre2_substring_copy_bynumber(match, 5, (PCRE2_UCHAR8 *)scope, &pcre2len);
-    if (res < 0) {
-        SCLogError("pcre2_substring_copy_bynumber failed");
-        goto error;
+    SCLogDebug("type %s", c->type);
+    if (strcmp(c->type, "type") != 0) {
+        SCLogError("only 'type' supported at this time");
+        return false;
     }
 
-    if (strcmp(scope, "scope") != 0) {
-        SCLogError("only 'scope' supported at this time");
-        goto error;
+    SCLogDebug("typeval %s", c->typeval);
+    if (strcmp(c->typeval, "tx") == 0) {
+        *type = CONFIG_TYPE_TX;
+    } else if (strcmp(c->typeval, "flow") == 0) {
+        *type = CONFIG_TYPE_FLOW;
+    } else {
+        SCLogError("only 'tx' and 'flow' supported at this time");
+        return false;
     }
-    SCLogDebug("scope %s", scope);
 
-    pcre2len = sizeof(scopeval);
-    res = pcre2_substring_copy_bynumber(match, 6, (PCRE2_UCHAR8 *)scopeval, &pcre2len);
-    if (res < 0) {
-        SCLogError("pcre2_substring_copy_bynumber failed");
-        goto error;
+    SCLogDebug("scope %s", c->scope);
+    if (strcmp(c->scope, "scope") != 0) {
+        SCLogError("only 'scope' supported at this time");
+        return false;
     }
 
-    if (!(strcmp(scopeval, "tx") == 0 ||strcmp(scopeval, "flow") == 0)) {
+    if (strcmp(c->scopeval, "tx") == 0) {
+        *scope = CONFIG_SCOPE_TX;
+    } else if (strcmp(c->scopeval, "flow") == 0) {
+        *scope = CONFIG_SCOPE_FLOW;
+    } else {
         SCLogError("only 'tx' and 'flow' supported at this time");
-        goto error;
+        return false;
     }
-    SCLogDebug("scopeval %s", scopeval);
+    SCLogDebug("scopeval %s", c->scopeval);
+    return true;
+}
 
-    fd = SCCalloc(1, sizeof(DetectConfigData));
-    if (unlikely(fd == NULL))
-        goto error;
+/**
+ * \brief this function is used to parse config option into the current signature
+ *
+ * \param de_ctx pointer to the Detection Engine Context
+ * \param s pointer to the Current Signature
+ * \param str pointer to the user provided "config" input option string
+ *
+ * \retval 0 on Success
+ * \retval -1 on Failure
+ */
+static int DetectConfigSetup(DetectEngineCtx *de_ctx, Signature *s, const char *str)
+{
+    SCEnter();
+
+    struct ConfigStrings c;
+    memset(&c, 0, sizeof(c));
 
-    if (strcmp(typeval, "tx") == 0) {
-        fd->type = CONFIG_TYPE_TX;
+    if (GetStrings(str, &c) != 0) {
+        SCReturnInt(-1);
     }
-    if (strcmp(scopeval, "tx") == 0) {
-        fd->scope = CONFIG_SCOPE_TX;
+
+    enum ConfigType type;
+    enum ConfigSubsys subsys;
+    enum ConfigScope scope;
+
+    if (ParseValues(&c, &type, &subsys, &scope) == false) {
+        SCReturnInt(-1);
     }
 
+    DetectConfigData *fd = SCCalloc(1, sizeof(DetectConfigData));
+    if (unlikely(fd == NULL))
+        return -1;
+
+    fd->type = type;
+    fd->scope = scope;
+    fd->subsys = subsys;
+
     if (fd->scope == CONFIG_SCOPE_TX) {
         s->flags |= SIG_FLAG_APPLAYER;
     }
 
     if (SCSigMatchAppendSMToList(
                 de_ctx, s, DETECT_CONFIG, (SigMatchCtx *)fd, DETECT_SM_LIST_POSTMATCH) == NULL) {
-        goto error;
+        return -1;
     }
 
-    pcre2_match_data_free(match);
     return 0;
-
-error:
-    if (match) {
-        pcre2_match_data_free(match);
-    }
-    return -1;
 }
 
 static void DetectConfigFree(DetectEngineCtx *de_ctx, void *ptr)