secgroup/Mignis

_check_filters not called on all user-added matching features

Opened this issue · 3 comments

diekmann commented

Hi,

I just played around with mignis. I tried the following rule:

* > local:22  tcp --sports 22

_check_filters should prevent me from doing this. However, when it is called, my offending string --sports is not in the variable filters but in the variable protocol:
https://github.com/secgroup/Mignis/blob/master/mignis.py#L45

As a result, the rule loads fine without a warning.

McCio commented

Hi @diekmann ,

--sports happens to be interpreted as part of the protocol because, as explained in the readme, the syntax for custom iptables filters is abstract-rule | iptables-filters.
So the correct way to write that rule is: * > local:22 tcp | --sports 22.

This issue highlights that trust is given to users for writing correct protocol names. Maybe a warning for "unknown protocol" should be raised.

Happy playing with Mignis!
McCio

  • 👍2
diekmann commented

Awesome, thanks 👍

Now a rather tough question:
I can also use -m multiport --ports 22 to match on ports and Mignis does not complain. Looking at the code, this could be fixed easily.

Yet, let's get to the corner cases of iptables: I could use the modules string, bpf, u32, ... to also match on ports and probably new match extensions might be added with new releases. Mignis also trusts the user not to do those things?

repnzscasb commented

@diekmann
You can write any filter you want after |, but mignis cannot provide any correctness guarantee about the whole configuration if you do that.
The abstract model doesn't cover all the cases that could be generated by using filters, so we don't support them.

@McCio
I think this could be added.
Currently iptables considers a protocol string as valid in 4 different cases:

  • special keyword "all".
  • any number between 0 and 255.
  • if matches any entry in /etc/protocols.
  • if matches any element in the array xtables_chain_protos (hardcoded in libxtables, link)

The only problem I see is that, if /etc/protocol doesn't exist in the system (which is the reason for xtables_chain_protos to exist), or nothing was matched, then mignis should somehow know beforehand the contents of that array, in order not to give a false "Unknown protocol" answer.
We could hardcode the current array in mignis but then only a warning could be raised, since it might not match very old iptables versions.

  • ❤️1