This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/is-there-a-way-to-extend-the-default-list-of-modules-imported-via-an-overlay-of-some-sort/5282/6

successfully tested with module overlay

# drop the new rules
iptables -t mangle -D PREROUTING -j nixos-fw-rpfilter
iptables -t mangle -F nixos-fw-rpfilter
iptables -t mangle -X nixos-fw-rpfilter

nixos-rebuild switch

# table mangle
iptables -t mangle -L | grep rpfilter

# table raw
iptables -t raw -L | grep rpfilter
nixos-fw-rpfilter  all  --  anywhere             anywhere            
Chain nixos-fw-rpfilter (1 references)
RETURN     all  --  anywhere             anywhere             rpfilter validmark

mkdir -p /etc/nixos/extra-modules/networking
cd /etc/nixos/extra-modules/networking
# https://github.com/NixOS/nixpkgs/pull/110197
wget https://github.com/NixOS/nixpkgs/raw/15e75e8ff564ae8e6b8349b537098c0c80fed0d9/nixos/modules/services/networking/firewall.nix
wget https://github.com/NixOS/nixpkgs/raw/master/nixos/modules/services/networking/helpers.nix
cd /etc/nixos/extra-modules
# https://github.com/Infinisil/system/pull/4
wget https://github.com/Infinisil/system/raw/838b30ea319e0334bd542048dd3a9a8554c6e843/config/new-modules/default.nix

{
  imports = [
      ./hardware-configuration.nix
      ./extra-modules # module overlays: https://discourse.nixos.org/t/5282
    ];
}

{
  ignorePaths = [
    "networking/helpers.nix"
  ];
}

{
  disabledModules = [ "services/networking/firewall.nix" ]; # FIXME only needed in module overlay
  ###### interface

# drop the old rules
iptables -t raw -D PREROUTING -j nixos-fw-rpfilter
iptables -t raw -F nixos-fw-rpfilter
iptables -t raw -X nixos-fw-rpfilter

nixos-rebuild switch

# table mangle
iptables -t mangle -L | grep rpfilter
nixos-fw-rpfilter  all  --  anywhere             anywhere            
Chain nixos-fw-rpfilter (1 references)
RETURN     all  --  anywhere             anywhere             rpfilter validmark

# table raw
iptables -t raw -L | grep rpfilter

I marked this as stale due to inactivity. → More info

thanks for your review

i also wanted to show a notice to change config

{
networking.firewall.checkReversePath = false; # -> show notice
networking.firewall.checkReversePathNoticePR110197 = false; # -> hide notice

sample notice:

please remove checkReversePath=false from your config. in most cases, this is a workaround for wireguard, which is not needed any longer. if you really want to disable rpfilter, set checkReversePathNoticePR110197=false; to hide this message

thanks for your review

i also wanted to show a notice to change config

{
networking.firewall.checkReversePath = false; # -> show notice
networking.firewall.checkReversePathNoticePR110197 = false; # -> hide notice

sample notice:

please remove checkReversePath=false from your config. in most cases, this is a workaround for wireguard, which is not needed any longer. if you really want to disable rpfilter, set checkReversePathNoticePR110197=false; to hide this message

Not sure this is a good idea. There are many legitimate reasons for disabling rpfilter, such as when running a router.

rp_filter is "default on" for better security

its just a warning in the console, similar to option x was renamed

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/844

Looks like somebody ran into this in the #wireguard IRC channel today. Any reason it's still pending?

It's me, I'm somebody. I'll probably have a few comments on this, let me just test it first.

See firewalld/firewalld#603 for a similar issue fixed by moving the rpfilter check to the mangle table.

fixed 2 comments, rebased

We should probably also remove

nixpkgs/nixos/modules/services/networking/wg-quick.nix

Lines 331 to 333 in d51fd78

	# This is forced to false for now because the default "--validmark" rpfilter we apply on reverse path filtering
	# breaks the wg-quick routing because wireguard packets leave with a fwmark from wireguard.
	networking.firewall.checkReversePath = false;

since this PR fixes the issue.

I was suggesting doing this in this PR

Last nit: please prefix the commit message with nixos/firewall:

(and maybe succinctly explain why this is needed in the commit description)

See firewalld/firewalld#603 for a similar issue fixed by moving the rpfilter check to the mangle table.

also adds --validmark to rpfilter
but this is not needed to fix my use case (wireguard)

https://ipset.netfilter.org/iptables-extensions.man.html

rpfilter

Performs a reverse path filter test on a packet. If a reply to the packet would be sent via the same interface that the packet arrived on, the packet will match. [...]

--validmark
Also use the packets' nfmark value when performing the reverse path route lookup.

We already have --validmark (in fact this would not work without --validmark because then the fwmark wouldn't be taken into account at all).

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/626

@bobby285271 why remove the approvals label?

I have a bot that runs https://github.com/nix-community/label-approved regularly and it removes approval labels in some cases, maybe we disable that behavior? /cc @Artturin

Anyway, please merge 😅

ping @SuperSandro2000 @NickCao @nrdxp

guys, can we please merge this trivial patch?

Sorry, I am not very confident with networking.

I have a bot that runs https://github.com/nix-community/label-approved regularly and it removes approval labels in some cases, maybe we disable that behavior? /cc @Artturin

No, it was requested by @zowoq

@Artturin I'm assuming the labels are from a bot? Having it label PRs as approved when the approval is outdated (the PR has new commits or force-pushed) is misleading.

#172542 (comment)