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

_check_filters not called on all user-added matching features #2

Open
diekmann opened this issue Sep 13, 2016 · 3 comments
Open

_check_filters not called on all user-added matching features #2

diekmann opened this issue Sep 13, 2016 · 3 comments

Comments

@diekmann
Copy link

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
Copy link
Contributor

McCio commented Sep 13, 2016

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

@diekmann
Copy link
Author

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
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants