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

Unfortunate chain ordering side effect and potential security vulnerability #46

Open
brianjmurrell opened this issue Feb 10, 2025 · 6 comments

Comments

@brianjmurrell
Copy link

If I have the following rules defined:

config rule
	option target 'DROP'
	option src 'wan'
        option src_ip '1.2.3.4'  # bad actor on the WAN, no SSH allowed!
	option proto 'tcp'
	option dest_port '22'
	option name 'Block Bad Actor on the WAN to SSH to router'

config rule
	option target 'ACCEPT'
	option src '*'
	option proto 'tcp'
	option dest_port '22'
	option name 'SSH to router'

Those will not do what I want and I expect them to and creates a security vulnerability.

Rather the above two rules (even in the order they are in the file!) results in a rule in the input chain allowing SSH to the router from everyone on the WAN (and every other zone, which is the desired behaviour) that gets put in the chain ahead of the rules which dispatch flow to the per-zone input chains. It is in the per-zone input chain for the WAN (wan_input) where the first rule gets installed, but since it comes after the rule in the input chain (that the second rule above installed), it is never given a chance to be evaluated and effected.

The result is that even despite the ordering of the rules in the config file even, the router is open to SSH from the Bad Actor at 1.2.3.4. Not good.

Further, to mitigate the above one has to proliferate the ACCEPT rule into different rule for every single zone one has and one has to keep adding a new rule every time one adds a new zone (think VPN zone growth, where new users get a zone for the VPN connection to the router -- so something that can be quite dynamic). You end up with a huge list of rules such as:

config rule
	option target 'ACCEPT'
	option src 'lan'
	option proto 'tcp'
	option dest_port '22'
	option name 'SSH to router'

config rule
	option target 'DROP'
	option src 'wan'
        option src_ip '1.2.3.4'  # bad actor on the WAN, no SSH allowed!
	option proto 'tcp'
	option dest_port '22'
	option name 'Block Bad Actor on the WAN to SSH to router'

config rule
	option target 'ACCEPT'
	option src 'wan'
	option proto 'tcp'
	option dest_port '22'
	option name 'SSH to router'

config rule
	option target 'ACCEPT'
	option src 'guest'
	option proto 'tcp'
	option dest_port '22'
	option name 'SSH to router'

config rule
	option target 'ACCEPT'
	option src 'vpn_user_1'
	option proto 'tcp'
	option dest_port '22'
	option name 'SSH to router'

config rule
	option target 'ACCEPT'
	option src 'vpn_user_2'
	option proto 'tcp'
	option dest_port '22'
	option name 'SSH to router'

…

You can see how this can get quite out of hand quite quickly compared to the single rule that should be able to cover all zones but also allow exceptions (i.e. the Bad Actor at 1.2.3.4).

@brada4
Copy link

brada4 commented Feb 10, 2025

It is described only as source code or resulting ruleset, but not checking the resulting ruleset creates exposure of vulnerability you have in your ssh.

@brianjmurrell
Copy link
Author

It is described only as source code or resulting ruleset,

I'm not sure I'm getting your meaning here.

but not checking the resulting ruleset

So I have to be able to understand nftables rules in order to use the firewall on OpenWRT and not be exposed to nasty surprises like this? Is it really reasonable to expect every OpenWRT user to be able to understand nftables rules and audit them for correctness, or at least expectedness?

creates exposure of vulnerability you have in your ssh.

I don't have any vulnerability in my ssh. The vulnerability here is that the resulting rules don't seem to reflect what the ordering of the rules in the config file is suggesting.

And the work-around is quite nasty.

@brada4
Copy link

brada4 commented Feb 11, 2025

previous fw3 orders rules same way,
it is counter intuitive since inception.

@brada4
Copy link

brada4 commented Feb 11, 2025

Would it help to have warning that there are no-interface rules preceding specific rule? Similar to one listing rejected invalid rule.

@brada4
Copy link

brada4 commented Feb 11, 2025

Actually it makes a lot of sense optimizing max chain length...

accept established
global rule
iif lan goto lan
iif wan goto wan
drop/accept/reject

then nftablize

acc est
iif vmap (lan : goto lan, wan : goto wan) else global rule(s)
drop/accept/reject

@brianjmurrell
Copy link
Author

Would it help to have warning that there are no-interface rules preceding specific rule? Similar to one listing rejected invalid rule.

Would that only be displayed to CLI fw4 users? If so, I think it would be mostly ineffective.

Actually it makes a lot of sense optimizing max chain length...

No disagreement. But optimisation should be a secondary goal behind correctness, I think, particularly where security is involved. Better to be verbose and correct than concise and surprising/insecure, IMO.

iif vmap (lan : goto lan, wan : goto wan) else global rule(s)

I think that will mean that the global rules won't apply to anything that matches the vmap. That is fine as long as the global rules are replicated in the chains in the vmap.

This is interesting because I was (without a terrible amount of thought) wondering if the <zone>_input chains should be traversed from the input chain first, without a verdict at the end of them so that they return to the input chain where the global rules could then be applied and if no match the verdict at the end of the input chain would apply. The downside of that is that you lose the idea of being able to log that something was dropped/rejected from a zone.

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

2 participants