detect/firewall: sweep defaults up to progress

When the tx progress crosses more than one app state and the last rule
for a direction sits below the current progress, the no-match path set
fw_next_progress_missing but never applied the default policies of the
states in between (only the match path did). With the hello phase now
entered at the start of the message, the record that completes a hello
walks over hello -> cert: without the missing-state policies a packet
whose last rule was at a lower hook was decided by the packet default
policy even though the app policy for the current state says accept.

Apply the missing-state policies up to the final state on the no-match
path as well, mirroring the match path - but only when the policy of
the current hook does not already carry the verdict: an accept with tx
or flow scope has decided the flow, and sweeping the config-less states
above it would hit the implicit drop:flow default and kill a flow the
rule just accepted.

Bound the sweep at the state the packet actually reached (tx_progress),
at both call sites: a state beyond the current progress was not walked
over, so its configured default must not decide this packet. Also make
a drop from the no-match sweep end the walk, like a policy drop at the
current hook does.

Ticket: #9047.
pull/16368/head
Victor Julien 1 month ago
parent fa695d3563
commit 76f7871783

@ -2128,11 +2128,16 @@ static void DetectRunTxFirewallApplyAccept(DetectEngineThreadCtx *det_ctx, Packe
fw_state->skip_before_progress, BOOL2STR(fw_state->fw_last_for_progress),
BOOL2STR(fw_state->fw_next_progress_missing));
/* if there is no fw rule for the next progress value,
* we invoke the defaul policies for the remaining available hooks. */
if (fw_state->fw_next_progress_missing) {
* we invoke the defaul policies for the remaining available hooks.
* Only when the tx walked over this rule's hook: at progress == hook
* the sweep range would start above its end. */
if (fw_state->fw_next_progress_missing && tx->tx_progress > s->app_progress_hook) {
/* only states the packet actually walked over: states beyond the
* current progress have not been reached, their defaults must not
* decide this packet */
const uint8_t last_hook = fw_state->last_fw_rule
? tx->tx_end_state
: MIN(tx->tx_end_state, s->app_progress_hook + 1);
? tx->tx_progress
: MIN(tx->tx_progress, s->app_progress_hook + 1);
enum DetectTxFirewallFlowControl r =
DetectFirewallApplyDefaultPolicies(det_ctx, det_ctx->de_ctx->fw_policies, tx, p,
s->alproto, direction, s->app_progress_hook + 1, last_hook);
@ -2315,6 +2320,32 @@ static int DetectRunTxFirewallRuleNoMatch(DetectEngineThreadCtx *det_ctx, const
if (policy.action & ACTION_DROP) {
return 1;
}
/* an accept with tx or flow scope already carries the verdict. For an
* accept:hook (which does not apply to this packet) the states walked
* over must still be decided: if the tx progressed beyond this rule's
* hook and there is no rule for the states in between, their default
* policies must be applied too - same as the match path does.
* Without this a packet that crosses two states (e.g. a hello
* completing into the certificate phase) would be decided by the
* packet default policy even though the app policy for the current
* state says accept. */
if ((policy.action & ACTION_ACCEPT) && policy.action_scope == ACTION_SCOPE_HOOK &&
fw_state->fw_next_progress_missing && tx->tx_progress > s->app_progress_hook) {
/* only states the packet actually walked over (see the match
* path) */
const uint8_t last_hook = fw_state->last_fw_rule
? tx->tx_progress
: MIN(tx->tx_progress, s->app_progress_hook + 1);
enum DetectTxFirewallFlowControl r =
DetectFirewallApplyDefaultPolicies(det_ctx, det_ctx->de_ctx->fw_policies, tx, p,
s->alproto, flow_flags, s->app_progress_hook + 1, last_hook);
if (r == DETECT_TX_FW_FC_BREAK) {
/* the sweep dropped the packet: same as a current-hook
* policy drop, the caller must stop the walk */
fw_state->fw_skip_app_filter = true;
return 1;
}
}
}
return 0;
}

Loading…
Cancel
Save