-
-
Notifications
You must be signed in to change notification settings - Fork 16.6k
networking/firewall-nftables: add extraInputRulesEarly to enable drop rules #432491
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me. I've just wondered that we only assert that no iptables commands are configured when nftables is used, but not the other way around?
I feel that this could be better served with the |
Please describe how you would image this be done? The |
You would have these default rules being added to the nixpkgs/nixos/modules/security/sudo.nix Lines 229 to 262 in ad7196a
which is introduced in 23.11: nixpkgs/nixos/doc/manual/release-notes/rl-2311.section.md Lines 523 to 526 in cfe3414
|
How would one prevent this from getting messy when we have to deal with the ordering numbers? In the sudo case, the is probably not the intention for users to add rules to the beginning? |
I'm not sure what you mean by "getting messy"? For user or for the module? In the module, it should just be a single Commit message of sudo module does mention injecting rules being one of the use case for the change: 93011e3 |
With messy I meant the |
BTW: to achieve the desired outcome in the motivation, I would actually suggest to create a different table to handle this. You can just register mutliple hooks in different tables, and nftables will traverse all of them, and any hook can decide to drop the packet. I would recommend just to do networking.nftables.tables.ip_drop = {
family = "inet";
content = ''
set blocked-ip4 {
typeof ip saddr
flags interval
auto-merge
}
chain input {
type filter hook input priority filter; policy accept;
ip saddr @blocked-ip4 counter drop
}
'';
}; Now you also have the flexibility of being able to dynamically modify the set. |
… rules this change introduces option `networking.firewall.extraInputRulesEarly` for the NFT based firewall configuration. the purpose of this function is to allow adding `drop` rules into the input chain while using the existing `networking.firewall.allowed{TCP,UDP}Port{,Range}s`. this is necessary because the `accept` rules generated but allowedPorts are terminal and rules added via the existing `networking.firewall.extraInputRules` cannot affect packets matching them.
7dda3e4
to
60074b6
Compare
hi, i'm not sure what the contribution flow is for core nixos module changes like this, CONTRIBUTING.md seems to be concerned primarily with package changes. i'm not sure if this deserves release note mention as it's a pure addition? should there be a test for this change?
this change introduces option
networking.firewall.extraInputRulesEarly
for the NFT based firewall configuration. the purpose of this option is to allow adding extradrop
rules into the input chain while using the existingnetworking.firewall.allowed{TCP,UDP}Port{,Range}s
. this is necessary because theaccept
rules generated by allowedPorts are terminal and rules added via the existingnetworking.firewall.extraInputRules
cannot affect packet matching them as they appear after those accept rules.personally i'm using the option to ban IP ranges which engage in inconsiderate scraping. i've been using this patch for about 10 months and think it's useful enough addition to nixos to be worth making a PR for.
Things done
passthru.tests
.nixpkgs-review
on this PR. See nixpkgs-review usage../result/bin/
.Add a 👍 reaction to pull requests you find important.