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

Allow specifying mod set multiple times #89

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

markkrj
Copy link

@markkrj markkrj commented Jul 26, 2022

Fixes #84

ipset matching module doesn't allow specifying multiple --match-set for a single -m set, so, we'll keep every mod set specified in the config.

@MaxKellermann feel free to edit this PR directly or close it if you have a better idea, as I'm not a programmer.

`ipset` matching module doesn't allow specifying multiple `--match-set` for a single `-m set`, so, we'll keep every `mod set` specified in the config.
@markkrj
Copy link
Author

markkrj commented Jul 27, 2022

I'm testing a script that runs before ferm and output rules that are later included by ferm, and after I opened this PR, I noticed that more modules (in this case, -m comment --comment) does not accept the same parameter multiple times for a single match instance.
So I wonder, why did you put this restriction in the first place @MaxKellermann?
I know that this is an old project, so maybe you don't remember 😅, but if it was just for sake of cleaner iptables output, it might be worth removing this restriction altogether and just keep what was input by the user...

So if user input:

mod set match-set valid-sources src mod set ! match-set invalid-dests MASQUERADE;

it would correctly render:

-A POSTROUTING --match set --match-set valid-sources src --match set ! --match-set invalid-dests dst --jump MASQUERADE

instead of current behavior:

-A POSTROUTING --match set --match-set valid-sources src ! --match-set invalid-dests dst --jump MASQUERADE

Currently, I patched ferm like this PR (but now included the comment module in the regex), but if there was no downside, I'd prefer removing it altogether...

@markkrj
Copy link
Author

markkrj commented Jul 27, 2022

I think this might need a more evolved patch, as I just tested this:

comment this comment is comment a comment test NOP;

and ferm outputs:
-A INPUT --match comment --comment this --comment is --comment a --comment test

which iptables complains:

iptables -A POSTROUTING --match comment --comment this --comment is --comment a --comment test
iptables v1.8.4 (legacy): comment: option "--comment" can only be used once.

Try `iptables -h' or 'iptables --help' for more information.

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

Successfully merging this pull request may close these issues.

Missing -m set option when matching source and destination against ipsets
1 participant