Project

General

Profile

Actions

Bug #8944

open
YD VJ

firewall: auto-accept-prior-states not honored when a same-hook drop rule leads the candidate list

Bug #8944: firewall: auto-accept-prior-states not honored when a same-hook drop rule leads the candidate list

Added by Yash Datre about 2 months ago. Updated 7 days ago.

Status:
In Review
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

In firewall mode, the "auto-accept prior states" notation (accept:... proto:<hook) is documented to accept the target hook and all hooks before it. This does not hold when a lower-SID drop rule at the same target hook has a prefilter that matches the traffic: that drop rule becomes the list-leading firewall candidate, the prior hook is then treated as uncovered ("missing fw rules at list start"), and the flow is dropped by the default app policy at the prior hook -- before the drop rule is ever evaluated.

Net effect: the intended per-SNI drop rule never fires (no alert), and the flow is instead terminated by the default policy. The block still happens, but for the wrong reason and without the rule's alert, and it depends entirely on the default policy being DROP rather than on the authored rule.

Steps to reproduce

Firewall ruleset (no explicit accept:hook tls:client_in_progress):

accept:hook tcp:all $HOME_NET any <> $EXTERNAL_NET any (flow:not_established; sid:1021;)
accept:hook tcp:all $HOME_NET any <> $EXTERNAL_NET any (flow:established; sid:1022;)
drop:flow   tls:client_hello_done  $HOME_NET any -> $EXTERNAL_NET any (tls.sni; content:"www.google.com"; endswith; nocase; alert; sid:200;)
accept:flow tls:<client_hello_done $HOME_NET any -> $EXTERNAL_NET any (alert; sid:201;)

Replay a TLS session whose SNI is www.google.com (e.g. tests/tls/tls-client-hello-frag-01/dump_mtu300.pcap) in simulated IPS mode:

suricata --simulate-ips -k none -c suricata.yaml \
    -r dump_mtu300.pcap --firewall-rules-exclusive firewall.rules

Expected behaviour

The SNI matches sid:200, so the flow is dropped by the rule:

  • event_type: alert, alert.signature_id: 200, alert.action: blocked, firewall.hook: "tls:client_hello_done"
  • first drop drop.reason: "firewall rules"
  • flow.alerted: true
  • stats.firewall.drop_reason.rules >= 1, default_app_policy: 0

Actual behaviour

  • sid:200 never fires; no alert is generated (flow.alerted: false).
  • First drop is drop.reason: "firewall default app policy", followed by "firewall flow drop" for the remaining packets.
  • stats.firewall.drop_reason: rules: 0, default_app_policy: 1, flow_drop: 56.

When the SNI does not match sid:200's prefilter, sid:201 (the < rule) leads the candidate list, its auto-accept covers the prior hook, and the flow is accepted as intended. So the prior-hook auto-accept is only honored when the < rule itself leads the candidate list.

Workaround

Add an explicit accept for the prior hook so it is covered regardless of candidate ordering:

accept:hook tls:<client_in_progress $HOME_NET any -> $EXTERNAL_NET any (sid:199;)

With that rule present, sid:200 drops www.google.com (drop.reason: "firewall rules", with the alert) and flow.alerted is true. (The existing ruletype-firewall-04/05-ruleset-vs-sni tests already include such an explicit client_in_progress accept.)

Regression test

A suricata-verify test that asserts the expected behaviour (and currently fails, reproducing the bug):


Subtasks 1 (1 open — 0 closed)

Bug #9154: firewall: auto-accept-prior-states not honored when a same-hook drop rule leads the candidate list (8.0.x backport)AssignedVictor JulienActions

Related issues 3 (3 open — 0 closed)

Related to Suricata - Bug #9046: ssh: clean up tx state machineResolvedVictor JulienActions
Related to Suricata - Bug #9047: tls: rewrite tx state machineResolvedVictor JulienActions
Related to Suricata - Bug #9122: firewall: file.data auto-prior-accept bypassResolvedVictor JulienActions

VJ Updated by Victor Julien about 2 months ago Actions #1

  • Status changed from New to In Progress
  • Assignee set to Victor Julien

VJ Updated by Victor Julien about 2 months ago Actions #2

  • Affected Versions git main added
  • Affected Versions deleted (9.0.0-beta1)

VJ Updated by Victor Julien about 1 month ago Actions #3

Quick status update: I have a fix for the initial report, but review has uncovered new issues and this is leading to a more fundamental issue with how different protocols in Suricata track state and hook keywords into them. I'm still mapping this currently.

VJ Updated by Victor Julien 26 days ago Actions #4

  • Related to Bug #9046: ssh: clean up tx state machine added

NS Updated by Naijian Sun 22 days ago Actions #5

Noticed anothe issue. Not sure if same root cause as this one.

accept:hook tcp:all any any <> any any (sid:1;)
accept:flow tls:<client_hello_done any any -> any any (tls.sni; content:"example.com"; sid:2;)
accept:flow http1:<request_line any any -> any any (http.uri; content:"/nope"; sid:5;)
accept:flow http1:<request_headers any any -> any any (http.host; content:"example.com"; sid:6;)

curl -sik --http1.1 http://example.com/api

Expect this to pass by sid:6, but got dropped by sid:5.

VJ Updated by Victor Julien 22 days ago Actions #6

Naijian Sun wrote in #note-5:

Noticed anothe issue. Not sure if same root cause as this one.

accept:hook tcp:all any any <> any any (sid:1;)
accept:flow tls:<client_hello_done any any -> any any (tls.sni; content:"example.com"; sid:2;)
accept:flow http1:<request_line any any -> any any (http.uri; content:"/nope"; sid:5;)
accept:flow http1:<request_headers any any -> any any (http.host; content:"example.com"; sid:6;)

curl -sik --http1.1 http://example.com/api

Expect this to pass by sid:6, but got dropped by sid:5.

Yes this is one of the edge cases already identified. Working on a fix.

VJ Updated by Victor Julien 21 days ago Actions #7

  • Related to Bug #9047: tls: rewrite tx state machine added

VJ Updated by Victor Julien 20 days ago Actions #8

  • Related to Bug #9122: firewall: file.data auto-prior-accept bypass added

VJ Updated by Victor Julien 10 days ago Actions #9

  • Status changed from In Progress to Resolved
  • Priority changed from High to Normal
  • Label Needs backport to 8.0 added

OT Updated by OISF Ticketbot 10 days ago Actions #10

  • Subtask #9154 added

OT Updated by OISF Ticketbot 10 days ago Actions #11

  • Label deleted (Needs backport to 8.0)

VJ Updated by Victor Julien 7 days ago Actions #12

  • Status changed from Resolved to In Review
Actions

Also available in: PDF Atom