diff --git a/doc/userguide/firewall/firewall-banned-keywords.rst b/doc/userguide/firewall/firewall-banned-keywords.rst new file mode 100644 index 000000000000..652b6b56d23e --- /dev/null +++ b/doc/userguide/firewall/firewall-banned-keywords.rst @@ -0,0 +1,43 @@ +Firewall Mode Banned Rules +========================== + +.. note:: In Suricata 8 the firewall mode is experimental and subject to change. + +Certain rule keywords are banned in firewall mode, either completely or based +on rulesets. + +A rule may be banned from firewall rules, threat detection rules, or from firewall mode altogether. + +This is done when there is a chance that the keyword would lead to the firewall +verdict being skipped, or contradicted, or when there could happen inconsistent +states from interactions between firewall and threat detection rules matching. + +.. note:: Future support may be added to keywords in upcoming releases, in + certain cases. + +Banned Keywords +--------------- + +Firewall Mode +~~~~~~~~~~~~~ + +* Replace + +Firewall Rules +~~~~~~~~~~~~~~ + +* Threshold + +``threshold`` is not currently supported for firewall rules: some of its types +withhold a rule's alert while still applying its actions, which would leave the +firewall verdict and the logged record disagreeing. The restriction also covers +the ``threshold.config`` file, whose entries may not name a firewall rule. + +``detection_filter`` is not restricted. It either applies a rule or it does not, +so below its rate a firewall rule simply does not take effect and evaluation +continues with the next rule. See :doc:`../rules/thresholding`. + +Threat Detection Rules +~~~~~~~~~~~~~~~~~~~~~~ + +* Bypass diff --git a/doc/userguide/firewall/firewall-design.rst b/doc/userguide/firewall/firewall-design.rst index b2eb9d38c721..612f94cb64ce 100644 --- a/doc/userguide/firewall/firewall-design.rst +++ b/doc/userguide/firewall/firewall-design.rst @@ -31,12 +31,16 @@ counted as ``ips.accepted``. If it was dropped by firewall, only ``firewall.bloc will be incremented. No ``ips.*`` counter will be updated as conceptually the TD instance won't have seen the packet. +.. note:: If a firewall rule uses the :ref:`bypass keyword`, an + accepted packet will not be passed along to the TD step of the pipeline. + Tables ------ A ``table`` is a collection of rules with different properties. These tables are built-in. No custom tables can be created. Tables are available within the scope of packet layer -and application layer (if available). Each rule can define its own :ref:`action scope`. +and application layer (if available). Each rule can define its own +:ref:`action scope`. Packet layer tables ~~~~~~~~~~~~~~~~~~~ diff --git a/doc/userguide/firewall/firewall-stats.rst b/doc/userguide/firewall/firewall-stats.rst index d8561e34dcbe..4fb80c4344a4 100644 --- a/doc/userguide/firewall/firewall-stats.rst +++ b/doc/userguide/firewall/firewall-stats.rst @@ -14,14 +14,25 @@ Statistics counters for the firewall mode cover: These will be present in the stats logs if the engine is run in firewall mode, only. +Bypassed packets +================ + +As the firewall bypass does not happen as the primary action in a firewall +policy/rule, the stats counters for bypassed packets continue to be the ones +that already exist. A `bypassed` packet will be counted as an `accepted` packet +in firewall stats. + Drop reasons ============ -If a drop was caused by the firewall, the corresponding counter will be incremented. The existing ones are: +If a drop was caused by the firewall, the corresponding counter will be +incremented. The existing ones are: - ``rules``: a firewall rule triggered the drop - - ``default_packet_policy``: drop caused by the default fail closed firewall behavior, on the packet hook level - - ``default_app_policy``: drop caused by the default fail close firewall behavior, on the app-layer hook level + - ``default_packet_policy``: drop caused by the default fail closed firewall + behavior, on the packet hook level + - ``default_app_policy``: drop caused by the default fail close firewall + behavior, on the app-layer hook level - ``pre_flow_hook``: drop caused by the pre-flow hook - ``pre_stream_hook``: drop caused by the pre-stream hook - ``flow_drop``: the whole flow was dropped after a firewall action. diff --git a/doc/userguide/firewall/index.rst b/doc/userguide/firewall/index.rst index c1386a371b25..484aed84fe9a 100644 --- a/doc/userguide/firewall/index.rst +++ b/doc/userguide/firewall/index.rst @@ -6,3 +6,4 @@ Firewall Mode firewall-design firewall-example firewall-stats + firewall-banned-keywords diff --git a/doc/userguide/rules/bypass-keyword.rst b/doc/userguide/rules/bypass-keyword.rst index 6572f72f0152..aff8c9c8f89a 100644 --- a/doc/userguide/rules/bypass-keyword.rst +++ b/doc/userguide/rules/bypass-keyword.rst @@ -1,3 +1,5 @@ +.. _bypass-keyword: + Bypass Keyword ============== @@ -13,8 +15,11 @@ The ``bypass`` keyword is considered a post-match keyword. .. note:: - ``bypass`` cannot be used in firewall mode, not even with Threat Detection - rules, as this could lead to bypassing the firewall altogether. + In firewall mode, ``bypass`` can only be used in firewall rules. If a threat + detection rule uses the ``bypass`` keyword and you want to run Suricata in + the offending rule will produce an error and won't be loaded. This is to + prevent a threat detection rule from bypassing the firewall altogether. + (To make the engine error out in such a case, use ``--init-errors-fatal``). bypass ------ @@ -26,3 +31,47 @@ Bypass a flow on matching http traffic. alert http any any -> any any (http.host; \ content:"suricata.io"; :example-rule-emphasis:`bypass;` \ sid:10001; rev:1;) + +Firewall mode +------------- + +``bypass`` is only accepted with a specific combination of `action` and `scope`: +``accept:flow``. + +Not accepted: + - Action: ``config`` + - Action: ``reject`` + - Action: ``drop`` + - Scope: ``packet`` + - Scope: ``tx`` + - Scope: ``hook`` + +.. attention:: `bypass` on a firewall rule is a terminating action. Threat + detection rules are not evaluated for the matching packet, respecting the + premise of what would happen if Firewall and IPS were two separate devices. + +.. note:: The type of bypass will depend on whether the engine is configured + for local or capture bypass: offloading is not guaranteed by a firewall + bypass rule. + +Valid firewall rule with bypass: + +.. container:: example-rule + + :example-rule-emphasis:`accept:flow,alert` http1:request_headers any any -> \ + any any (http.host; content:"suricata.io"; :example-rule-emphasis:`bypass;` \ + sid:10001; rev:1;) + + +Special hooks +~~~~~~~~~~~~~ + +``pre_flow`` hook +^^^^^^^^^^^^^^^^^ + +If the ``bypass`` is applied locally, ``pre_flow`` rules will still be processed +and invoked, due to the fact that the engine can't apply nor control a flow +bypass at a stage where the packet hasn't been tied to its flow yet. + +This won't happen in the case of an offloaded bypass, as there won't be +anything for the engine to inspect against. diff --git a/doc/userguide/rules/thresholding.rst b/doc/userguide/rules/thresholding.rst index f18438ba05b7..c6ed7623ab46 100644 --- a/doc/userguide/rules/thresholding.rst +++ b/doc/userguide/rules/thresholding.rst @@ -19,6 +19,19 @@ Thresholds are tracked in a hash table that is sized according to configuration, Rule actions ``drop`` (IPS mode) and ``reject`` are applied to each packet (not only the one that meets the limit condition). +.. note:: + + In firewall mode, the ``threshold`` keyword is not allowed in firewall + rules, and ``threshold.config`` entries may not name a firewall rule's + signature id. + Either leads to a fatal error on engine initialization when + ``--init-errors-fatal`` is set, rather than silently ignoring the entry. + Without that option, and on a rule reload, the entry is skipped with a + warning. Firewall rules matched by a wildcard ``threshold.config`` entry are + always skipped with a warning summarizing how many rules were skipped. + ``detection_filter`` is allowed in firewall rules. Below its rate the rule + does not apply. + threshold --------- diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst index f89362e796f0..6e003984318d 100644 --- a/doc/userguide/upgrade.rst +++ b/doc/userguide/upgrade.rst @@ -77,6 +77,11 @@ Keyword Changes being split per direction. This means that some rules should match sooner, some rules will have less false negatives, and some rules will trigger once per transaction instead of twice (one time for each direction) +- Thresholding is restricted in firewall mode. The ``threshold`` keyword is + rejected in firewall rules, and ``threshold.config`` entries (``threshold``, + ``event_filter``, ``suppress``, ``rate_filter``) may no longer name a firewall + rule's signature id. Wildcard ``threshold.config`` entries skip firewall + rules. ``detection_filter`` and Threat Detection rules are unaffected. Other Changes ~~~~~~~~~~~~~ diff --git a/rust/sys/src/sys.rs b/rust/sys/src/sys.rs index 0f0e7d3d65c8..d6096b58ef4a 100644 --- a/rust/sys/src/sys.rs +++ b/rust/sys/src/sys.rs @@ -24,6 +24,13 @@ pub const SIGMATCH_INFO_ENUM_UINT: u32 = 524288; pub const SIGMATCH_INFO_BITFLAGS_UINT: u32 = 1048576; pub const SIGMATCH_BAN_FIREWALL_RULE: u32 = 2097152; pub const SIGMATCH_BAN_FIREWALL_MODE: u32 = 4194304; +pub const SIGMATCH_BAN_TD_FIREWALL_MODE: u32 = 8388608; +pub const SIGMATCH_BAN_FIREWALL_SCOPE_PACKET: u32 = 16777216; +pub const SIGMATCH_BAN_FIREWALL_SCOPE_TX: u32 = 33554432; +pub const SIGMATCH_BAN_FIREWALL_SCOPE_HOOK: u32 = 67108864; +pub const SIGMATCH_BAN_ACTION_CONFIG: u32 = 134217728; +pub const SIGMATCH_BAN_ACTION_DROP: u32 = 268435456; +pub const SIGMATCH_BAN_ACTION_REJECT: u32 = 536870912; pub type __intmax_t = ::std::os::raw::c_long; pub type intmax_t = __intmax_t; #[repr(u32)] diff --git a/src/decode.c b/src/decode.c index e6800d80063a..ca3a56fde9b7 100644 --- a/src/decode.c +++ b/src/decode.c @@ -1063,6 +1063,13 @@ static bool VerdictByFirewall(const Packet *p) return false; } +static bool PacketBypassed(const Packet *p) +{ + if ((p->flags & PKT_FW_BYPASSED) != 0) + return true; + return false; +} + void CaptureStatsUpdate(ThreadVars *tv, const Packet *p) { if (!EngineModeIsIPS() || PKT_IS_PSEUDOPKT(p)) @@ -1096,7 +1103,9 @@ void CaptureStatsUpdate(ThreadVars *tv, const Packet *p) } } else if (PacketCheckAction(p, ACTION_ACCEPT)) { StatsCounterIncr(&tv->stats, s->counter_fw_accepted); - StatsCounterIncr(&tv->stats, s->counter_ips_accepted); + /* A packet bypassed by the firewall isn't seen by IPS */ + if (!PacketBypassed(p)) + StatsCounterIncr(&tv->stats, s->counter_ips_accepted); } } else { if (unlikely(PacketCheckAction(p, ACTION_REJECT_ANY))) { diff --git a/src/decode.h b/src/decode.h index c7e8e31d9869..40f8f6ea368b 100644 --- a/src/decode.h +++ b/src/decode.h @@ -1321,7 +1321,10 @@ void DecodeUnregisterCounters(void); depth reached. */ #define PKT_STREAM_NOPCAPLOG BIT_U32(12) -// vacancy 2x +/** Packet was bypassed by a (firewall) rule */ +#define PKT_FW_BYPASSED BIT_U32(13) + +// vacancy /** Packet checksum is not computed (TX packet for example) */ #define PKT_IGNORE_CHECKSUM BIT_U32(15) diff --git a/src/detect-bypass.c b/src/detect-bypass.c index 61f93c05648c..b18e9d408487 100644 --- a/src/detect-bypass.c +++ b/src/detect-bypass.c @@ -64,7 +64,11 @@ void DetectBypassRegister(void) sigmatch_table[DETECT_BYPASS].Match = DetectBypassMatch; sigmatch_table[DETECT_BYPASS].Setup = DetectBypassSetup; sigmatch_table[DETECT_BYPASS].Free = NULL; - sigmatch_table[DETECT_BYPASS].flags = SIGMATCH_NOOPT | SIGMATCH_BAN_FIREWALL_MODE; + sigmatch_table[DETECT_BYPASS].flags = + SIGMATCH_NOOPT | SIGMATCH_SUPPORT_FIREWALL | SIGMATCH_BAN_TD_FIREWALL_MODE | + SIGMATCH_BAN_FIREWALL_SCOPE_PACKET | SIGMATCH_BAN_FIREWALL_SCOPE_TX | + SIGMATCH_BAN_FIREWALL_SCOPE_HOOK | SIGMATCH_BAN_ACTION_CONFIG | + SIGMATCH_BAN_ACTION_DROP | SIGMATCH_BAN_ACTION_REJECT; } static int DetectBypassSetup(DetectEngineCtx *de_ctx, Signature *s, const char *str) diff --git a/src/detect-engine-alert.c b/src/detect-engine-alert.c index 38d262c364ea..ef4efe65d464 100644 --- a/src/detect-engine-alert.c +++ b/src/detect-engine-alert.c @@ -600,6 +600,35 @@ static struct DetectFirewallPolicy HandleFirewallRule( return pol; } +/** + * \brief see if a firewall rule in the queue bypassed the flow + * + * `bypass` is applied from the postmatch list at match time. By the time the queue + * is processed the flow is already out of inspection. So, Threat detection must + * not be consulted for this packet. Especially when a TD rule sorts ahead + * of the firewall rule, as packet:td does relative to app:filter. + * + * Independent of the rule's action: `bypass` is a keyword, not an action, and is not tied to + * `accept` + */ +static inline bool AlertQueueHasFirewallBypass( + const DetectEngineThreadCtx *det_ctx, const Packet *p) +{ + if (p->flow == NULL || PKT_IS_PSEUDOPKT(p) || !FlowIsBypassed(p->flow)) + return false; + + for (uint16_t i = 0; i < det_ctx->alert_queue_size; i++) { + const Signature *s = det_ctx->alert_queue[i].s; + if ((s->flags & (SIG_FLAG_FIREWALL | SIG_FLAG_BYPASS)) == + (SIG_FLAG_FIREWALL | SIG_FLAG_BYPASS)) { + SCLogDebug("packet %" PRIu64 ": fw sid %u bypassed the flow, skipping td", + PcapPacketCntGet(p), s->id); + return true; + } + } + return false; +} + /* * Queue order after sorting: * @@ -650,6 +679,13 @@ static inline void PacketAlertFinalizeProcessQueue( #endif /* DEBUG */ uint8_t skip_table_id = 0; bool skip_table = false; + + if (AlertQueueHasFirewallBypass(det_ctx, p)) { + skip_td = true; + /* a bypass verdict won't be changed at this point */ + p->flags |= PKT_FW_BYPASSED; + } + for (uint16_t i = 0; i < det_ctx->alert_queue_size; i++) { PacketAlert *pa = &det_ctx->alert_queue[i]; const Signature *s = pa->s; @@ -707,9 +743,17 @@ static inline void PacketAlertFinalizeProcessQueue( /* if a firewall rule told us to skip, we don't count the skipped * alerts. */ if (have_fw_rules && skip_td) { + /* a bypass means this packet should never have reached TD, so its + * post-match state must not stand. Other skips (pass rules) only + * withhold the alert. */ + if (!(p->flags & PKT_FW_BYPASSED)) + DetectRunPostMatchDeferred(det_ctx, p, s); continue; } + /* run state-changing post-match for remaining TD rules */ + DetectRunPostMatchDeferred(det_ctx, p, s); + int res = PacketAlertHandle(de_ctx, det_ctx, s, p, pa); SCLogDebug("sid %u: res %d", pa->s->id, res); if (res > 0) { @@ -789,7 +833,9 @@ static inline void PacketAlertFinalizeProcessQueue( fw_dropped: /* after threat detection has been handled, see if the fw intended to accept (drop is handled - * immediately by the fw), as fw accept can be overruled by td drop. */ + * immediately by the fw), as fw accept can be overruled by td drop. + * (this is not the case if a flow is accepted _and_ bypassed with a firewall rule + * (accept:flow+bypass)) */ if (have_fw_rules) { if (p->action & ACTION_DROP) { SCLogDebug("packet %" PRIu64 ": dropped by TD", PcapPacketCntGet(p)); diff --git a/src/detect-engine-build.c b/src/detect-engine-build.c index add1f2f7f8ac..ae7a5a5d434b 100644 --- a/src/detect-engine-build.c +++ b/src/detect-engine-build.c @@ -37,6 +37,7 @@ #include "detect-flow.h" #include "detect-config.h" #include "detect-flowbits.h" +#include "detect-xbits.h" #include "app-layer-events.h" @@ -2212,6 +2213,31 @@ int SigPrepareStage4(DetectEngineCtx *de_ctx) SCReturnInt(0); } +/** \internal + * \brief post-match keywords whose state outlives the flow + * + * A threat detection rule can match a packet that a firewall rule then + * bypasses. The alert is dropped during alert finalization, but the post-match + * list has already run at match time. Flow-scoped entries are harmless -- the + * flow is leaving inspection -- but host, ip-pair and packet-mark state + * outlive it, so those are held back until the verdict is known. + */ +static bool PostMatchIsDeferred(const SigMatchData *smd) +{ + switch (smd->type) { + case DETECT_XBITS: { + const DetectXbitsData *xd = (const DetectXbitsData *)smd->ctx; + /* tx-scoped bits die with the bypassed flow */ + return xd->type != VAR_TYPE_TX_BIT; + } + case DETECT_HOSTBITS: + case DETECT_MARK: + return true; + default: + return false; + } +} + extern bool rule_engine_analysis_set; /** \internal * \brief perform final per signature setup tasks @@ -2239,6 +2265,17 @@ static int SigMatchPrepare(DetectEngineCtx *de_ctx) SigMatch *sm = s->init_data->smlists[type]; s->sm_arrays[type] = SigMatchList2DataArray(sm); } + /* firewall mode: hold back threat detect post-match state that + * outlives the flow, so a firewall bypass can suppress it. */ + if (EngineModeIsFirewall() && (s->flags & SIG_FLAG_FIREWALL) == 0) { + SigMatchData *smd = s->sm_arrays[DETECT_SM_LIST_POSTMATCH]; + while (smd != NULL) { + smd->deferred = PostMatchIsDeferred(smd); + if (smd->is_last) + break; + smd++; + } + } /* set up the pkt inspection engines */ DetectEnginePktInspectionSetup(s); diff --git a/src/detect-engine-register.c b/src/detect-engine-register.c index 845521acac1e..6c4fbe02daf6 100644 --- a/src/detect-engine-register.c +++ b/src/detect-engine-register.c @@ -343,6 +343,48 @@ static void PrintFeatureList(const SigTableElmt *e, char sep) printf("banned from firewall mode"); prev = 1; } + if (flags & SIGMATCH_BAN_TD_FIREWALL_MODE) { + if (prev == 1) + printf("%c", sep); + printf("banned from threat detection rules in firewall mode"); + prev = 1; + } + if (flags & SIGMATCH_BAN_FIREWALL_SCOPE_PACKET) { + if (prev == 1) + printf("%c", sep); + printf("banned from firewall rules with \'packet\' scope"); + prev = 1; + } + if (flags & SIGMATCH_BAN_FIREWALL_SCOPE_TX) { + if (prev == 1) + printf("%c", sep); + printf("banned from firewall rules with \'tx\' scope"); + prev = 1; + } + if (flags & SIGMATCH_BAN_FIREWALL_SCOPE_HOOK) { + if (prev == 1) + printf("%c", sep); + printf("banned from firewall rules with \'hook\' scope"); + prev = 1; + } + if (flags & SIGMATCH_BAN_ACTION_CONFIG) { + if (prev == 1) + printf("%c", sep); + printf("banned from firewall rules with \'config\' action"); + prev = 1; + } + if (flags & SIGMATCH_BAN_ACTION_DROP) { + if (prev == 1) + printf("%c", sep); + printf("banned from firewall rules with \'drop\' action"); + prev = 1; + } + if (flags & SIGMATCH_BAN_ACTION_REJECT) { + if (prev == 1) + printf("%c", sep); + printf("banned from firewall rules with \'reject\' action"); + prev = 1; + } if (e->Transform) { if (prev == 1) printf("%c", sep); diff --git a/src/detect-engine-register.h b/src/detect-engine-register.h index 221b25ef6997..d24c952534a1 100644 --- a/src/detect-engine-register.h +++ b/src/detect-engine-register.h @@ -356,6 +356,16 @@ extern int DETECT_TBLSIZE_IDX; #define SIGMATCH_BAN_FIREWALL_RULE (1UL << (21)) /** keyword cannot be used in firewall mode */ #define SIGMATCH_BAN_FIREWALL_MODE (1UL << (22)) +/** keyword cannot be used in td rules with firewall mode */ +#define SIGMATCH_BAN_TD_FIREWALL_MODE (1UL << (23)) +/** keyword cannot be used in combination with indicated action scope */ +#define SIGMATCH_BAN_FIREWALL_SCOPE_PACKET (1UL << (24)) +#define SIGMATCH_BAN_FIREWALL_SCOPE_TX (1UL << (25)) +#define SIGMATCH_BAN_FIREWALL_SCOPE_HOOK (1UL << (26)) +/** keyword cannot be used in combination with indicated action */ +#define SIGMATCH_BAN_ACTION_CONFIG (1UL << (27)) +#define SIGMATCH_BAN_ACTION_DROP (1UL << (28)) +#define SIGMATCH_BAN_ACTION_REJECT (1UL << (29)) int SigTableList(const char *keyword); void SigTableCleanup(void); diff --git a/src/detect-parse.c b/src/detect-parse.c index 44d546984c19..783e670631d0 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -901,6 +901,59 @@ static int DetectSetupDirection(Signature *s, char **str, bool only_dir) return 0; } +/** + * \brief called only with firewall rules, to validate options + * + * It is valid to call this before the keyword's value, if any, has been parsed. + * + * \retval false if the keyword is not allowed for this rule's action and/or action scope + */ +static bool SigParseFirewallRuleAllowed( + uint8_t action, uint8_t action_scope, uint32_t sig_flags, const char *optname) +{ + if ((sig_flags & SIGMATCH_BAN_FIREWALL_RULE) != 0) { + SCLogError("keyword \'%s\' is not allowed with firewall rules", optname); + return false; + } + if ((sig_flags & SIGMATCH_BAN_FIREWALL_MODE) != 0) { + SCLogError("keyword \'%s\' is not allowed in firewall mode", optname); + return false; + } + if ((action & ACTION_CONFIG) != 0 && (sig_flags & SIGMATCH_BAN_ACTION_CONFIG) != 0) { + SCLogError("keyword \'%s\' cannot be used in combination with \'config\' action", optname); + return false; + } + if ((action & ACTION_REJECT_ANY) != 0 && (sig_flags & SIGMATCH_BAN_ACTION_REJECT) != 0) { + SCLogError("keyword \'%s\' cannot be used in combination with \'reject\' action", optname); + return false; + } + if ((action & ACTION_DROP) != 0 && (sig_flags & SIGMATCH_BAN_ACTION_DROP) != 0) { + SCLogError("keyword \'%s\' cannot be used in combination with \'drop\' action", optname); + return false; + } + if (action_scope == (uint8_t)ACTION_SCOPE_PACKET) { + if ((sig_flags & SIGMATCH_BAN_FIREWALL_SCOPE_PACKET) != 0) { + SCLogError( + "keyword \'%s\' cannot be used in combination with \'packet\' scope", optname); + return false; + } + } + if (action_scope == (uint8_t)ACTION_SCOPE_TX) { + if ((sig_flags & SIGMATCH_BAN_FIREWALL_SCOPE_TX) != 0) { + SCLogError("keyword \'%s\' cannot be used in combination with \'tx\' scope", optname); + return false; + } + } + if (action_scope == (uint8_t)ACTION_SCOPE_HOOK) { + if ((sig_flags & SIGMATCH_BAN_FIREWALL_SCOPE_HOOK) != 0) { + SCLogError("keyword \'%s\' cannot be used in combination with \'hook\' scope", optname); + return false; + } + } + /* for most cases, firewall rules should be allowed */ + return true; +} + static int SigParseOptions(DetectEngineCtx *de_ctx, Signature *s, char *optstr, char *output, size_t output_size, bool requires) { @@ -1001,16 +1054,24 @@ static int SigParseOptions(DetectEngineCtx *de_ctx, Signature *s, char *optstr, #undef URL } - if (s->init_data->firewall_rule && (st->flags & SIGMATCH_BAN_FIREWALL_RULE) != 0) { - SCLogError("keyword \'%s\' is not allowed with firewall rules", optname); + if (EngineModeIsFirewall() && s->init_data->firewall_rule && + !SigParseFirewallRuleAllowed(s->action, s->action_scope, st->flags, optname)) { goto error; } - if (EngineModeIsFirewall() && (st->flags & SIGMATCH_BAN_FIREWALL_MODE) != 0) { + /* For non-firewall rules */ + if (EngineModeIsFirewall() && !s->init_data->firewall_rule && + (st->flags & SIGMATCH_BAN_FIREWALL_MODE) != 0) { SCLogError("keyword \'%s\' is not allowed in firewall mode", optname); goto error; } + if (EngineModeIsFirewall() && !s->init_data->firewall_rule && + (st->flags & SIGMATCH_BAN_TD_FIREWALL_MODE) != 0) { + SCLogError("keyword \'%s\' is not allowed in threat detection rules with firewall mode", + optname); + goto error; + } int setup_ret = 0; /* Validate double quoting, trimming trailing white space along the way. */ @@ -2758,6 +2819,7 @@ static bool DetectRuleValidateTable(const Signature *s) return true; } +/** \brief validates firewall rule action scope */ static bool DetectFirewallRuleValidate(const DetectEngineCtx *de_ctx, const Signature *s) { if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_NOT_SET) { diff --git a/src/detect-threshold.c b/src/detect-threshold.c index 1ac1e6390254..c86077de3e13 100644 --- a/src/detect-threshold.c +++ b/src/detect-threshold.c @@ -94,8 +94,8 @@ void DetectThresholdRegister(void) #ifdef UNITTESTS sigmatch_table[DETECT_THRESHOLD].RegisterTests = ThresholdRegisterTests; #endif - /* this is compatible to ip-only signatures */ - sigmatch_table[DETECT_THRESHOLD].flags |= SIGMATCH_IPONLY_COMPAT; + /* this is compatible to ip-only signatures, and not compatible with firewall rules */ + sigmatch_table[DETECT_THRESHOLD].flags = SIGMATCH_IPONLY_COMPAT | SIGMATCH_BAN_FIREWALL_RULE; DetectSetupParseRegexes(PARSE_REGEX, &parse_regex); } diff --git a/src/detect.c b/src/detect.c index 424d47495049..0e84ae20474b 100644 --- a/src/detect.c +++ b/src/detect.c @@ -272,9 +272,11 @@ static void DetectRunPostMatch(ThreadVars *tv, SCLogDebug("running match functions, sm %p", smd); while (1) { - KEYWORD_PROFILING_START; - (void)sigmatch_table[smd->type].Match(det_ctx, p, s, smd->ctx); - KEYWORD_PROFILING_END(det_ctx, smd->type, 1); + if (!smd->deferred) { + KEYWORD_PROFILING_START; + (void)sigmatch_table[smd->type].Match(det_ctx, p, s, smd->ctx); + KEYWORD_PROFILING_END(det_ctx, smd->type, 1); + } if (smd->is_last) break; smd++; @@ -282,6 +284,31 @@ static void DetectRunPostMatch(ThreadVars *tv, } } +/** + * \brief Run the post-match entries held back for the firewall verdict + * + * Counterpart to the entries DetectRunPostMatch skips. Called from alert queue + * finalization, once we know whether the packet was bypassed. + */ +void DetectRunPostMatchDeferred(DetectEngineThreadCtx *det_ctx, Packet *p, const Signature *s) +{ + const SigMatchData *smd = s->sm_arrays[DETECT_SM_LIST_POSTMATCH]; + if (smd == NULL) + return; + + KEYWORD_PROFILING_SET_LIST(det_ctx, DETECT_SM_LIST_POSTMATCH); + while (1) { + if (smd->deferred) { + KEYWORD_PROFILING_START; + (void)sigmatch_table[smd->type].Match(det_ctx, p, s, smd->ctx); + KEYWORD_PROFILING_END(det_ctx, smd->type, 1); + } + if (smd->is_last) + break; + smd++; + } +} + /** * \brief Get the SigGroupHead for a packet. * diff --git a/src/detect.h b/src/detect.h index 030a6a0e0563..f18d2cd7424b 100644 --- a/src/detect.h +++ b/src/detect.h @@ -368,6 +368,8 @@ typedef struct SigMatch_ { typedef struct SigMatchData_ { uint16_t type; /**< match type */ bool is_last; /**< Last element of the list */ + bool deferred; /**< post-match entry withheld until firewall verdict is known. + * Only ever set for TD rules in FW mode */ SigMatchCtx *ctx; /**< plugin specific data */ } SigMatchData; @@ -1777,6 +1779,7 @@ extern SigTableElmt *sigmatch_table; TmEcode Detect(ThreadVars *tv, Packet *p, void *data); uint8_t DetectPreFlow(ThreadVars *tv, DetectEngineThreadCtx *det_ctx, Packet *p); uint8_t DetectPreStream(ThreadVars *tv, DetectEngineThreadCtx *det_ctx, Packet *p); +void DetectRunPostMatchDeferred(DetectEngineThreadCtx *det_ctx, Packet *p, const Signature *s); SigMatch *SigMatchAlloc(void); Signature *SigFindSignatureBySidGid(DetectEngineCtx *, uint32_t, uint32_t); diff --git a/src/util-threshold-config.c b/src/util-threshold-config.c index deb314edeecf..806274e9684d 100644 --- a/src/util-threshold-config.c +++ b/src/util-threshold-config.c @@ -258,8 +258,16 @@ static int SetupSuppressRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid, SCLogWarning("suppressing all rules"); } + uint32_t fw_rules_count = 0; /* update each sig with our suppress info */ for (s = de_ctx->sig_list; s != NULL; s = s->next) { + /* firewall rules are banned from threshold, for now. Cf #8943 */ + if (s->flags & SIG_FLAG_FIREWALL) { + SCLogDebug("Ignoring rule sid %" PRIu32 ": firewall rules can't be thresholded", + s->id); + fw_rules_count++; + continue; + } /* tag the rule as noalert */ if (parsed_track == TRACK_RULE) { s->action &= ~ACTION_ALERT; @@ -275,15 +283,29 @@ static int SetupSuppressRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid, goto error; } } + if (fw_rules_count > 0) { + SCLogWarning("Ignoring %u firewall rules: firewall rules can't be thresholded", + fw_rules_count); + } + } else if (id == 0 && gid > 0) { if (parsed_track == TRACK_RULE) { SCLogWarning("suppressing all rules with gid %" PRIu32, gid); } + + uint32_t fw_rules_count = 0; /* set up suppression for each signature with a matching gid */ for (s = de_ctx->sig_list; s != NULL; s = s->next) { if (s->gid != gid) continue; + /* firewall rules are banned from threshold, for now. Cf #8943 */ + if (s->flags & SIG_FLAG_FIREWALL) { + SCLogDebug("Ignoring rule sid %" PRIu32 ": firewall rules can't be thresholded", + s->id); + fw_rules_count++; + continue; + } /* tag the rule as noalert */ if (parsed_track == TRACK_RULE) { s->action &= ~ACTION_ALERT; @@ -299,6 +321,11 @@ static int SetupSuppressRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid, goto error; } } + if (fw_rules_count > 0) { + SCLogWarning("Ignoring %u firewall rules: firewall rules can't be thresholded", + fw_rules_count); + } + } else if (id > 0 && gid == 0) { SCLogError("Can't use a event config that has " "sid > 0 and gid == 0. Please fix this " @@ -311,6 +338,13 @@ static int SetupSuppressRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid, "%" PRIu32 ", gid %" PRIu32 ": unknown rule", id, gid); } else { + /* firewall rules are banned from threshold, for now. Cf #8943 */ + if (s->flags & SIG_FLAG_FIREWALL) { + FatalErrorOnInit("can't suppress sid " + "%" PRIu32 ", gid %" PRIu32 ": firewall rule", + s->id, gid); + goto error; + } if (parsed_track == TRACK_RULE) { s->action &= ~ACTION_ALERT; goto end; @@ -362,7 +396,16 @@ static int SetupThresholdRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid /* Install it */ if (id == 0 && gid == 0) { + uint32_t fw_rules_count = 0; for (s = de_ctx->sig_list; s != NULL; s = s->next) { + /* firewall rules are banned from threshold, for now. Cf #8943 */ + if (s->flags & SIG_FLAG_FIREWALL) { + SCLogDebug("Ignoring rule sid %" PRIu32 ": firewall rules can't be thresholded", + s->id); + fw_rules_count++; + continue; + } + sm = DetectGetLastSMByListId(s, DETECT_SM_LIST_THRESHOLD, DETECT_THRESHOLD, -1); if (sm != NULL) { SCLogWarning("signature sid:%" PRIu32 " has " @@ -404,10 +447,21 @@ static int SetupThresholdRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid goto error; } } - + if (fw_rules_count > 0) { + SCLogWarning("Ignoring %u firewall rules: firewall rules can't be thresholded", + fw_rules_count); + } } else if (id == 0 && gid > 0) { + uint32_t fw_rules_count = 0; for (s = de_ctx->sig_list; s != NULL; s = s->next) { if (s->gid == gid) { + /* firewall rules are banned from threshold, for now. Cf #8943 */ + if (s->flags & SIG_FLAG_FIREWALL) { + SCLogDebug("Ignoring rule sid %" PRIu32 ": firewall rules can't be thresholded", + s->id); + fw_rules_count++; + continue; + } sm = DetectGetLastSMByListId(s, DETECT_SM_LIST_THRESHOLD, DETECT_THRESHOLD, DETECT_DETECTION_FILTER, -1); if (sm != NULL) { @@ -440,6 +494,10 @@ static int SetupThresholdRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid } } } + if (fw_rules_count > 0) { + SCLogWarning("Ignoring %u firewall rules: firewall rules can't be thresholded", + fw_rules_count); + } } else if (id > 0 && gid == 0) { SCLogError("Can't use a event config that has " "sid > 0 and gid == 0. Please fix this " @@ -451,6 +509,13 @@ static int SetupThresholdRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid "%" PRIu32 ", gid %" PRIu32 ": unknown rule", id, gid); } else { + /* firewall rules are banned from threshold, for now. Cf #8943 */ + if (s->flags & SIG_FLAG_FIREWALL) { + FatalErrorOnInit("can't threshold sid " + "%" PRIu32 ", gid %" PRIu32 ": firewall rule", + s->id, gid); + goto error; + } if (parsed_type != TYPE_SUPPRESS && parsed_type != TYPE_THRESHOLD && parsed_type != TYPE_BOTH && parsed_type != TYPE_LIMIT) {