Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

exception policies batch backports - v3 #9279

Closed

Conversation

jufajardini
Copy link
Contributor

Previous PR: #9167

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/6035
https://redmine.openinfosecfoundation.org/issues/5935
https://redmine.openinfosecfoundation.org/issues/6149 (backport ticket not created)
https://redmine.openinfosecfoundation.org/issues/6169 (backport ticket not created)

Describe changes from previous PR:

  • adjust default behavior for 6 to ignore, instead of fail close

SV_BRANCH=pr/1322
OISF/suricata-verify#1322

jufajardini and others added 15 commits July 24, 2023 09:48
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 OISF#5219

(cherry picked from commit 0d92890)
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 OISF#6109

(cherry picked from commit 8f324e3)
7a044a9 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 OISF#5816

(cherry picked from commit a37a88d)
Updated FlowGetNew documentation, where it said NULL was only returned
in case of error.

(cherry picked from commit f511a4a)
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 OISF#5825

(cherry picked from commit 0c2922f)
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 OISF#5825

(cherry picked from commit c0db25d)
Split up ExceptionPolicyParse to try to improve readability.

Related to
Bug OISF#5825

(cherry picked from commit bf22129)
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 OISF#5825

(cherry picked from commit f97af0c)
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 OISF#5825

(cherry picked from commit 7f8536b)
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 OISF#5825

(cherry picked from commit 69311ab)
Part of
Bug OISF#5825

(cherry picked from commit e849afb)
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 OISF#5825

(cherry picked from commit 69d3750)
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 OISF#6149

(cherry picked from commit feb47f9)
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 OISF#6169

(cherry picked from commit e306bc6)
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 15376

This was referenced Jul 28, 2023
@victorjulien
Copy link
Member

Merged in #9304, thanks!

@jufajardini jufajardini deleted the eps-backports-batch/v3.1 branch August 2, 2023 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants