firewall: reject flow scope for the pre-flow hook

Action scopes for packet hook default policies were validated per hook
class, so `flow` was accepted for every packet hook. The pre_flow hook is
a special case as it is evaluated before Suricata obtains a flow record.
Firewall rules on the pre_flow hook were not validated at all, so `flow`
and `tx` scopes were accepted there as well.

This commit adds extra validation to scope parsing for the pre_flow
hook default policy and validates pre_flow firewall rules against the
same scopes.

Ticket: 8967
This commit is contained in:
Lukas Sismis
2026-10-08 13:08:23 +00:00
committed by Victor Julien
parent 6fe3a3dfc3
commit a329e0a3b9
3 changed files with 92 additions and 55 deletions
+10 -1
View File
@@ -420,8 +420,17 @@ Precedence:
An action scope must be valid for the hook it is applied to. For example,
defining ``accept:tx`` as a global default policy will fail to start Suricata,
because ``packet`` policies do not accept ``tx``.
Similarly, ``packet.pre-flow`` only accepts the ``packet`` and ``hook`` scopes,
so a ``drop:flow`` set in ``policies.default-policy`` or
``packet.default-policy`` will fail to start Suricata.
Cover such hooks with a more specific setting so the incompatible default never
reaches them.
reaches them::
firewall:
policies:
default-policy: ["drop:flow"]
packet:
pre-flow: ["accept:hook"]
A ``<`` hook rule at the protocol's first state (progress 0) is accepted and is
equivalent to the plain hook form, as there are no prior states to auto-accept.
+81 -54
View File
@@ -111,6 +111,7 @@ typedef struct SignatureParser_ {
/** Valid action scopes per firewall hook class. */
enum DetectFirewallPolicyClass {
DETECT_FIREWALL_POLICY_CLASS_PACKET,
DETECT_FIREWALL_POLICY_CLASS_PACKET_PRE_FLOW,
DETECT_FIREWALL_POLICY_CLASS_APP
};
@@ -119,6 +120,10 @@ static const uint8_t fw_packet_hook_scopes[] = {
ACTION_SCOPE_HOOK,
ACTION_SCOPE_FLOW,
};
static const uint8_t fw_packet_pre_flow_hook_scopes[] = {
ACTION_SCOPE_PACKET,
ACTION_SCOPE_HOOK,
};
static const uint8_t fw_app_hook_scopes[] = {
ACTION_SCOPE_FLOW,
ACTION_SCOPE_TX,
@@ -2449,6 +2454,67 @@ void SigFree(DetectEngineCtx *de_ctx, Signature *s)
SCFree(s);
}
static bool FirewallScopeValidForClass(uint8_t scope, enum DetectFirewallPolicyClass pol_class)
{
const uint8_t *set = NULL;
size_t n = 0;
switch (pol_class) {
case DETECT_FIREWALL_POLICY_CLASS_PACKET:
set = fw_packet_hook_scopes;
n = ARRAY_SIZE(fw_packet_hook_scopes);
break;
case DETECT_FIREWALL_POLICY_CLASS_PACKET_PRE_FLOW:
set = fw_packet_pre_flow_hook_scopes;
n = ARRAY_SIZE(fw_packet_pre_flow_hook_scopes);
break;
case DETECT_FIREWALL_POLICY_CLASS_APP:
set = fw_app_hook_scopes;
n = ARRAY_SIZE(fw_app_hook_scopes);
break;
default:
FatalError("Invalid firewall policy class %u", (unsigned)pol_class);
}
for (size_t i = 0; i < n; i++) {
if (set[i] == scope) {
return true;
}
}
return false;
}
/**
* \brief Render the valid scopes for a hook class to a string.
*/
static void FirewallScopeHintForClass(
enum DetectFirewallPolicyClass pol_class, char *out, size_t out_size)
{
const uint8_t *set = NULL;
size_t n = 0;
switch (pol_class) {
case DETECT_FIREWALL_POLICY_CLASS_PACKET:
set = fw_packet_hook_scopes;
n = ARRAY_SIZE(fw_packet_hook_scopes);
break;
case DETECT_FIREWALL_POLICY_CLASS_PACKET_PRE_FLOW:
set = fw_packet_pre_flow_hook_scopes;
n = ARRAY_SIZE(fw_packet_pre_flow_hook_scopes);
break;
case DETECT_FIREWALL_POLICY_CLASS_APP:
set = fw_app_hook_scopes;
n = ARRAY_SIZE(fw_app_hook_scopes);
break;
default:
FatalError("Invalid firewall policy class %u", (unsigned)pol_class);
}
out[0] = '\0';
for (size_t i = 0; i < n; i++) {
if ((i > 0 && strlcat(out, "/", out_size) >= out_size) ||
strlcat(out, ActionScopeToString((enum ActionScope)set[i]), out_size) >= out_size) {
FatalError("firewall policy scope hint too long");
}
}
}
/**
* \brief this function is used to set multiple possible app-layer protos
* \brief into the current signature (for example ja4 for both tls and quic)
@@ -2800,6 +2866,17 @@ static bool DetectFirewallRuleValidate(const DetectEngineCtx *de_ctx, const Sign
s->id);
return false;
}
if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_PKT &&
s->init_data->hook.t.pkt.ph == SIGNATURE_HOOK_PKT_PRE_FLOW &&
!FirewallScopeValidForClass(
s->action_scope, DETECT_FIREWALL_POLICY_CLASS_PACKET_PRE_FLOW)) {
char hint[32]; // space to combine ActionScopeToString results
FirewallScopeHintForClass(DETECT_FIREWALL_POLICY_CLASS_PACKET_PRE_FLOW, hint, sizeof(hint));
SCLogError("rule %u: action scope (\"%s\") is not valid for the \"pre_flow\" hook. Valid "
"scopes: %s",
s->id, ActionScopeToString((enum ActionScope)s->action_scope), hint);
return false;
}
if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_APP) {
switch (s->action_scope) {
case ACTION_SCOPE_PACKET:
@@ -4246,59 +4323,6 @@ static int DoParsePolicy(const char *policy_name, struct DetectFirewallPolicy *p
return 1;
}
static bool FirewallScopeValidForClass(uint8_t scope, enum DetectFirewallPolicyClass pol_class)
{
const uint8_t *set = NULL;
size_t n = 0;
switch (pol_class) {
case DETECT_FIREWALL_POLICY_CLASS_PACKET:
set = fw_packet_hook_scopes;
n = ARRAY_SIZE(fw_packet_hook_scopes);
break;
case DETECT_FIREWALL_POLICY_CLASS_APP:
set = fw_app_hook_scopes;
n = ARRAY_SIZE(fw_app_hook_scopes);
break;
default:
FatalError("Invalid firewall policy class %u", (unsigned)pol_class);
}
for (size_t i = 0; i < n; i++) {
if (set[i] == scope) {
return true;
}
}
return false;
}
/**
* \brief Render the valid scopes for a hook class to a string.
*/
static void FirewallScopeHintForClass(
enum DetectFirewallPolicyClass pol_class, char *out, size_t out_size)
{
const uint8_t *set = NULL;
size_t n = 0;
switch (pol_class) {
case DETECT_FIREWALL_POLICY_CLASS_PACKET:
set = fw_packet_hook_scopes;
n = ARRAY_SIZE(fw_packet_hook_scopes);
break;
case DETECT_FIREWALL_POLICY_CLASS_APP:
set = fw_app_hook_scopes;
n = ARRAY_SIZE(fw_app_hook_scopes);
break;
default:
FatalError("Invalid firewall policy class %u", (unsigned)pol_class);
}
out[0] = '\0';
for (size_t i = 0; i < n; i++) {
if ((i > 0 && strlcat(out, "/", out_size) >= out_size) ||
strlcat(out, ActionScopeToString((enum ActionScope)set[i]), out_size) >= out_size) {
FatalError("firewall policy scope hint too long");
}
}
}
/**
* \brief Append a unique inheritance tier to the chain of firewall policies to query.
*/
@@ -4500,8 +4524,11 @@ static int DetectFirewallLoadPacketPolicy(struct DetectFirewallPolicies *fw_poli
/* <prefix>.default-policy */
FirewallPolicyChainAdd(&chain, "%s.default-policy", prefix);
const enum DetectFirewallPolicyClass pol_class =
(id == DETECT_FIREWALL_POLICY_PRE_FLOW) ? DETECT_FIREWALL_POLICY_CLASS_PACKET_PRE_FLOW
: DETECT_FIREWALL_POLICY_CLASS_PACKET;
struct DetectFirewallPolicy *pol = &fw_policies->pkt[id]; // built-in default
int r = ResolveFirewallPolicy(pol, DETECT_FIREWALL_POLICY_CLASS_PACKET, &chain);
int r = ResolveFirewallPolicy(pol, pol_class, &chain);
if (r < 0) {
return -1;
}
+1
View File
@@ -2410,6 +2410,7 @@ firewall:
# default-policy: ["drop:flow"]
# packet:
# filter: ["drop:packet"]
# pre-flow: ["accept:hook"]
# app:
# dns:
# request-started: ["accept:hook"]