Merge pull request #239028 from chayleaf/miniupnpd

miniupnpd-nftables: init at 2.3.3

nixos/modules/services/networking/miniupnpd.nix

@@ -13,8 +13,17 @@ let
       listening_ip=${range}
     '') cfg.internalIPs}
 
+    ${lib.optionalString (firewall == "nftables") ''
+      upnp_table_name=miniupnpd
+      upnp_nat_table_name=miniupnpd
+    ''}
+
     ${cfg.appendConfig}
   '';
+  firewall = if config.networking.nftables.enable then "nftables" else "iptables";
+  miniupnpd = pkgs.miniupnpd.override { inherit firewall; };
+  firewallScripts = lib.optionals (firewall == "iptables")
+    ([ "iptables"] ++ lib.optional (config.networking.enableIPv6) "ip6tables");
 in
 {
   options = {
@@ -57,20 +66,50 @@ in
   };
 
   config = mkIf cfg.enable {
-    networking.firewall.extraCommands = ''
-      ${pkgs.bash}/bin/bash -x ${pkgs.miniupnpd}/etc/miniupnpd/iptables_init.sh -i ${cfg.externalInterface}
-    '';
+    networking.firewall.extraCommands = lib.mkIf (firewallScripts != []) (builtins.concatStringsSep "\n" (map (fw: ''
+      EXTIF=${cfg.externalInterface} ${pkgs.bash}/bin/bash -x ${miniupnpd}/etc/miniupnpd/${fw}_init.sh
+    '') firewallScripts));
 
-    networking.firewall.extraStopCommands = ''
-      ${pkgs.bash}/bin/bash -x ${pkgs.miniupnpd}/etc/miniupnpd/iptables_removeall.sh -i ${cfg.externalInterface}
-    '';
+    networking.firewall.extraStopCommands = lib.mkIf (firewallScripts != []) (builtins.concatStringsSep "\n" (map (fw: ''
+      EXTIF=${cfg.externalInterface} ${pkgs.bash}/bin/bash -x ${miniupnpd}/etc/miniupnpd/${fw}_removeall.sh
+    '') firewallScripts));
+
+    networking.nftables = lib.mkIf (firewall == "nftables") {
+      # see nft_init in ${miniupnpd-nftables}/etc/miniupnpd
+      tables.miniupnpd = {
+        family = "inet";
+        # The following is omitted because it's expected that the firewall is to be responsible for it.
+        #
+        # chain forward {
+        #   type filter hook forward priority filter; policy drop;
+        #   jump miniupnpd
+        # }
+        #
+        # Otherwise, it quickly gets ugly with (potentially) two forward chains with "policy drop".
+        # This means the chain "miniupnpd" never actually gets triggered and is simply there to satisfy
+        # miniupnpd. If you're doing it yourself (without networking.firewall), the easiest way to get
+        # it to work is adding a rule "ct status dnat accept" - this is what networking.firewall does.
+        # If you don't want to simply accept forwarding for all "ct status dnat" packets, override
+        # upnp_table_name with whatever your table is, create a chain "miniupnpd" in your table and
+        # jump into it from your forward chain.
+        content = ''
+          chain miniupnpd {}
+          chain prerouting_miniupnpd {
+            type nat hook prerouting priority dstnat; policy accept;
+          }
+          chain postrouting_miniupnpd {
+            type nat hook postrouting priority srcnat; policy accept;
+          }
+        '';
+      };
+    };
 
     systemd.services.miniupnpd = {
       description = "MiniUPnP daemon";
       after = [ "network.target" ];
       wantedBy = [ "multi-user.target" ];
       serviceConfig = {
-        ExecStart = "${pkgs.miniupnpd}/bin/miniupnpd -f ${configFile}";
+        ExecStart = "${miniupnpd}/bin/miniupnpd -f ${configFile}";
         PIDFile = "/run/miniupnpd.pid";
         Type = "forking";
       };

nixos/tests/all-tests.nix

@@ -910,7 +910,8 @@ in {
   unbound = handleTest ./unbound.nix {};
   unifi = handleTest ./unifi.nix {};
   unit-php = handleTest ./web-servers/unit-php.nix {};
-  upnp = handleTest ./upnp.nix {};
+  upnp.iptables = handleTest ./upnp.nix { useNftables = false; };
+  upnp.nftables = handleTest ./upnp.nix { useNftables = true; };
   uptermd = handleTest ./uptermd.nix {};
   uptime-kuma = handleTest ./uptime-kuma.nix {};
   usbguard = handleTest ./usbguard.nix {};

nixos/tests/upnp.nix

@@ -5,7 +5,7 @@
 # this succeeds an external client will try to connect to the port
 # mapping.
 
-import ./make-test-python.nix ({ pkgs, ... }:
+import ./make-test-python.nix ({ pkgs, useNftables, ... }:
 
 let
   internalRouterAddress = "192.168.3.1";
@@ -27,6 +27,7 @@ in
           networking.nat.enable = true;
           networking.nat.internalInterfaces = [ "eth2" ];
           networking.nat.externalInterface = "eth1";
+          networking.nftables.enable = useNftables;
           networking.firewall.enable = true;
           networking.firewall.trustedInterfaces = [ "eth2" ];
           networking.interfaces.eth1.ipv4.addresses = [
@@ -82,7 +83,7 @@ in
       # Wait for network and miniupnpd.
       router.wait_for_unit("network-online.target")
       # $router.wait_for_unit("nat")
-      router.wait_for_unit("firewall.service")
+      router.wait_for_unit("${if useNftables then "nftables" else "firewall"}.service")
       router.wait_for_unit("miniupnpd")
 
       client1.wait_for_unit("network-online.target")

pkgs/tools/networking/miniupnpd/default.nix

@@ -1,10 +1,30 @@
 { stdenv, lib, fetchurl, iptables-legacy, libuuid, openssl, pkg-config
-, which, iproute2, gnused, coreutils, gawk, makeWrapper
+, which, iproute2, gnused, coreutils, gnugrep, gawk, makeWrapper
 , nixosTests
+, firewall ? "iptables", nftables, libmnl, libnftnl
 }:
 
 let
-  scriptBinEnv = lib.makeBinPath [ which iproute2 iptables-legacy gnused coreutils gawk ];
+  scriptBinEnv = lib.makeBinPath {
+    iptables = [
+      # needed for dirname in ip{,6}tables_*.sh
+      coreutils
+      # used in miniupnpd_functions.sh:
+      which
+      iproute2
+      iptables-legacy
+      gnused
+      gnugrep
+      gawk
+    ];
+    nftables = [
+      # needed for dirname in nft_*.sh & cat in nft_init.sh
+      coreutils
+      # used in miniupnpd_functions.sh:
+      which
+      nftables
+    ];
+  }.${firewall};
 in
 stdenv.mkDerivation rec {
   pname = "miniupnpd";
@@ -15,22 +35,42 @@ stdenv.mkDerivation rec {
     sha256 = "sha256-b9cBn5Nv+IxB58gi9G8QtRvXLWZZePZYZIPedbMMNr8=";
   };
 
-  buildInputs = [ iptables-legacy libuuid openssl ];
+  buildInputs = [ iptables-legacy libuuid openssl ]
+    ++ lib.optionals (firewall == "nftables") [ libmnl libnftnl ];
   nativeBuildInputs= [ pkg-config makeWrapper ];
 
-
   # ./configure is not a standard configure file, errors with:
   # Option not recognized : --prefix=
   dontAddPrefix = true;
+  configureFlags = [
+    "--firewall=${firewall}"
+    # allow using various config options
+    "--ipv6"
+    "--leasefile"
+    "--regex"
+    "--vendorcfg"
+    # hardening
+    "--portinuse"
+  ];
 
   installFlags = [ "PREFIX=$(out)" "INSTALLPREFIX=$(out)" ];
 
-  postFixup = ''
-    for script in $out/etc/miniupnpd/ip{,6}tables_{init,removeall}.sh
-    do
-      wrapProgram $script --set PATH '${scriptBinEnv}:$PATH'
-    done
-  '';
+  postFixup = {
+    # Ideally we'd prefer using system's config.firewall.package here for iptables,
+    # however for some reason switching --prefix to --suffix breaks the script
+    iptables = ''
+      for script in $out/etc/miniupnpd/ip{,6}tables_{init,removeall}.sh
+      do
+        wrapProgram $script --prefix PATH : '${scriptBinEnv}:$PATH'
+      done
+    '';
+    nftables = ''
+      for script in $out/etc/miniupnpd/nft_{delete_chain,flush,init,removeall}.sh
+      do
+        wrapProgram $script --suffix PATH : '${scriptBinEnv}:$PATH'
+      done
+    '';
+  }.${firewall};
 
   passthru.tests = {
     bittorrent-integration = nixosTests.bittorrent;
@@ -42,5 +82,6 @@ stdenv.mkDerivation rec {
     description = "A daemon that implements the UPnP Internet Gateway Device (IGD) specification";
     platforms = platforms.linux;
     license = licenses.bsd3;
+    mainProgram = "miniupnpd";
   };
 }

pkgs/top-level/all-packages.nix

@@ -10894,6 +10894,8 @@ with pkgs;
 
   miniupnpd = callPackage ../tools/networking/miniupnpd { };
 
+  miniupnpd-nftables = callPackage ../tools/networking/miniupnpd { firewall = "nftables"; };
+
   miniball = callPackage ../development/libraries/miniball { };
 
   minijail = callPackage ../tools/system/minijail { };