minvws / nl-kat-coordination

Repo nl-kat-coordination for minvws
European Union Public License 1.2
125 stars 56 forks source link

Disable Specific Findings #1349

Open zcrt opened 1 year ago

zcrt commented 1 year ago

Is your feature request related to a problem? Please describe. Some organisations are not interested in certain types of findings. An example would be findings regarding missing IPv6. Currently each of these findings can be muted by hand, but I would be better if KAT can be configured to not show certain findings.

Describe the solution you'd like An option to exclude certain findings for a(n) (selection of) organisation(s):

Describe alternatives you've considered Manually muting each time it comes up


Edit from @Donnype:

Status

This might depend on and be partially solved by:

dekkers commented 1 year ago

There are at least two usecases with disabling/muting specific findings:

Donnype commented 1 year ago

@dekkers The second bullet also sounds like the basic use-case for procedures (@underdarknl "Is this not true?" "Is this true but do we accept the risk?", or "Is it true and you fixed it?").

Tackling the first bullet with ConfigOOIs has the caveat that ConfigOOIs are currently only available for bits, while we still yield a lot of Findings and FindingTypes in the normalizers.

Another thought on this is that for this specific issue, being able to enable/disable bits per organisation like we do for boefjes would provide a solution. Going that route currently has the limitation that 1. bits and normalizers cannot be enabled/disabled yet and 2. normalizers often yield both Findings/FindingTypes we possibly want to mute and actual OOIs we might not want to mute.

So currently after a swift look at the code and the issue and keeping the options open, I see three routes.

1. Config OOIs

Issue: not available for normalizers

We could change the normaliser run-interface to pass ConfigOOI's. (We could technically also add them to the boefje_meta.arguments, but this is a bad idea. Another bad idea is to pass this as environment settings for normalizers while passing the same information with ConfigOOIs to bits.) We could also move the creation of all Findings/FindingType to bits, but this might introduce a whole bunch of generic OOI models only to pass semi-normalized and suggestive data to bits to interpret as Findings.

Issue: they apply to all bits

I would opt to decouple this policy from the bit implementation, since we don't want to rewrite all bits with a policy check on the allowed finding (types) and duplicate this everywhere. This means we could special-case on these types of mute configuration and use these objects to filter out all muted finding types from the bit result. We might be doing a lot of unnecessary work in that case

Issue: OOI's are always tied to an organization

We would have to create a ConfigOOI per organization or start thinking about a template organization (and xtdb database) to manage these global policies and configurations.

2. MutedFindingTypes

We could generalize the MutedFinding logic to MutedFindingTypes, and allow the Findings to still live in the graph but we add more special-casing for excluded findings everywhere we query or aggregate them. UPDATE: I just realised this will not work for the use-case above as that has a generic KATFindingType and we could only filter on the id.

Issue: more special-casing

Octopoes will not look nicer when we add more query clauses that are added upon boolean flags such as exclude_findings.

(Repeated) Issue: OOI's are always tied to an organization

3. Enable/Disable bits and normalizers

We could also move forward with being able to enable/disable bits and normalizers in Rocky. If this resolves the use-cases that we have and foresee now, I have a big preference for this route as this is on the roadmap already and makes sense to include in KAT. This would also be an extension of what has been built already instead of adding special cases or changing the current runners/plugins too much.

Issue: we might want to add OOIs that are not Finding(Type)s

This would mean we would have to split those normalizers/bits, but given the setup this would be quite a trivial change per normalizer/bit. It is also possible that we actually only want to mute one of the FindingTypes in a bit.

Issue: the Findings are not physically stored in Octopoes

If actually the user wants to see that they have the finding but only do not treat it as a finding for the rest of KAT, this solution will not suffice. However, the scenario's where we encounter this might also be indications that the given Finding should actually be an OOI and we should create a simpler Finding pointing to that OOI.

zcrt commented 1 year ago
  1. Enable/Disable bits and normalizers

This one is indeed high on the wishlist anyways (#498, #230, #1301). However, one should then be able to enforce that a bit (and a normalizer?) can only create one specific finding type.

Donnype commented 1 year ago

@zcrt Yes that's roughly wat I meant with the first issue under "3.": the question indeed would be whether this would require rewriting a lot of bits and normalizers to get proper granularity for current (and perhaps future) requirements.