From 6a32139a65295aeffdd9bbd31bd2377f2d3a9e88 Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Tue, 6 Dec 2022 18:29:34 -0300 Subject: [PATCH 01/16] exceptions: add master switch config option This allows all traffic Exception Policies to be set from one configuration point. All exception policy options are available in IPS mode. Bypass, pass and auto (disabled) are also available in iDS mode Exception Policies set up individually will overwrite this setup for the given traffic exception. Task #5219 (cherry picked from commit 0d9289014bd8f65c7100e7173f24f5c5ff9de0ac) --- .../configuration/exception-policies.rst | 39 +++++ src/app-layer-parser.c | 2 +- src/stream-tcp.c | 8 +- src/suricata.c | 2 + src/util-exception-policy.c | 155 ++++++++++++------ src/util-exception-policy.h | 4 +- suricata.yaml.in | 12 ++ 7 files changed, 169 insertions(+), 53 deletions(-) diff --git a/doc/userguide/configuration/exception-policies.rst b/doc/userguide/configuration/exception-policies.rst index 2d394d0df9cd..201a7a3ecb33 100644 --- a/doc/userguide/configuration/exception-policies.rst +++ b/doc/userguide/configuration/exception-policies.rst @@ -16,6 +16,45 @@ simulate failures or errors and understand Suricata behavior under such conditio Exception Policies ------------------ +.. _master-switch: + +Master Switch +~~~~~~~~~~~~~ + +It is possible to set all configuration policies via what we call "master +switch". This offers a quick way to define what the engine should do in case of +traffic exceptions, while still allowing for the flexibility of indicating a +different behavior for specific exception policies your setup/environment may +have the need to. + +:: + + # In IPS mode, the default is drop-packet/drop-flow. To fallback to old + # behavior (setting each of them individually, or ignoring all), set this + # to ignore. + # All values available for exception policies can be used, and there is one + # extra option: auto - which means drop-packet/drop-flow in IPS mode and + # ignore in IDS mode). + # Exception policy values are: drop-packet, drop-flow, reject, bypass, + # pass-packet, pass-flow, ignore (disable). + exception-policy: auto + +This value will be overwritten by specific exception policies whose settings are +also defined in the yaml file. + +Auto +'''' + +**In IPS mode**, the default behavior for all exception policies is to drop +packets and/or flows. It is possible to disable this default, by setting the +exception policies "master switch" yaml config option to ``ignore``. + +**In IDS mode**, setting auto mode actually means disabling the +``master-swtich``, or ignoring the exception policies. + +Specific settings +~~~~~~~~~~~~~~~~~ + Exception policies are implemented for: .. list-table:: Exception Policy configuration variables diff --git a/src/app-layer-parser.c b/src/app-layer-parser.c index 473ce277d3a4..34594c97808d 100644 --- a/src/app-layer-parser.c +++ b/src/app-layer-parser.c @@ -169,7 +169,7 @@ struct AppLayerParserState_ { AppLayerDecoderEvents *decoder_events; }; -enum ExceptionPolicy g_applayerparser_error_policy = EXCEPTION_POLICY_IGNORE; +enum ExceptionPolicy g_applayerparser_error_policy = EXCEPTION_POLICY_NOT_SET; static void AppLayerConfg(void) { diff --git a/src/stream-tcp.c b/src/stream-tcp.c index dc6d8f1e20cb..d736edef2a09 100644 --- a/src/stream-tcp.c +++ b/src/stream-tcp.c @@ -481,11 +481,11 @@ void StreamTcpInitConfig(char quiet) SCLogConfig("memcap-policy: %u/%u", stream_config.ssn_memcap_policy, stream_config.reassembly_memcap_policy); stream_config.midstream_policy = ExceptionPolicyParse("stream.midstream-policy", true); - if (stream_config.midstream && stream_config.midstream_policy != EXCEPTION_POLICY_IGNORE) { + if (stream_config.midstream && stream_config.midstream_policy != EXCEPTION_POLICY_NOT_SET) { SCLogWarning(SC_WARN_COMPATIBILITY, "stream.midstream_policy setting conflicting with stream.midstream enabled. " "Ignoring stream.midstream_policy."); - stream_config.midstream_policy = EXCEPTION_POLICY_IGNORE; + stream_config.midstream_policy = EXCEPTION_POLICY_NOT_SET; } if (!quiet) { @@ -961,7 +961,7 @@ static int StreamTcpPacketStateNone(ThreadVars *tv, Packet *p, SCLogDebug("Midstream not enabled, so won't pick up a session"); return 0; } - if (!(stream_config.midstream_policy == EXCEPTION_POLICY_IGNORE || + if (!(stream_config.midstream_policy == EXCEPTION_POLICY_NOT_SET || stream_config.midstream_policy == EXCEPTION_POLICY_PASS_FLOW || stream_config.midstream_policy == EXCEPTION_POLICY_PASS_PACKET)) { SCLogDebug("Midstream policy not permissive, so won't pick up a session"); @@ -1132,7 +1132,7 @@ static int StreamTcpPacketStateNone(ThreadVars *tv, Packet *p, SCLogDebug("Midstream not enabled, so won't pick up a session"); return 0; } - if (!(stream_config.midstream_policy == EXCEPTION_POLICY_IGNORE || + if (!(stream_config.midstream_policy == EXCEPTION_POLICY_NOT_SET || stream_config.midstream_policy == EXCEPTION_POLICY_PASS_FLOW || stream_config.midstream_policy == EXCEPTION_POLICY_PASS_PACKET)) { SCLogDebug("Midstream policy not permissive, so won't pick up a session"); diff --git a/src/suricata.c b/src/suricata.c index fac1ccdda888..6ebf24ba293f 100644 --- a/src/suricata.c +++ b/src/suricata.c @@ -2607,6 +2607,8 @@ int PostConfLoadedSetup(SCInstance *suri) EngineModeSetIDS(); } + SetMasterExceptionPolicy(); + AppLayerSetup(); /* Suricata will use this umask if provided. By default it will use the diff --git a/src/util-exception-policy.c b/src/util-exception-policy.c index a1ead767787d..65fd5f4cec59 100644 --- a/src/util-exception-policy.c +++ b/src/util-exception-policy.c @@ -24,49 +24,80 @@ #include "util-misc.h" #include "stream-tcp-reassemble.h" +enum ExceptionPolicy g_eps_master_switch = EXCEPTION_POLICY_NOT_SET; + +static const char *ExceptionPolicyEnumToString(enum ExceptionPolicy policy) +{ + switch (policy) { + case EXCEPTION_POLICY_NOT_SET: + return "ignore"; + case EXCEPTION_POLICY_REJECT: + return "reject"; + case EXCEPTION_POLICY_BYPASS_FLOW: + return "bypass"; + case EXCEPTION_POLICY_DROP_FLOW: + return "drop-flow"; + case EXCEPTION_POLICY_DROP_PACKET: + return "drop-packet"; + case EXCEPTION_POLICY_PASS_PACKET: + return "pass-packet"; + case EXCEPTION_POLICY_PASS_FLOW: + return "pass-flow"; + } + // TODO we shouldn't reach this, but if we do, better not to leave this as simply null... + return "not set"; +} + +void SetMasterExceptionPolicy(void) +{ + g_eps_master_switch = ExceptionPolicyParse("exception-policy", true); +} + +static enum ExceptionPolicy GetMasterExceptionPolicy(const char *option) +{ + return g_eps_master_switch; +} + void ExceptionPolicyApply(Packet *p, enum ExceptionPolicy policy, enum PacketDropReason drop_reason) { SCLogDebug("start: pcap_cnt %" PRIu64 ", policy %u", p->pcap_cnt, policy); - if (EngineModeIsIPS()) { - switch (policy) { - case EXCEPTION_POLICY_IGNORE: - break; - case EXCEPTION_POLICY_REJECT: - SCLogDebug("EXCEPTION_POLICY_REJECT"); - PacketDrop(p, ACTION_REJECT, drop_reason); - /* fall through */ - case EXCEPTION_POLICY_DROP_FLOW: - SCLogDebug("EXCEPTION_POLICY_DROP_FLOW"); - if (p->flow) { - p->flow->flags |= FLOW_ACTION_DROP; - FlowSetNoPayloadInspectionFlag(p->flow); - FlowSetNoPacketInspectionFlag(p->flow); - StreamTcpDisableAppLayer(p->flow); - } - /* fall through */ - case EXCEPTION_POLICY_DROP_PACKET: - SCLogDebug("EXCEPTION_POLICY_DROP_PACKET"); - DecodeSetNoPayloadInspectionFlag(p); - DecodeSetNoPacketInspectionFlag(p); - PacketDrop(p, ACTION_DROP, drop_reason); - break; - case EXCEPTION_POLICY_BYPASS_FLOW: - PacketBypassCallback(p); - /* fall through */ - case EXCEPTION_POLICY_PASS_FLOW: - SCLogDebug("EXCEPTION_POLICY_PASS_FLOW"); - if (p->flow) { - p->flow->flags |= FLOW_ACTION_PASS; - FlowSetNoPacketInspectionFlag(p->flow); // TODO util func - } - /* fall through */ - case EXCEPTION_POLICY_PASS_PACKET: - SCLogDebug("EXCEPTION_POLICY_PASS_PACKET"); - DecodeSetNoPayloadInspectionFlag(p); - DecodeSetNoPacketInspectionFlag(p); - PacketSetActionOnCurrentPkt(p, ACTION_PASS); - break; - } + switch (policy) { + case EXCEPTION_POLICY_NOT_SET: + break; + case EXCEPTION_POLICY_REJECT: + SCLogDebug("EXCEPTION_POLICY_REJECT"); + PacketDrop(p, ACTION_REJECT, drop_reason); + /* fall through */ + case EXCEPTION_POLICY_DROP_FLOW: + SCLogDebug("EXCEPTION_POLICY_DROP_FLOW"); + if (p->flow) { + p->flow->flags |= FLOW_ACTION_DROP; + FlowSetNoPayloadInspectionFlag(p->flow); + FlowSetNoPacketInspectionFlag(p->flow); + StreamTcpDisableAppLayer(p->flow); + } + /* fall through */ + case EXCEPTION_POLICY_DROP_PACKET: + SCLogDebug("EXCEPTION_POLICY_DROP_PACKET"); + DecodeSetNoPayloadInspectionFlag(p); + DecodeSetNoPacketInspectionFlag(p); + PacketDrop(p, ACTION_DROP, drop_reason); + break; + case EXCEPTION_POLICY_BYPASS_FLOW: + PacketBypassCallback(p); + /* fall through */ + case EXCEPTION_POLICY_PASS_FLOW: + SCLogDebug("EXCEPTION_POLICY_PASS_FLOW"); + if (p->flow) { + p->flow->flags |= FLOW_ACTION_PASS; + FlowSetNoPacketInspectionFlag(p->flow); // TODO util func + } + /* fall through */ + case EXCEPTION_POLICY_PASS_PACKET: + SCLogDebug("EXCEPTION_POLICY_PASS_PACKET"); + DecodeSetNoPayloadInspectionFlag(p); + DecodeSetNoPacketInspectionFlag(p); + break; } SCLogDebug("end"); } @@ -85,7 +116,7 @@ static enum ExceptionPolicy PickPacketAction(const char *option, enum ExceptionP case EXCEPTION_POLICY_BYPASS_FLOW: SCLogWarning(SC_ERR_INVALID_VALUE, "flow actions not supported for %s, defaulting to \"ignore\"", option); - return EXCEPTION_POLICY_IGNORE; + return EXCEPTION_POLICY_NOT_SET; /* add all cases, to make sure new cases not handle will raise * errors */ case EXCEPTION_POLICY_DROP_PACKET: @@ -94,19 +125,29 @@ static enum ExceptionPolicy PickPacketAction(const char *option, enum ExceptionP break; case EXCEPTION_POLICY_REJECT: break; - case EXCEPTION_POLICY_IGNORE: + case EXCEPTION_POLICY_NOT_SET: break; } return p; } +static enum ExceptionPolicy SetIPSOption( + const char *option, const char *value_str, enum ExceptionPolicy p) +{ + if (!EngineModeIsIPS()) { + SCLogConfig("%s: %s not a valid config in IDS mode. Ignoring it.)", option, value_str); + return EXCEPTION_POLICY_NOT_SET; + } + return p; +} + enum ExceptionPolicy ExceptionPolicyParse(const char *option, const bool support_flow) { - enum ExceptionPolicy policy = EXCEPTION_POLICY_IGNORE; + enum ExceptionPolicy policy = EXCEPTION_POLICY_NOT_SET; const char *value_str = NULL; if ((ConfGet(option, &value_str)) == 1 && value_str != NULL) { if (strcmp(value_str, "drop-flow") == 0) { - policy = EXCEPTION_POLICY_DROP_FLOW; + policy = SetIPSOption(option, value_str, EXCEPTION_POLICY_DROP_FLOW); SCLogConfig("%s: %s", option, value_str); } else if (strcmp(value_str, "pass-flow") == 0) { policy = EXCEPTION_POLICY_PASS_FLOW; @@ -115,7 +156,7 @@ enum ExceptionPolicy ExceptionPolicyParse(const char *option, const bool support policy = EXCEPTION_POLICY_BYPASS_FLOW; SCLogConfig("%s: %s", option, value_str); } else if (strcmp(value_str, "drop-packet") == 0) { - policy = EXCEPTION_POLICY_DROP_PACKET; + policy = SetIPSOption(option, value_str, EXCEPTION_POLICY_DROP_PACKET); SCLogConfig("%s: %s", option, value_str); } else if (strcmp(value_str, "pass-packet") == 0) { policy = EXCEPTION_POLICY_PASS_PACKET; @@ -124,7 +165,10 @@ enum ExceptionPolicy ExceptionPolicyParse(const char *option, const bool support policy = EXCEPTION_POLICY_REJECT; SCLogConfig("%s: %s", option, value_str); } else if (strcmp(value_str, "ignore") == 0) { // TODO name? - policy = EXCEPTION_POLICY_IGNORE; + policy = EXCEPTION_POLICY_NOT_SET; + SCLogConfig("%s: %s", option, value_str); + } else if (strcmp(value_str, "auto") == 0) { + policy = SetIPSOption(option, value_str, EXCEPTION_POLICY_DROP_FLOW); SCLogConfig("%s: %s", option, value_str); } else { FatalErrorOnInit(SC_ERR_INVALID_ARGUMENT, @@ -137,8 +181,25 @@ enum ExceptionPolicy ExceptionPolicyParse(const char *option, const bool support policy = PickPacketAction(option, policy); } + } else if (strcmp(option, "exception-policy") == 0) { + /* not enabled, we won't change the master exception policy, + for now */ + SCLogInfo("'exception-policy' master switch not set, so ignoring it." + " This behavior will change in Suricata 8, so please update your" + " config. See ticket #5219 for more details."); + g_eps_master_switch = EXCEPTION_POLICY_NOT_SET; } else { - SCLogConfig("%s: ignore", option); + /* Exception Policy was not defined individually */ + enum ExceptionPolicy master_policy = GetMasterExceptionPolicy(option); + if (master_policy == EXCEPTION_POLICY_NOT_SET) { + SCLogConfig("%s: ignore", option); + } else { + /* If the master switch was set and the Exception Policy option was not + individually set, use the defined master Exception Policy */ + const char *value = ExceptionPolicyEnumToString(master_policy); + SCLogConfig("%s: %s (defined via 'exception-policy' master switch", option, value); + policy = master_policy; + } } return policy; } diff --git a/src/util-exception-policy.h b/src/util-exception-policy.h index 0a3b78d9f7e6..96e22c67503c 100644 --- a/src/util-exception-policy.h +++ b/src/util-exception-policy.h @@ -23,7 +23,7 @@ #define __UTIL_EXCEPTION_POLICY_H__ enum ExceptionPolicy { - EXCEPTION_POLICY_IGNORE = 0, + EXCEPTION_POLICY_NOT_SET = 0, EXCEPTION_POLICY_PASS_PACKET, EXCEPTION_POLICY_PASS_FLOW, EXCEPTION_POLICY_BYPASS_FLOW, @@ -32,10 +32,12 @@ enum ExceptionPolicy { EXCEPTION_POLICY_REJECT, }; +void SetMasterExceptionPolicy(void); void ExceptionPolicyApply( Packet *p, enum ExceptionPolicy policy, enum PacketDropReason drop_reason); enum ExceptionPolicy ExceptionPolicyParse(const char *option, const bool support_flow); +extern enum ExceptionPolicy g_eps_master_switch; #ifdef DEBUG extern uint64_t g_eps_applayer_error_offset_ts; extern uint64_t g_eps_applayer_error_offset_tc; diff --git a/suricata.yaml.in b/suricata.yaml.in index 081378fb8dcc..eb985c260e93 100644 --- a/suricata.yaml.in +++ b/suricata.yaml.in @@ -1129,6 +1129,18 @@ legacy: # packet. Default is 15 #packet-alert-max: 15 +# Exception Policies +# +# Define a common behavior for all exception policies. +# Default is ignore. +# All values available for exception policies can be used, and there is one +# extra option: auto - which means ignore (in Suricata 7.0 this changes to drop +# in IPS mode). +# +# Exception policy values are: drop-packet, drop-flow, reject, bypass, +# pass-packet, pass-flow, auto or ignore (disable). +exception-policy: auto + # IP Reputation #reputation-categories-file: @e_sysconfdir@iprep/categories.txt #default-reputation-path: @e_sysconfdir@iprep From c3b97b4d08ea8f539a3dc3efd83e06013c25c89f Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Mon, 5 Jun 2023 13:02:26 -0300 Subject: [PATCH 02/16] exception: in ids mode, only REJECT the packet In case of 'EXCEPTION_POLICY_REJECT', we were applying the same behavior regardless of being in IDS or IPS mode. This meant that (at least) the 'flow.action' was changed to drop when we hit an exception policy in IDS mode. Bug #6109 (cherry picked from commit 8f324e3b3d4137b1092b877e8f0dab42e7c824fd) --- src/util-exception-policy.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util-exception-policy.c b/src/util-exception-policy.c index 65fd5f4cec59..d3a50fb4cceb 100644 --- a/src/util-exception-policy.c +++ b/src/util-exception-policy.c @@ -67,6 +67,9 @@ void ExceptionPolicyApply(Packet *p, enum ExceptionPolicy policy, enum PacketDro case EXCEPTION_POLICY_REJECT: SCLogDebug("EXCEPTION_POLICY_REJECT"); PacketDrop(p, ACTION_REJECT, drop_reason); + if (!EngineModeIsIPS()) { + break; + } /* fall through */ case EXCEPTION_POLICY_DROP_FLOW: SCLogDebug("EXCEPTION_POLICY_DROP_FLOW"); From 5e674aaaa7cdf59baa7b36ee0198383b92500b02 Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Thu, 30 Mar 2023 10:40:46 -0300 Subject: [PATCH 03/16] defrag: clean up existing stats counters 7a044a99ee14101fbc removed the lines that incremented these defrag counters, but kept the entities themselves. This commit removes counters that we judge too complex to maintain, given the current state of the code, and re-adds incrementing max_hit (memcap related). Related to Task #5816 (cherry picked from commit a37a88dcd5950344fc0b4529f1731c3dab9f0888) --- src/decode.c | 10 ++-------- src/decode.h | 2 -- src/defrag.c | 6 +++++- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/decode.c b/src/decode.c index 574f91451a0f..8d0e3feb2e03 100644 --- a/src/decode.c +++ b/src/decode.c @@ -584,16 +584,10 @@ void DecodeRegisterPerfCounters(DecodeThreadVars *dtv, ThreadVars *tv) dtv->counter_defrag_ipv4_fragments = StatsRegisterCounter("defrag.ipv4.fragments", tv); - dtv->counter_defrag_ipv4_reassembled = - StatsRegisterCounter("defrag.ipv4.reassembled", tv); - dtv->counter_defrag_ipv4_timeouts = - StatsRegisterCounter("defrag.ipv4.timeouts", tv); + dtv->counter_defrag_ipv4_reassembled = StatsRegisterCounter("defrag.ipv4.reassembled", tv); dtv->counter_defrag_ipv6_fragments = StatsRegisterCounter("defrag.ipv6.fragments", tv); - dtv->counter_defrag_ipv6_reassembled = - StatsRegisterCounter("defrag.ipv6.reassembled", tv); - dtv->counter_defrag_ipv6_timeouts = - StatsRegisterCounter("defrag.ipv6.timeouts", tv); + dtv->counter_defrag_ipv6_reassembled = StatsRegisterCounter("defrag.ipv6.reassembled", tv); dtv->counter_defrag_max_hit = StatsRegisterCounter("defrag.max_frag_hits", tv); diff --git a/src/decode.h b/src/decode.h index e4fed6f6464b..33655a584214 100644 --- a/src/decode.h +++ b/src/decode.h @@ -706,10 +706,8 @@ typedef struct DecodeThreadVars_ /** frag stats - defrag runs in the context of the decoder. */ uint16_t counter_defrag_ipv4_fragments; uint16_t counter_defrag_ipv4_reassembled; - uint16_t counter_defrag_ipv4_timeouts; uint16_t counter_defrag_ipv6_fragments; uint16_t counter_defrag_ipv6_reassembled; - uint16_t counter_defrag_ipv6_timeouts; uint16_t counter_defrag_max_hit; uint16_t counter_flow_memcap; diff --git a/src/defrag.c b/src/defrag.c index 010bb02121e9..e02cd9c073a0 100644 --- a/src/defrag.c +++ b/src/defrag.c @@ -1041,8 +1041,12 @@ Defrag(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p) /* return a locked tracker or NULL */ tracker = DefragGetTracker(tv, dtv, p); - if (tracker == NULL) + if (tracker == NULL) { + if (tv != NULL && dtv != NULL) { + StatsIncr(tv, dtv->counter_defrag_max_hit); + } return NULL; + } Packet *rp = DefragInsertFrag(tv, dtv, tracker, p); DefragTrackerRelease(tracker); From 05ad4bd9b313764594f3f2f48f392a0316eaf25c Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Fri, 17 Feb 2023 17:34:09 -0300 Subject: [PATCH 04/16] misc: fix typos, doc, update copyright years Updated FlowGetNew documentation, where it said NULL was only returned in case of error. (cherry picked from commit f511a4ae3f954a3c55b57a0fdffc0ae22a965dd5) --- src/app-layer.c | 2 +- src/decode.h | 2 +- src/defrag-hash.h | 2 +- src/flow-hash.c | 5 +++-- src/util-exception-policy.c | 2 +- src/util-exception-policy.h | 2 +- 6 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/app-layer.c b/src/app-layer.c index 3e75037ab1fa..68d4e5e6fb09 100644 --- a/src/app-layer.c +++ b/src/app-layer.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2021 Open Information Security Foundation +/* Copyright (C) 2007-2023 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free diff --git a/src/decode.h b/src/decode.h index 33655a584214..91029fa1f146 100644 --- a/src/decode.h +++ b/src/decode.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2022 Open Information Security Foundation +/* Copyright (C) 2007-2023 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free diff --git a/src/defrag-hash.h b/src/defrag-hash.h index 19a9fc0dc6bf..e01e692e9c86 100644 --- a/src/defrag-hash.h +++ b/src/defrag-hash.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2012 Open Information Security Foundation +/* Copyright (C) 2007-2023 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free diff --git a/src/flow-hash.c b/src/flow-hash.c index c9cb8964bd67..a77b4e13b381 100644 --- a/src/flow-hash.c +++ b/src/flow-hash.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2020 Open Information Security Foundation +/* Copyright (C) 2007-2023 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -534,7 +534,8 @@ static inline void NoFlowHandleIPS(Packet *p) * \param tv thread vars * \param fls lookup support vars * - * \retval f *LOCKED* flow on succes, NULL on error. + * \retval f *LOCKED* flow on success, NULL on error or if we should not create + * a new flow. */ static Flow *FlowGetNew(ThreadVars *tv, FlowLookupStruct *fls, Packet *p) { diff --git a/src/util-exception-policy.c b/src/util-exception-policy.c index d3a50fb4cceb..1812bf016363 100644 --- a/src/util-exception-policy.c +++ b/src/util-exception-policy.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2023 Open Information Security Foundation +/* Copyright (C) 2022-2023 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free diff --git a/src/util-exception-policy.h b/src/util-exception-policy.h index 96e22c67503c..924adb31cef3 100644 --- a/src/util-exception-policy.h +++ b/src/util-exception-policy.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2022 Open Information Security Foundation +/* Copyright (C) 2022-2023 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free From 63b2792f65ffc54afe9fc83de685d9c6f7bc4d8c Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Tue, 18 Apr 2023 20:09:16 -0300 Subject: [PATCH 05/16] doc: add midstream scenarios for exception policy The different interactions between midstream pick-up sessions and the exception policy can be quite difficult to visualize. Add a section for that in the userguide. Related to Bug #5825 (cherry picked from commit 0c2922f02efe3e785d6ede01581891683007a6f2) --- .../configuration/exception-policies.rst | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/doc/userguide/configuration/exception-policies.rst b/doc/userguide/configuration/exception-policies.rst index 201a7a3ecb33..981a349bc2bc 100644 --- a/doc/userguide/configuration/exception-policies.rst +++ b/doc/userguide/configuration/exception-policies.rst @@ -113,6 +113,91 @@ are: The *Drop*, *pass* and *reject* are similar to the rule actions described in :ref:`rule actions`. +Exception Policies and Midstream Pick-up Sessions +------------------------------------------------- + +Suricata behavior can be difficult to track in case of midstream session +pick-ups. Consider this matrix illustrating the different interactions for +midstream pick-ups enabled or not and the various exception policy values: + +.. list-table:: **Exception Policy Behaviors - IDS Mode** + :widths: auto + :header-rows: 1 + :stub-columns: 1 + + * - Exception Policy + - Midstream pick-up sessions ENABLED (stream.midstream=true) + - Midstream pick-up sessions DISABLED (stream.midstream=false) + * - Ignore + - Session tracket and parsed. + - Session not tracked. No app-layer inspection or logging. No detection. No stream reassembly. + * - Drop-flow + - Not valid.* + - Not valid.* + * - Drop-packet + - Not valid.* + - Not valid.* + * - Reject + - Not valid.* + - Session not tracked, flow REJECTED. + * - Pass-flow + - Track session, inspect and log app-layer traffic, no detection. + - Session not tracked. No app-layer inspection or logging. No detection. No stream reassembly. + * - Pass-packet + - Not valid.* + - Not valid.* + * - Bypass + - Not valid.* + - Session not tracked. No app-layer inspection or logging. No detection. No stream reassembly. + * - Auto + - Midstream policy applied: "ignore". Same behavior. + - Midstream policy applied: "ignore". Same behavior. + +The main difference between IDS and IPS scenarios is that in IPS mode flows can +be allowed or blocked (as in with the PASS and DROP rule actions). Packet +actions are not valid, as midstream pick-up is a configuration that affects the +whole flow. + +.. list-table:: **Exception Policy Behaviors - IPS Mode** + :widths: 15 42 43 + :header-rows: 1 + :stub-columns: 1 + + * - Exception Policy + - Midstream pick-up sessions ENABLED (stream.midstream=true) + - Midstream pick-up sessions DISABLED (stream.midstream=false) + * - Ignore + - Session tracket and parsed. + - Session not tracked. No app-layer inspection or logging. No detection. No stream reassembly. + * - Drop-flow + - Not valid.* + - Session not tracked. No app-layer inspection or logging. No detection. No stream reassembly. + Flow DROPPED. + * - Drop-packet + - Not valid.* + - Not valid.* + * - Reject + - Not valid.* + - Session not tracked, flow DROPPED and REJECTED. + * - Pass-flow + - Track session, inspect and log app-layer traffic, no detection. + - Session not tracked. No app-layer inspection or logging. No detection. No stream reassembly. + * - Pass-packet + - Not valid.* + - Not valid.* + * - Bypass + - Not valid.* + - Session not tracked. No app-layer inspection or logging. No detection. No stream reassembly. + Packets ALLOWED. + * - Auto + - Midstream policy applied: "ignore". Same behavior. + - Midstream policy applied: "drop-flow". Same behavior. + +Notes: + + * Not valid means that Suricata will error out and won't start. + * ``REJECT`` will make Suricata send a Reset-packet unreach error to the sender of the matching packet. + Command-line Options for Simulating Exceptions ---------------------------------------------- From 82aa48f0e8ab51819c6cef241d9e7fd2c6711ad1 Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Mon, 24 Apr 2023 16:42:34 -0300 Subject: [PATCH 06/16] userguide: update exception policy behaviors table Some exception policies can only be applied to the triggering packet or only make sense considering the whole flow. Highlight such cases in the table showing each exception policy. Related to Bug #5825 (cherry picked from commit c0db25d055e095a099d8a23fd2c2023e4af761c4) --- .../configuration/exception-policies.rst | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/doc/userguide/configuration/exception-policies.rst b/doc/userguide/configuration/exception-policies.rst index 981a349bc2bc..86cfd37427bc 100644 --- a/doc/userguide/configuration/exception-policies.rst +++ b/doc/userguide/configuration/exception-policies.rst @@ -66,28 +66,26 @@ Exception policies are implemented for: - Expected behavior * - stream.memcap - memcap-policy - - If a stream memcap limit is reached, call the memcap policy on the packet - and flow. + - If a stream memcap limit is reached, apply the memcap policy to the packet and/or + flow. * - stream.midstream - midstream-policy - - If a session is picked up midstream, call the memcap policy on the packet - and flow. + - If a session is picked up midstream, apply the midstream policy to the flow. * - stream.reassembly.memcap - memcap-policy - - If stream reassembly reaches memcap limit, call the memcap policy on the - packet and flow. + - If stream reassembly reaches memcap limit, apply memcap policy to the + packet and/or flow. * - flow.memcap - memcap-policy - Apply policy when the memcap limit for flows is reached and no flow could - be freed up. Apply policy to the packet. + be freed up. **Policy can only be applied to the packet.** * - defrag.memcap - memcap-policy - Apply policy when the memcap limit for defrag is reached and no tracker - could be picked up. Apply policy to the packet. + could be picked up. **Policy can only be applied to the packet.** * - app-layer - error-policy - - Apply policy if a parser reaches an error state. Apply policy to the - packet and flow. + - Apply policy if a parser reaches an error state. Policy can be applied to packet and/or flow. To change any of these, go to the specific section in the suricata.yaml file (for more configuration details, check the :doc:`suricata.yaml's` From 4e067da14a822e512ee6108c0424fe412559f4ea Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Mon, 29 May 2023 15:26:22 -0300 Subject: [PATCH 07/16] exception: refactor exception policy parse fn Split up ExceptionPolicyParse to try to improve readability. Related to Bug #5825 (cherry picked from commit bf22129a0fc133b3f4f18997fc0d384c4f9d3751) --- src/util-exception-policy.c | 122 ++++++++++++++++++++++-------------- 1 file changed, 74 insertions(+), 48 deletions(-) diff --git a/src/util-exception-policy.c b/src/util-exception-policy.c index 1812bf016363..c3dc0d0da3b0 100644 --- a/src/util-exception-policy.c +++ b/src/util-exception-policy.c @@ -25,6 +25,8 @@ #include "stream-tcp-reassemble.h" enum ExceptionPolicy g_eps_master_switch = EXCEPTION_POLICY_NOT_SET; +/** true if exception policy was defined in config */ +static bool g_eps_have_exception_policy = false; static const char *ExceptionPolicyEnumToString(enum ExceptionPolicy policy) { @@ -144,65 +146,89 @@ static enum ExceptionPolicy SetIPSOption( return p; } -enum ExceptionPolicy ExceptionPolicyParse(const char *option, const bool support_flow) +static enum ExceptionPolicy ExceptionPolicyConfigValueParse( + const char *option, const char *value_str) { enum ExceptionPolicy policy = EXCEPTION_POLICY_NOT_SET; - const char *value_str = NULL; - if ((ConfGet(option, &value_str)) == 1 && value_str != NULL) { - if (strcmp(value_str, "drop-flow") == 0) { - policy = SetIPSOption(option, value_str, EXCEPTION_POLICY_DROP_FLOW); - SCLogConfig("%s: %s", option, value_str); - } else if (strcmp(value_str, "pass-flow") == 0) { - policy = EXCEPTION_POLICY_PASS_FLOW; - SCLogConfig("%s: %s", option, value_str); - } else if (strcmp(value_str, "bypass") == 0) { - policy = EXCEPTION_POLICY_BYPASS_FLOW; - SCLogConfig("%s: %s", option, value_str); - } else if (strcmp(value_str, "drop-packet") == 0) { - policy = SetIPSOption(option, value_str, EXCEPTION_POLICY_DROP_PACKET); - SCLogConfig("%s: %s", option, value_str); - } else if (strcmp(value_str, "pass-packet") == 0) { - policy = EXCEPTION_POLICY_PASS_PACKET; - SCLogConfig("%s: %s", option, value_str); - } else if (strcmp(value_str, "reject") == 0) { - policy = EXCEPTION_POLICY_REJECT; - SCLogConfig("%s: %s", option, value_str); - } else if (strcmp(value_str, "ignore") == 0) { // TODO name? + if (strcmp(value_str, "drop-flow") == 0) { + policy = SetIPSOption(option, value_str, EXCEPTION_POLICY_DROP_FLOW); + } else if (strcmp(value_str, "pass-flow") == 0) { + policy = EXCEPTION_POLICY_PASS_FLOW; + } else if (strcmp(value_str, "bypass") == 0) { + policy = EXCEPTION_POLICY_BYPASS_FLOW; + } else if (strcmp(value_str, "drop-packet") == 0) { + policy = SetIPSOption(option, value_str, EXCEPTION_POLICY_DROP_PACKET); + } else if (strcmp(value_str, "pass-packet") == 0) { + policy = EXCEPTION_POLICY_PASS_PACKET; + } else if (strcmp(value_str, "reject") == 0) { + policy = EXCEPTION_POLICY_REJECT; + } else if (strcmp(value_str, "ignore") == 0) { // TODO name? + policy = EXCEPTION_POLICY_NOT_SET; + } else if (strcmp(value_str, "auto") == 0) { + if (!EngineModeIsIPS()) { policy = EXCEPTION_POLICY_NOT_SET; - SCLogConfig("%s: %s", option, value_str); - } else if (strcmp(value_str, "auto") == 0) { - policy = SetIPSOption(option, value_str, EXCEPTION_POLICY_DROP_FLOW); - SCLogConfig("%s: %s", option, value_str); } else { - FatalErrorOnInit(SC_ERR_INVALID_ARGUMENT, - "\"%s\" is not a valid exception policy value. Valid options are drop-flow, " - "pass-flow, bypass, drop-packet, pass-packet or ignore.", - value_str); + policy = EXCEPTION_POLICY_DROP_FLOW; } + } else { + FatalErrorOnInit(SC_ERR_INVALID_ARGUMENT, + "\"%s\" is not a valid exception policy value. Valid options are drop-flow, " + "pass-flow, bypass, reject, drop-packet, pass-packet or ignore.", + value_str); + } + + return policy; +} + +static enum ExceptionPolicy ExceptionPolicyMasterParse(const char *value) +{ + enum ExceptionPolicy policy = EXCEPTION_POLICY_NOT_SET; + + policy = ExceptionPolicyConfigValueParse("exception-policy", value); + g_eps_have_exception_policy = true; + policy = SetIPSOption("exception-policy", value, policy); + SCLogConfig("exception-policy set to: %s", ExceptionPolicyEnumToString(policy)); + + return policy; +} +static enum ExceptionPolicy ExceptionPolicyGetDefault(const char *option, bool support_flow) +{ + enum ExceptionPolicy p = EXCEPTION_POLICY_NOT_SET; + if (g_eps_have_exception_policy) { + p = GetMasterExceptionPolicy(option); if (!support_flow) { - policy = PickPacketAction(option, policy); + p = PickPacketAction(option, p); } + SCLogConfig("%s: %s (defined via 'exception-policy' master switch)", option, + ExceptionPolicyEnumToString(p)); + return p; + } else if (EngineModeIsIPS()) { + p = EXCEPTION_POLICY_DROP_FLOW; + } + SCLogConfig("%s: %s (defined via 'built-in default' for %s-mode)", option, + ExceptionPolicyEnumToString(p), EngineModeIsIPS() ? "IPS" : "IDS"); - } else if (strcmp(option, "exception-policy") == 0) { - /* not enabled, we won't change the master exception policy, - for now */ - SCLogInfo("'exception-policy' master switch not set, so ignoring it." - " This behavior will change in Suricata 8, so please update your" - " config. See ticket #5219 for more details."); - g_eps_master_switch = EXCEPTION_POLICY_NOT_SET; - } else { - /* Exception Policy was not defined individually */ - enum ExceptionPolicy master_policy = GetMasterExceptionPolicy(option); - if (master_policy == EXCEPTION_POLICY_NOT_SET) { - SCLogConfig("%s: ignore", option); + return p; +} + +enum ExceptionPolicy ExceptionPolicyParse(const char *option, bool support_flow) +{ + enum ExceptionPolicy policy = EXCEPTION_POLICY_NOT_SET; + const char *value_str = NULL; + + if ((ConfGet(option, &value_str)) == 1 && value_str != NULL) { + if (strcmp(option, "exception-policy") == 0) { + policy = ExceptionPolicyMasterParse(value_str); } else { - /* If the master switch was set and the Exception Policy option was not - individually set, use the defined master Exception Policy */ - const char *value = ExceptionPolicyEnumToString(master_policy); - SCLogConfig("%s: %s (defined via 'exception-policy' master switch", option, value); - policy = master_policy; + policy = ExceptionPolicyConfigValueParse(option, value_str); + if (!support_flow) { + policy = PickPacketAction(option, policy); + } + SCLogConfig("%s: %s", option, ExceptionPolicyEnumToString(policy)); } + } else { + policy = ExceptionPolicyGetDefault(option, support_flow); } return policy; } From d6bfcc0124c82bfa574bdcb5390f35a1605db2cb Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Mon, 29 May 2023 16:55:00 -0300 Subject: [PATCH 08/16] exception/midstream: parse midstream policy alone As the midstream exception policy has its own specific scenarios, have a dedicated function to parse and process its config values, and check for midstream enabled when needed. Related to Bug #5825 (cherry picked from commit f97af0c0b1916ada6cf860b429e2ccfb5b4a3da2) --- src/util-exception-policy.c | 49 ++++++++++++++++++++++++++++++++++--- src/util-exception-policy.h | 1 + 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/util-exception-policy.c b/src/util-exception-policy.c index c3dc0d0da3b0..18aaf8ffee70 100644 --- a/src/util-exception-policy.c +++ b/src/util-exception-policy.c @@ -192,7 +192,8 @@ static enum ExceptionPolicy ExceptionPolicyMasterParse(const char *value) return policy; } -static enum ExceptionPolicy ExceptionPolicyGetDefault(const char *option, bool support_flow) +static enum ExceptionPolicy ExceptionPolicyGetDefault( + const char *option, bool support_flow, bool midstream) { enum ExceptionPolicy p = EXCEPTION_POLICY_NOT_SET; if (g_eps_have_exception_policy) { @@ -203,7 +204,7 @@ static enum ExceptionPolicy ExceptionPolicyGetDefault(const char *option, bool s SCLogConfig("%s: %s (defined via 'exception-policy' master switch)", option, ExceptionPolicyEnumToString(p)); return p; - } else if (EngineModeIsIPS()) { + } else if (EngineModeIsIPS() && !midstream) { p = EXCEPTION_POLICY_DROP_FLOW; } SCLogConfig("%s: %s (defined via 'built-in default' for %s-mode)", option, @@ -217,7 +218,7 @@ enum ExceptionPolicy ExceptionPolicyParse(const char *option, bool support_flow) enum ExceptionPolicy policy = EXCEPTION_POLICY_NOT_SET; const char *value_str = NULL; - if ((ConfGet(option, &value_str)) == 1 && value_str != NULL) { + if ((ConfGet(option, &value_str) == 1) && value_str != NULL) { if (strcmp(option, "exception-policy") == 0) { policy = ExceptionPolicyMasterParse(value_str); } else { @@ -228,7 +229,47 @@ enum ExceptionPolicy ExceptionPolicyParse(const char *option, bool support_flow) SCLogConfig("%s: %s", option, ExceptionPolicyEnumToString(policy)); } } else { - policy = ExceptionPolicyGetDefault(option, support_flow); + policy = ExceptionPolicyGetDefault(option, support_flow, false); + } + + return policy; +} + +enum ExceptionPolicy ExceptionPolicyMidstreamParse(bool midstream_enabled) +{ + enum ExceptionPolicy policy = EXCEPTION_POLICY_NOT_SET; + const char *value_str = NULL; + /* policy was set directly */ + if ((ConfGet("stream.midstream-policy", &value_str)) == 1 && value_str != NULL) { + policy = ExceptionPolicyConfigValueParse("midstream-policy", value_str); + if (midstream_enabled) { + if (policy != EXCEPTION_POLICY_NOT_SET && policy != EXCEPTION_POLICY_PASS_FLOW) { + FatalErrorOnInit(SC_ERR_INVALID_VALUE, + "Error parsing stream.midstream-policy from config file. \"%s\" is " + "not a valid exception policy when midstream is enabled. Valid options " + "are pass-flow and ignore.", + value_str); + } + } + if (!EngineModeIsIPS()) { + if (policy == EXCEPTION_POLICY_DROP_FLOW) { + FatalErrorOnInit(SC_ERR_INVALID_VALUE, + "Error parsing stream.midstream-policy from config file. \"%s\" is " + "not a valid exception policy in IDS mode. See our documentation for a " + "list of all possible values.", + value_str); + } + } + } else { + policy = ExceptionPolicyGetDefault("midstream-policy", true, midstream_enabled); + } + + if (policy == EXCEPTION_POLICY_PASS_PACKET || policy == EXCEPTION_POLICY_DROP_PACKET) { + FatalErrorOnInit(SC_ERR_INVALID_VALUE, + "Error parsing stream.midstream-policy from config file. \"%s\" is " + "not valid for this exception policy. See our documentation for a list of " + "all possible values.", + value_str); } return policy; } diff --git a/src/util-exception-policy.h b/src/util-exception-policy.h index 924adb31cef3..ddf1d3690c07 100644 --- a/src/util-exception-policy.h +++ b/src/util-exception-policy.h @@ -36,6 +36,7 @@ void SetMasterExceptionPolicy(void); void ExceptionPolicyApply( Packet *p, enum ExceptionPolicy policy, enum PacketDropReason drop_reason); enum ExceptionPolicy ExceptionPolicyParse(const char *option, const bool support_flow); +enum ExceptionPolicy ExceptionPolicyMidstreamParse(bool midstream_enabled); extern enum ExceptionPolicy g_eps_master_switch; #ifdef DEBUG From 5116713e9514ea0a223a91b9f5163f95a1bd3fdb Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Wed, 31 May 2023 11:56:43 -0300 Subject: [PATCH 09/16] exception: parse config values, don't post process Get the enum values from the config file. Update the new extracted functions. Post-process the config values based on runmode and policy. Also handle 'auto' enum value in these. Related to Bug #5825 (cherry picked from commit 7f8536b81c59205032676efda62a0b18ff0de224) --- src/util-exception-policy.c | 18 ++++++++++-------- src/util-exception-policy.h | 1 + 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/util-exception-policy.c b/src/util-exception-policy.c index 18aaf8ffee70..29955d5ea2f7 100644 --- a/src/util-exception-policy.c +++ b/src/util-exception-policy.c @@ -33,6 +33,8 @@ static const char *ExceptionPolicyEnumToString(enum ExceptionPolicy policy) switch (policy) { case EXCEPTION_POLICY_NOT_SET: return "ignore"; + case EXCEPTION_POLICY_AUTO: + return "auto"; case EXCEPTION_POLICY_REJECT: return "reject"; case EXCEPTION_POLICY_BYPASS_FLOW: @@ -64,6 +66,8 @@ void ExceptionPolicyApply(Packet *p, enum ExceptionPolicy policy, enum PacketDro { SCLogDebug("start: pcap_cnt %" PRIu64 ", policy %u", p->pcap_cnt, policy); switch (policy) { + case EXCEPTION_POLICY_AUTO: + break; case EXCEPTION_POLICY_NOT_SET: break; case EXCEPTION_POLICY_REJECT: @@ -132,6 +136,8 @@ static enum ExceptionPolicy PickPacketAction(const char *option, enum ExceptionP break; case EXCEPTION_POLICY_NOT_SET: break; + case EXCEPTION_POLICY_AUTO: + break; } return p; } @@ -151,13 +157,13 @@ static enum ExceptionPolicy ExceptionPolicyConfigValueParse( { enum ExceptionPolicy policy = EXCEPTION_POLICY_NOT_SET; if (strcmp(value_str, "drop-flow") == 0) { - policy = SetIPSOption(option, value_str, EXCEPTION_POLICY_DROP_FLOW); + policy = EXCEPTION_POLICY_DROP_FLOW; } else if (strcmp(value_str, "pass-flow") == 0) { policy = EXCEPTION_POLICY_PASS_FLOW; } else if (strcmp(value_str, "bypass") == 0) { policy = EXCEPTION_POLICY_BYPASS_FLOW; } else if (strcmp(value_str, "drop-packet") == 0) { - policy = SetIPSOption(option, value_str, EXCEPTION_POLICY_DROP_PACKET); + policy = EXCEPTION_POLICY_DROP_PACKET; } else if (strcmp(value_str, "pass-packet") == 0) { policy = EXCEPTION_POLICY_PASS_PACKET; } else if (strcmp(value_str, "reject") == 0) { @@ -165,15 +171,11 @@ static enum ExceptionPolicy ExceptionPolicyConfigValueParse( } else if (strcmp(value_str, "ignore") == 0) { // TODO name? policy = EXCEPTION_POLICY_NOT_SET; } else if (strcmp(value_str, "auto") == 0) { - if (!EngineModeIsIPS()) { - policy = EXCEPTION_POLICY_NOT_SET; - } else { - policy = EXCEPTION_POLICY_DROP_FLOW; - } + policy = EXCEPTION_POLICY_AUTO; } else { FatalErrorOnInit(SC_ERR_INVALID_ARGUMENT, "\"%s\" is not a valid exception policy value. Valid options are drop-flow, " - "pass-flow, bypass, reject, drop-packet, pass-packet or ignore.", + "pass-flow, bypass, reject, drop-packet, pass-packet, ignore or auto.", value_str); } diff --git a/src/util-exception-policy.h b/src/util-exception-policy.h index ddf1d3690c07..41af1aabce95 100644 --- a/src/util-exception-policy.h +++ b/src/util-exception-policy.h @@ -24,6 +24,7 @@ enum ExceptionPolicy { EXCEPTION_POLICY_NOT_SET = 0, + EXCEPTION_POLICY_AUTO, EXCEPTION_POLICY_PASS_PACKET, EXCEPTION_POLICY_PASS_FLOW, EXCEPTION_POLICY_BYPASS_FLOW, From 298706d5bd04cd3088769fdc8f6c30d262ade13a Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Thu, 1 Jun 2023 20:33:18 -0300 Subject: [PATCH 10/16] exception: use mix of logconfig/info/warning Use a mix of SCLogConfig, Warning and Info. This mix works as follows: when something unnexpected for the user happens - for instance, the engine ignoring an invalid config value, we use warning. For indicating the value for the master switch, which happens only once, we use Info. For all the other cases, we use SCLogConfig. It is possible that SCLogConfig isn't showing at the moment, this is a possible bug to investigate further. Related to Bug #5825 (cherry picked from commit 69311ab02f33c8396babfe810ac5a066c900d31e) --- src/util-exception-policy.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/util-exception-policy.c b/src/util-exception-policy.c index 29955d5ea2f7..383c72d37031 100644 --- a/src/util-exception-policy.c +++ b/src/util-exception-policy.c @@ -146,7 +146,8 @@ static enum ExceptionPolicy SetIPSOption( const char *option, const char *value_str, enum ExceptionPolicy p) { if (!EngineModeIsIPS()) { - SCLogConfig("%s: %s not a valid config in IDS mode. Ignoring it.)", option, value_str); + SCLogWarning(SC_ERR_INVALID_VALUE, "%s: %s not a valid config in IDS mode. Ignoring it.", + option, value_str); return EXCEPTION_POLICY_NOT_SET; } return p; @@ -189,7 +190,8 @@ static enum ExceptionPolicy ExceptionPolicyMasterParse(const char *value) policy = ExceptionPolicyConfigValueParse("exception-policy", value); g_eps_have_exception_policy = true; policy = SetIPSOption("exception-policy", value, policy); - SCLogConfig("exception-policy set to: %s", ExceptionPolicyEnumToString(policy)); + + SCLogInfo("exception-policy set to: %s", ExceptionPolicyEnumToString(policy)); return policy; } From 49ba6db3f354b7e9d15c17831d709e3d5e84b20d Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Wed, 31 May 2023 22:52:48 -0300 Subject: [PATCH 11/16] exception: extract 'auto' check to function Part of Bug #5825 (cherry picked from commit e849afbda14aae690ab7b2b8c0734aaeba490ec9) --- src/util-exception-policy.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/util-exception-policy.c b/src/util-exception-policy.c index 383c72d37031..820fd2a10a8f 100644 --- a/src/util-exception-policy.c +++ b/src/util-exception-policy.c @@ -183,6 +183,19 @@ static enum ExceptionPolicy ExceptionPolicyConfigValueParse( return policy; } +static enum ExceptionPolicy ExceptionPolicyPickAuto(bool midstream_enabled, bool support_flow) +{ + enum ExceptionPolicy policy = EXCEPTION_POLICY_NOT_SET; + if (!midstream_enabled && EngineModeIsIPS()) { + if (support_flow) { + policy = EXCEPTION_POLICY_DROP_FLOW; + } else { + policy = EXCEPTION_POLICY_DROP_PACKET; + } + } + return policy; +} + static enum ExceptionPolicy ExceptionPolicyMasterParse(const char *value) { enum ExceptionPolicy policy = EXCEPTION_POLICY_NOT_SET; @@ -227,6 +240,9 @@ enum ExceptionPolicy ExceptionPolicyParse(const char *option, bool support_flow) policy = ExceptionPolicyMasterParse(value_str); } else { policy = ExceptionPolicyConfigValueParse(option, value_str); + if (policy == EXCEPTION_POLICY_AUTO) { + policy = ExceptionPolicyPickAuto(false, support_flow); + } if (!support_flow) { policy = PickPacketAction(option, policy); } @@ -246,7 +262,9 @@ enum ExceptionPolicy ExceptionPolicyMidstreamParse(bool midstream_enabled) /* policy was set directly */ if ((ConfGet("stream.midstream-policy", &value_str)) == 1 && value_str != NULL) { policy = ExceptionPolicyConfigValueParse("midstream-policy", value_str); - if (midstream_enabled) { + if (policy == EXCEPTION_POLICY_AUTO) { + policy = ExceptionPolicyPickAuto(midstream_enabled, true); + } else if (midstream_enabled) { if (policy != EXCEPTION_POLICY_NOT_SET && policy != EXCEPTION_POLICY_PASS_FLOW) { FatalErrorOnInit(SC_ERR_INVALID_VALUE, "Error parsing stream.midstream-policy from config file. \"%s\" is " From c0efcbc407937b2d0a311a8b4dab55b9ede4a56e Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Tue, 30 May 2023 10:41:49 -0300 Subject: [PATCH 12/16] stream/tcp: re-enable midstream-policy usage We were always setting it to ignore, due to bug 5825. The engine will now issue an initialization error if an invalid value is passed in the configuration file for midstream exception policy. 'pass-packet' or 'drop-packet' are never valid, as the midstream policy concerns the whole flow, not making sense for just a packet. If midstream is enabled, only two actual config values are allowed: 'ignore' and 'pass-flow', both in IDS and in IPS mode. In default mode ('auto' or if no policy is defined), midstream-policy is set to 'ignore'. All other values will lead to initialization error. In IDS mode, 'drop-flow' will also lead to initialization error. Part of Bug #5825 (cherry picked from commit 69d3750aaf29940c87797eb49ceef7c385e06f43) --- src/stream-tcp.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/stream-tcp.c b/src/stream-tcp.c index d736edef2a09..cad45ea7f088 100644 --- a/src/stream-tcp.c +++ b/src/stream-tcp.c @@ -478,15 +478,7 @@ void StreamTcpInitConfig(char quiet) stream_config.ssn_memcap_policy = ExceptionPolicyParse("stream.memcap-policy", true); stream_config.reassembly_memcap_policy = ExceptionPolicyParse("stream.reassembly.memcap-policy", true); - SCLogConfig("memcap-policy: %u/%u", stream_config.ssn_memcap_policy, - stream_config.reassembly_memcap_policy); - stream_config.midstream_policy = ExceptionPolicyParse("stream.midstream-policy", true); - if (stream_config.midstream && stream_config.midstream_policy != EXCEPTION_POLICY_NOT_SET) { - SCLogWarning(SC_WARN_COMPATIBILITY, - "stream.midstream_policy setting conflicting with stream.midstream enabled. " - "Ignoring stream.midstream_policy."); - stream_config.midstream_policy = EXCEPTION_POLICY_NOT_SET; - } + stream_config.midstream_policy = ExceptionPolicyMidstreamParse(stream_config.midstream); if (!quiet) { SCLogConfig("stream.\"inline\": %s", @@ -962,8 +954,7 @@ static int StreamTcpPacketStateNone(ThreadVars *tv, Packet *p, return 0; } if (!(stream_config.midstream_policy == EXCEPTION_POLICY_NOT_SET || - stream_config.midstream_policy == EXCEPTION_POLICY_PASS_FLOW || - stream_config.midstream_policy == EXCEPTION_POLICY_PASS_PACKET)) { + stream_config.midstream_policy == EXCEPTION_POLICY_PASS_FLOW)) { SCLogDebug("Midstream policy not permissive, so won't pick up a session"); return 0; } @@ -1133,8 +1124,7 @@ static int StreamTcpPacketStateNone(ThreadVars *tv, Packet *p, return 0; } if (!(stream_config.midstream_policy == EXCEPTION_POLICY_NOT_SET || - stream_config.midstream_policy == EXCEPTION_POLICY_PASS_FLOW || - stream_config.midstream_policy == EXCEPTION_POLICY_PASS_PACKET)) { + stream_config.midstream_policy == EXCEPTION_POLICY_PASS_FLOW)) { SCLogDebug("Midstream policy not permissive, so won't pick up a session"); return 0; } From 922706498c269d38247d9ecfe58d2ee27b0778c7 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 13 Jun 2023 09:51:48 +0200 Subject: [PATCH 13/16] exception/policy: minor code cleanup (cherry picked from commit 479fa609fa03719936d147342551d97797c92623) --- src/util-exception-policy.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/util-exception-policy.c b/src/util-exception-policy.c index 820fd2a10a8f..a43e5fee24b8 100644 --- a/src/util-exception-policy.c +++ b/src/util-exception-policy.c @@ -198,11 +198,9 @@ static enum ExceptionPolicy ExceptionPolicyPickAuto(bool midstream_enabled, bool static enum ExceptionPolicy ExceptionPolicyMasterParse(const char *value) { - enum ExceptionPolicy policy = EXCEPTION_POLICY_NOT_SET; - - policy = ExceptionPolicyConfigValueParse("exception-policy", value); - g_eps_have_exception_policy = true; + enum ExceptionPolicy policy = ExceptionPolicyConfigValueParse("exception-policy", value); policy = SetIPSOption("exception-policy", value, policy); + g_eps_have_exception_policy = true; SCLogInfo("exception-policy set to: %s", ExceptionPolicyEnumToString(policy)); From 33bd94ccbe14a1e785db72deb6d9d01124f07945 Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Wed, 14 Jun 2023 20:58:44 -0300 Subject: [PATCH 14/16] exception: fix 'auto' for master switch in IDS If the master exception policy was set to 'auto' in IDS mode, instead of just setting the master switch to the default in this case, which is 'ignore', the engine would switch a warning saying that auto wasn't a valid config and then set the policy to ignore. This makes 'auto' work for the master switch in IDS, removes function for setting IPS option and handles the valid IDS options directly from the function that parses the master policy, as this was the only place where the function was still called. Bug #6149 (cherry picked from commit feb47f9a896b049694f7b5ab40365fab8bbe9d51) --- src/util-exception-policy.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/util-exception-policy.c b/src/util-exception-policy.c index a43e5fee24b8..251183dafe95 100644 --- a/src/util-exception-policy.c +++ b/src/util-exception-policy.c @@ -142,17 +142,6 @@ static enum ExceptionPolicy PickPacketAction(const char *option, enum ExceptionP return p; } -static enum ExceptionPolicy SetIPSOption( - const char *option, const char *value_str, enum ExceptionPolicy p) -{ - if (!EngineModeIsIPS()) { - SCLogWarning(SC_ERR_INVALID_VALUE, "%s: %s not a valid config in IDS mode. Ignoring it.", - option, value_str); - return EXCEPTION_POLICY_NOT_SET; - } - return p; -} - static enum ExceptionPolicy ExceptionPolicyConfigValueParse( const char *option, const char *value_str) { @@ -199,10 +188,15 @@ static enum ExceptionPolicy ExceptionPolicyPickAuto(bool midstream_enabled, bool static enum ExceptionPolicy ExceptionPolicyMasterParse(const char *value) { enum ExceptionPolicy policy = ExceptionPolicyConfigValueParse("exception-policy", value); - policy = SetIPSOption("exception-policy", value, policy); + if (policy == EXCEPTION_POLICY_AUTO) { + policy = ExceptionPolicyPickAuto(false, true); + } else if (!EngineModeIsIPS() && + (policy == EXCEPTION_POLICY_DROP_PACKET || policy == EXCEPTION_POLICY_DROP_FLOW)) { + policy = EXCEPTION_POLICY_NOT_SET; + } g_eps_have_exception_policy = true; - SCLogInfo("exception-policy set to: %s", ExceptionPolicyEnumToString(policy)); + SCLogInfo("master exception-policy set to: %s", ExceptionPolicyEnumToString(policy)); return policy; } From bbfc445b4aa6f20a9dc423c08fea2ae3dbf0b854 Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Wed, 21 Jun 2023 17:54:41 -0300 Subject: [PATCH 15/16] exception: fix use of master switch with default If an exception policy wasn't set up individually, use the GetDefault function to pick one. This will check for the master switch option and handle 'auto' cases. Instead of deciding what the auto value should be when we are parsing the master switch, leave that for when some of the other policies is to be set via the master switch, when since this can change for specific exception policies - like for midstream, for instance. Update exceptions policies documentation to clarify that the default configuration in IPS when midstream is enabled is `ignore`, not `drop-flow`. Bug #6169 (cherry picked from commit e306bc6ecc9f526d02d178c5715e40e493fa8cb6) --- .../configuration/exception-policies.rst | 20 ++++++---- src/util-exception-policy.c | 38 ++++++++++--------- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/doc/userguide/configuration/exception-policies.rst b/doc/userguide/configuration/exception-policies.rst index 86cfd37427bc..b6c952328055 100644 --- a/doc/userguide/configuration/exception-policies.rst +++ b/doc/userguide/configuration/exception-policies.rst @@ -29,25 +29,29 @@ have the need to. :: - # In IPS mode, the default is drop-packet/drop-flow. To fallback to old - # behavior (setting each of them individually, or ignoring all), set this - # to ignore. + # The default is ``ignore``. + # # All values available for exception policies can be used, and there is one - # extra option: auto - which means drop-packet/drop-flow in IPS mode and - # ignore in IDS mode). + # extra option: auto - same as ``ignore``. # Exception policy values are: drop-packet, drop-flow, reject, bypass, # pass-packet, pass-flow, ignore (disable). exception-policy: auto +.. note:: + + The default/``auto`` value changes to ``drop-flow`` in Suricata 7.0, for IPS mode. + This value will be overwritten by specific exception policies whose settings are also defined in the yaml file. Auto '''' -**In IPS mode**, the default behavior for all exception policies is to drop -packets and/or flows. It is possible to disable this default, by setting the -exception policies "master switch" yaml config option to ``ignore``. +The default behavior is to ``ignore`` exception policies. This behavior changes +with Suricata 7.0, where **in IPS mode**, the default for most of the exception +policies is to fail close, that is, ``drop-flow``, or ``drop-packet`` if the +flow action is not supported. For the midstream exception policy, the default +will be ``ignore`` if midstream flows are accepted. **In IDS mode**, setting auto mode actually means disabling the ``master-swtich``, or ignoring the exception policies. diff --git a/src/util-exception-policy.c b/src/util-exception-policy.c index 251183dafe95..eb077007b9a7 100644 --- a/src/util-exception-policy.c +++ b/src/util-exception-policy.c @@ -172,26 +172,17 @@ static enum ExceptionPolicy ExceptionPolicyConfigValueParse( return policy; } +/* 'auto' means ignore, for now */ static enum ExceptionPolicy ExceptionPolicyPickAuto(bool midstream_enabled, bool support_flow) { - enum ExceptionPolicy policy = EXCEPTION_POLICY_NOT_SET; - if (!midstream_enabled && EngineModeIsIPS()) { - if (support_flow) { - policy = EXCEPTION_POLICY_DROP_FLOW; - } else { - policy = EXCEPTION_POLICY_DROP_PACKET; - } - } - return policy; + return EXCEPTION_POLICY_NOT_SET; } static enum ExceptionPolicy ExceptionPolicyMasterParse(const char *value) { enum ExceptionPolicy policy = ExceptionPolicyConfigValueParse("exception-policy", value); - if (policy == EXCEPTION_POLICY_AUTO) { - policy = ExceptionPolicyPickAuto(false, true); - } else if (!EngineModeIsIPS() && - (policy == EXCEPTION_POLICY_DROP_PACKET || policy == EXCEPTION_POLICY_DROP_FLOW)) { + if (!EngineModeIsIPS() && + (policy == EXCEPTION_POLICY_DROP_PACKET || policy == EXCEPTION_POLICY_DROP_FLOW)) { policy = EXCEPTION_POLICY_NOT_SET; } g_eps_have_exception_policy = true; @@ -207,17 +198,28 @@ static enum ExceptionPolicy ExceptionPolicyGetDefault( enum ExceptionPolicy p = EXCEPTION_POLICY_NOT_SET; if (g_eps_have_exception_policy) { p = GetMasterExceptionPolicy(option); + + if (p == EXCEPTION_POLICY_AUTO) { + p = ExceptionPolicyPickAuto(midstream, support_flow); + SCLogConfig("%s: %s (defined via 'exception-policy' master switch). " + "Warning: this will change to drop-flow or drop-packet in Suricata 7.", + option, ExceptionPolicyEnumToString(p)); + return p; + } + if (!support_flow) { p = PickPacketAction(option, p); } SCLogConfig("%s: %s (defined via 'exception-policy' master switch)", option, ExceptionPolicyEnumToString(p)); return p; - } else if (EngineModeIsIPS() && !midstream) { - p = EXCEPTION_POLICY_DROP_FLOW; } - SCLogConfig("%s: %s (defined via 'built-in default' for %s-mode)", option, - ExceptionPolicyEnumToString(p), EngineModeIsIPS() ? "IPS" : "IDS"); + + /* If we don't have the master switch set, default is `not_set` */ + + SCLogConfig("%s: %s (defined via 'built-in default' for %s-mode). " + "Warning: this will change to drop-flow or drop-packet in Suricata 7.", + option, ExceptionPolicyEnumToString(p), EngineModeIsIPS() ? "IPS" : "IDS"); return p; } @@ -275,7 +277,7 @@ enum ExceptionPolicy ExceptionPolicyMidstreamParse(bool midstream_enabled) } } } else { - policy = ExceptionPolicyGetDefault("midstream-policy", true, midstream_enabled); + policy = ExceptionPolicyGetDefault("stream.midstream-policy", true, midstream_enabled); } if (policy == EXCEPTION_POLICY_PASS_PACKET || policy == EXCEPTION_POLICY_DROP_PACKET) { From 9e2fb158ab33bdb9d65c4450606133de98111d55 Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Wed, 26 Jul 2023 15:11:59 +0530 Subject: [PATCH 16/16] dcerpc: accept ALTER_CONTEXT as a valid request So far, if only the starting request was a DCERPC request, it would be considered DCERPC traffic. Since ALTER_CONTEXT is a valid request type, it should be accepted too. Reported and patch proposed in the following Redmine ticket by InterNALXz. Bug 6191 (cherry picked from commit 8770431986598f195d57e570287c40ee3dec0cfa) --- rust/src/dcerpc/dcerpc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/src/dcerpc/dcerpc.rs b/rust/src/dcerpc/dcerpc.rs index bf524a161061..f2a6a46eaf85 100644 --- a/rust/src/dcerpc/dcerpc.rs +++ b/rust/src/dcerpc/dcerpc.rs @@ -1338,7 +1338,7 @@ pub unsafe extern "C" fn rs_dcerpc_get_stub_data( fn probe(input: &[u8]) -> (bool, bool) { match parser::parse_dcerpc_header(input) { Ok((_, hdr)) => { - let is_request = hdr.hdrtype == 0x00; + let is_request = hdr.hdrtype == 0x00 || hdr.hdrtype == 0x0e; let is_dcerpc = hdr.rpc_vers == 0x05 && hdr.rpc_vers_minor == 0x00; return (is_dcerpc, is_request); },