detect/firewall: implement app policy as hash table

With substate support the fixed table approach for policies was no
longer a good fit, so convert to a hash table. The policies are stored
per alproto, sub_state, progress and direction.

src/detect-engine-analyzer.c

@@ -2135,13 +2135,16 @@ static void AddPolicy(const DetectEngineCtx *de_ctx, RuleAnalyzer *ctx, const Ap
         const uint8_t state, const uint8_t direction)
 {
     char policy_string[64] = "";
-    const struct DetectFirewallPolicy *p = NULL;
     const struct DetectFirewallPolicies *fw_policies = de_ctx->fw_policies;
-    if (direction == STREAM_TOSERVER) {
-        p = &fw_policies->app[a].ts[state];
-    } else {
-        p = &fw_policies->app[a].tc[state];
-    }
+    const struct DetectFirewallAppPolicy lookup = {
+        .alproto = a, .sub_state = 0, .progress = state, .direction = direction
+    };
+    const struct DetectFirewallAppPolicy *ap =
+            HashTableLookup(fw_policies->app_policies, (void *)&lookup, 0);
+    if (ap == NULL)
+        return;
+    const struct DetectFirewallPolicy *p = &ap->policy;
+
     const char *as = ActionScopeToString(p->action_scope);
     DEBUG_VALIDATE_BUG_ON(as == NULL);
     if (as == NULL)

src/detect-engine.c

@@ -2920,6 +2920,7 @@ void DetectEngineCtxFree(DetectEngineCtx *de_ctx)
             }
         }
         HashTableFree(de_ctx->fw_policies->policy_signatures);
+        HashTableFree(de_ctx->fw_policies->app_policies);
     }
     SCFree(de_ctx->fw_policies);
     SCFree(de_ctx);

src/detect-parse.c

@@ -3945,6 +3945,33 @@ void DetectSetupParseRegexes(const char *parse_str, DetectParseRegex *detect_par
     }
 }
 
+static uint32_t AppPolicyHashFunc(HashTable *ht, void *data, uint16_t datalen)
+{
+    const struct DetectFirewallAppPolicy *p = data;
+    /* use a prime-mix hash */
+    uint32_t hash = p->alproto * 65537 + p->sub_state * 257 + p->progress * 5 +
+                    (p->direction == STREAM_TOSERVER);
+    hash ^= (hash >> 10) ^ (hash >> 20);
+    return hash % ht->array_size;
+}
+
+static char AppPolicyCompareFunc(void *data1, uint16_t datalen1, void *data2, uint16_t datalen2)
+{
+    const struct DetectFirewallAppPolicy *p1 = data1;
+    const struct DetectFirewallAppPolicy *p2 = data2;
+
+    if (p1 == NULL || p2 == NULL)
+        return 0;
+
+    return p1->direction == p2->direction && p1->alproto == p2->alproto &&
+           p1->sub_state == p2->sub_state && p1->progress == p2->progress;
+}
+
+static void AppPolicyHashFree(void *data)
+{
+    struct DetectFirewallAppPolicy *p = data;
+    SCFree(p);
+}
 static uint32_t PolicySignatureHashFunc(HashTable *ht, void *data, uint16_t datalen)
 {
     const Signature *s = data;
@@ -4125,7 +4152,7 @@ static int DoParsePolicy(const char *policy_name, struct DetectFirewallPolicy *p
 static int DoParseAppSubStatePolicy(const char *prefix, const AppProto app_proto,
         const uint8_t sub_state, const char *sub_state_name, const uint8_t state,
         const char *hookname, const uint8_t complete_state, const int direction,
-        struct DetectFirewallPolicies *fw_policies, struct DetectFirewallAppPolicy *app_fw_policies)
+        struct DetectFirewallPolicies *fw_policies)
 {
     char policy_name[256];
     BUG_ON(sub_state_name == NULL);
@@ -4148,18 +4175,33 @@ static int DoParseAppSubStatePolicy(const char *prefix, const AppProto app_proto
         FatalError("internal error: failed to assemble firewall policy config string");
     }
 
-    struct DetectFirewallPolicy *pol;
-    if (direction == STREAM_TOSERVER)
-        pol = &fw_policies->http2_substates[sub_state - 1].ts[state];
-    else
-        pol = &fw_policies->http2_substates[sub_state - 1].tc[state];
+    struct DetectFirewallAppPolicy *app_pol = SCCalloc(1, sizeof(*app_pol));
+    if (app_pol == NULL)
+        return -1;
+
+    app_pol->alproto = app_proto;
+    app_pol->sub_state = sub_state;
+    app_pol->progress = state;
+    app_pol->direction = (uint8_t)direction;
+    /* init to drop:flow by default, will be overwritten by DoParsePolicy if there
+     * is a config for this hook. */
+    app_pol->policy.action = ACTION_DROP;
+    app_pol->policy.action_scope = ACTION_SCOPE_FLOW;
+
+    r = DoParsePolicy(policy_name, &app_pol->policy);
+    if (r < 0) {
+        SCFree(app_pol);
+        return -1;
+    }
 
-    r = DoParsePolicy(policy_name, pol);
+    if (HashTableAdd(fw_policies->app_policies, app_pol, 0) != 0) {
+        FatalError("internal error: insert policy into hash table");
+    }
     /* for policies with an alert action, create a policy sig */
-    if (r == 1 && pol->action & ACTION_ALERT) {
+    if (r == 1 && app_pol->policy.action & ACTION_ALERT) {
         SCLogDebug("adding policy signature");
         return AddAppPolicySignature(fw_policies->policy_signatures, direction, app_proto, app_name,
-                state, hookname, pol);
+                state, hookname, &app_pol->policy);
     }
     SCLogDebug("r %d", r);
     return r;
@@ -4167,7 +4209,7 @@ static int DoParseAppSubStatePolicy(const char *prefix, const AppProto app_proto
 
 static int DoParseAppPolicy(const char *prefix, const AppProto app_proto, const char *hookname,
         const uint8_t state, const uint8_t complete_state, const int direction,
-        struct DetectFirewallPolicies *fw_policies, struct DetectFirewallAppPolicy *app_fw_policies)
+        struct DetectFirewallPolicies *fw_policies)
 {
     char policy_name[256];
     const char *in_name = hookname;
@@ -4201,12 +4243,20 @@ static int DoParseAppPolicy(const char *prefix, const AppProto app_proto, const
         FatalError("internal error: failed to assemble firewall policy config string");
     }
 
-    struct DetectFirewallPolicy *pol;
-    if (direction == STREAM_TOSERVER)
-        pol = &app_fw_policies[app_proto].ts[state];
-    else
-        pol = &app_fw_policies[app_proto].tc[state];
-    r = DoParsePolicy(policy_name, pol);
+    struct DetectFirewallAppPolicy *app_pol = SCCalloc(1, sizeof(*app_pol));
+    if (app_pol == NULL)
+        return -1;
+
+    app_pol->alproto = app_proto;
+    app_pol->sub_state = 0;
+    app_pol->progress = state;
+    app_pol->direction = (uint8_t)direction;
+    /* init to drop:flow by default, will be overwritten by DoParsePolicy if there
+     * is a config for this hook. */
+    app_pol->policy.action = ACTION_DROP;
+    app_pol->policy.action_scope = ACTION_SCOPE_FLOW;
+
+    r = DoParsePolicy(policy_name, &app_pol->policy);
     if (r == 0 && in_name != NULL) {
         if (state == 0) {
             if (direction == STREAM_TOSERVER)
@@ -4226,32 +4276,46 @@ static int DoParseAppPolicy(const char *prefix, const AppProto app_proto, const
             FatalError("internal error: failed to assemble firewall policy config string");
         }
 
-        r = DoParsePolicy(policy_name, pol);
+        r = DoParsePolicy(policy_name, &app_pol->policy);
+    }
+    if (r < 0) {
+        SCFree(app_pol);
+        return -1;
+    }
+
+    if (HashTableAdd(fw_policies->app_policies, app_pol, 0) != 0) {
+        FatalError("internal error: insert policy into hash table");
     }
 
     /* for policies with an alert action, create a policy sig */
-    if (r == 1 && pol->action & ACTION_ALERT) {
+    if (r == 1 && app_pol->policy.action & ACTION_ALERT) {
         SCLogDebug("adding policy signature");
         return AddAppPolicySignature(fw_policies->policy_signatures, direction, app_proto, app_name,
-                state, hookname, pol);
+                state, hookname, &app_pol->policy);
     }
+
     return r;
 }
 
 /** \brief allocate and initialize to default values the policies table */
 int DetectFirewallInitDefaultPolicies(DetectEngineCtx *de_ctx)
 {
-    struct DetectFirewallPolicies *fw_policies = SCCalloc(
-            1, sizeof(*fw_policies) + g_alproto_max * sizeof(struct DetectFirewallAppPolicy));
+    struct DetectFirewallPolicies *fw_policies = SCCalloc(1, sizeof(*fw_policies));
     if (fw_policies == NULL)
         return -1;
-    struct DetectFirewallAppPolicy *app_fw_policies = fw_policies->app;
-    if (app_fw_policies == NULL)
-        goto error;
     fw_policies->policy_signatures = HashTableInit(
             512, PolicySignatureHashFunc, PolicySignatureCompareFunc, PolicySignatureHashFree);
-    if (fw_policies->policy_signatures == NULL)
-        goto error;
+    if (fw_policies->policy_signatures == NULL) {
+        SCFree(fw_policies);
+        return -1;
+    }
+    fw_policies->app_policies =
+            HashTableInit(512, AppPolicyHashFunc, AppPolicyCompareFunc, AppPolicyHashFree);
+    if (fw_policies->app_policies == NULL) {
+        HashTableFree(fw_policies->policy_signatures);
+        SCFree(fw_policies);
+        return -1;
+    }
 
     fw_policies->pkt[DETECT_FIREWALL_POLICY_PACKET_FILTER].action = ACTION_DROP;
     fw_policies->pkt[DETECT_FIREWALL_POLICY_PACKET_FILTER].action_scope = ACTION_SCOPE_PACKET;
@@ -4262,31 +4326,8 @@ int DetectFirewallInitDefaultPolicies(DetectEngineCtx *de_ctx)
     fw_policies->pkt[DETECT_FIREWALL_POLICY_PRE_STREAM].action = ACTION_ACCEPT;
     fw_policies->pkt[DETECT_FIREWALL_POLICY_PRE_STREAM].action_scope = ACTION_SCOPE_HOOK;
 
-    for (AppProto a = 0; a < g_alproto_max; a++) {
-        for (int i = 0; i < 48; i++) {
-            app_fw_policies[a].ts[i].action = ACTION_DROP;
-            app_fw_policies[a].ts[i].action_scope = ACTION_SCOPE_FLOW;
-            app_fw_policies[a].tc[i].action = ACTION_DROP;
-            app_fw_policies[a].tc[i].action_scope = ACTION_SCOPE_FLOW;
-        }
-    }
-
-    /* TODO hard coded for HTTP/2 for now */
-    for (int s = 0; s < 2; s++) {
-        for (int i = 0; i < 48; i++) {
-            fw_policies->http2_substates[s].ts[i].action = ACTION_DROP;
-            fw_policies->http2_substates[s].ts[i].action_scope = ACTION_SCOPE_FLOW;
-            fw_policies->http2_substates[s].tc[i].action = ACTION_DROP;
-            fw_policies->http2_substates[s].tc[i].action_scope = ACTION_SCOPE_FLOW;
-        }
-    }
-
     de_ctx->fw_policies = fw_policies;
     return 0;
-
-error:
-    SCFree(fw_policies);
-    return -1;
 }
 
 int DetectFirewallLoadDefaultPolicies(DetectEngineCtx *de_ctx)
@@ -4301,9 +4342,6 @@ int DetectFirewallLoadDefaultPolicies(DetectEngineCtx *de_ctx)
     struct DetectFirewallPolicies *fw_policies = de_ctx->fw_policies;
     if (fw_policies == NULL)
         return -1;
-    struct DetectFirewallAppPolicy *app_fw_policies = fw_policies->app;
-    if (app_fw_policies == NULL)
-        return -1;
 
     r = snprintf(policy_name, sizeof(policy_name), "%s.packet-filter", prefix);
     if (r < 0 || (size_t)r >= sizeof(policy_name)) {
@@ -4369,7 +4407,7 @@ int DetectFirewallLoadDefaultPolicies(DetectEngineCtx *de_ctx)
                     SCLogDebug("protocol %s: sub state:%s state:%s", AppProtoToString(a),
                             sub_state_name, state_name);
                     if (DoParseAppSubStatePolicy(prefix, a, s, sub_state_name, state, state_name,
-                                max_state, STREAM_TOSERVER, fw_policies, app_fw_policies) < 0)
+                                max_state, STREAM_TOSERVER, fw_policies) < 0)
                         return -1;
                 }
                 /* to_client */
@@ -4382,7 +4420,7 @@ int DetectFirewallLoadDefaultPolicies(DetectEngineCtx *de_ctx)
                     SCLogDebug("protocol %s: to_client: sub state:%s state:%s", AppProtoToString(a),
                             sub_state_name, state_name);
                     if (DoParseAppSubStatePolicy(prefix, a, s, sub_state_name, state, state_name,
-                                max_state, STREAM_TOCLIENT, fw_policies, app_fw_policies) < 0)
+                                max_state, STREAM_TOCLIENT, fw_policies) < 0)
                         return -1;
                 }
             }
@@ -4394,7 +4432,7 @@ int DetectFirewallLoadDefaultPolicies(DetectEngineCtx *de_ctx)
                 const char *name =
                         AppLayerParserGetStateNameById(IPPROTO_TCP, a, state, STREAM_TOSERVER);
                 if (DoParseAppPolicy(prefix, a, name, state, complete_state_ts, STREAM_TOSERVER,
-                            fw_policies, app_fw_policies) < 0)
+                            fw_policies) < 0)
                     return -1;
             }
             const uint8_t complete_state_tc =
@@ -4404,7 +4442,7 @@ int DetectFirewallLoadDefaultPolicies(DetectEngineCtx *de_ctx)
                 const char *name =
                         AppLayerParserGetStateNameById(IPPROTO_TCP, a, state, STREAM_TOCLIENT);
                 if (DoParseAppPolicy(prefix, a, name, state, complete_state_tc, STREAM_TOCLIENT,
-                            fw_policies, app_fw_policies) < 0)
+                            fw_policies) < 0)
                     return -1;
             }
         }

src/detect.c

@@ -1745,36 +1745,16 @@ static const struct DetectFirewallPolicy *DetectFirewallApplyDefaultAppPolicy(
         const DetectTransaction *tx, Packet *p, const AppProto alproto, const uint8_t direction,
         const uint8_t progress)
 {
-    const struct DetectFirewallPolicy *policy;
     SCLogDebug("packet %" PRIu64 ": tx type %u", PcapPacketCntGet(p), tx->tx_type);
-    if (tx->tx_type != 0) {
-        // TODO hard coded to HTTP/2 for now
-        BUG_ON(alproto != ALPROTO_HTTP2);
-        if (direction & STREAM_TOSERVER) {
-            policy = &policies->http2_substates[tx->tx_type - 1].ts[progress];
-            SCLogDebug("packet %" PRIu64
-                       ", sub_state:%u, hook:%u, toserver, policy: action %02x scope %u",
-                    PcapPacketCntGet(p), tx->tx_type, progress, policy->action,
-                    policy->action_scope);
-        } else {
-            policy = &policies->http2_substates[tx->tx_type - 1].tc[progress];
-            SCLogDebug("packet %" PRIu64
-                       ", sub_state:%u, hook:%u, toclient, policy: action %02x scope %u",
-                    PcapPacketCntGet(p), tx->tx_type, progress, policy->action,
-                    policy->action_scope);
-        }
 
-    } else {
-        if (direction & STREAM_TOSERVER) {
-            policy = &policies->app[alproto].ts[progress];
-            SCLogDebug("packet %" PRIu64 ", hook:%u, toserver, policy: action %02x scope %u",
-                    PcapPacketCntGet(p), progress, policy->action, policy->action_scope);
-        } else {
-            policy = &policies->app[alproto].tc[progress];
-            SCLogDebug("packet %" PRIu64 ", hook:%u, toclient, policy: action %02x scope %u",
-                    PcapPacketCntGet(p), progress, policy->action, policy->action_scope);
-        }
-    }
+    const struct DetectFirewallAppPolicy lookup = {
+        .alproto = alproto, .sub_state = tx->tx_type, .progress = progress, .direction = direction
+    };
+    const struct DetectFirewallAppPolicy *ap =
+            HashTableLookup(policies->app_policies, (void *)&lookup, 0);
+    /* table should be fully populated, so this should not be able to fail */
+    DEBUG_VALIDATE_BUG_ON(ap == NULL);
+    const struct DetectFirewallPolicy *policy = &ap->policy;
 
     if (policy->action & ACTION_DROP) {
         SCLogDebug("dropping packet PKT_DROP_REASON_FW_DEFAULT_APP_POLICY");

src/detect.h

@@ -934,12 +934,12 @@ struct DetectFirewallPolicy {
     uint8_t action_scope; /**< same as Signature::action_scope. Scope argument for the action. */
 };
 
-/** Application layer firewall policies per hook. */
 struct DetectFirewallAppPolicy {
-    /** policy per hook/progress value (max 48) for toserver direction. */
-    struct DetectFirewallPolicy ts[48];
-    /** policy per hook/progress value (max 48) for toclient direction. */
-    struct DetectFirewallPolicy tc[48];
+    AppProto alproto;
+    uint8_t sub_state;
+    uint8_t progress;
+    uint8_t direction;
+    struct DetectFirewallPolicy policy;
 };
 
 struct DetectFirewallPolicies {
@@ -950,11 +950,8 @@ struct DetectFirewallPolicies {
     /* hash table with a Signature object per default policy that has `alert` enabled. */
     HashTable *policy_signatures;
 
-    /* hard coded for now: http2 substates. Index at substate - 1. */
-    struct DetectFirewallAppPolicy http2_substates[2];
-
-    /** app layer policies, one per alproto */
-    struct DetectFirewallAppPolicy app[];
+    /* hash table with policies, hashed by alproto, sub_state, progress and direction */
+    HashTable *app_policies;
 };
 
 /* Flow states: