From f58d67677e3179258159d0dc3145ac1aac65e1ec Mon Sep 17 00:00:00 2001
From: Erik Arvstedt <erik.arvstedt@gmail.com>
Date: Mon, 8 Nov 2021 12:45:29 +0100
Subject: [PATCH] netns-isolation: separate host and netns setup

- Improves readability

- `netns exec ...` (called via `netnsIptables`) incurs a large
  overhead: In addition to netns switching, a mount namespace
  is setup and populated with the contents of /etc/netns/<ns>/.
  Instead, simply run `nsenter`.
---
 modules/netns-isolation.nix | 70 ++++++++++++++++++++++---------------
 1 file changed, 41 insertions(+), 29 deletions(-)

diff --git a/modules/netns-isolation.nix b/modules/netns-isolation.nix
index 655325e..f12e391 100644
--- a/modules/netns-isolation.nix
+++ b/modules/netns-isolation.nix
@@ -155,9 +155,42 @@ in {
         veth = "nb-veth-${toString v.id}";
         peer = "nb-veth-br-${toString v.id}";
         inherit (v) netnsName;
-        ipNetns = "${ip} -n ${netnsName}";
-        netnsIptables = "${ip} netns exec ${netnsName} ${config.networking.firewall.package}/bin/iptables";
+        nsenter = "${pkgs.utillinux}/bin/nsenter";
         allowedAddresses = concatMapStringsSep "," (available: netns.${available}.address) v.availableNetns;
+
+        setup = ''
+          ${ip} netns add ${netnsName}
+          ${ip} link add ${veth} type veth peer name ${peer}
+          ${ip} link set ${veth} netns ${netnsName}
+          # The peer link is never used directly, so don't auto-assign an IPv6 address
+          echo 1 > /proc/sys/net/ipv6/conf/${peer}/disable_ipv6
+          ${ip} link set ${peer} up
+          ${ip} link set ${peer} master nb-br
+          exec ${nsenter} --net=/run/netns/${netnsName} ${script "in-netns" setupInNetns}
+        '';
+
+        setupInNetns = ''
+          ${ip} link set lo up
+          ${ip} addr add ${v.address}/24 dev ${veth}
+          ${ip} link set ${veth} up
+          ${ip} route add default via ${bridgeIp}
+
+          ${iptables} -w -P INPUT DROP
+          ${iptables} -w -A INPUT -s 127.0.0.1,${bridgeIp},${v.address} -j ACCEPT
+          # allow return traffic to outgoing connections initiated by the service itself
+          ${iptables} -w -A INPUT -m conntrack --ctstate ESTABLISHED -j ACCEPT
+        '' + optionalString (config.services.${n}.enforceTor or false) ''
+          ${iptables} -w -P OUTPUT DROP
+          ${iptables} -w -A OUTPUT -d 127.0.0.1,${bridgeIp},${v.address} -j ACCEPT
+        '' + optionalString (v.availableNetns != []) ''
+          ${iptables} -w -A INPUT -s ${allowedAddresses} -j ACCEPT
+          ${iptables} -w -A OUTPUT -d ${allowedAddresses} -j ACCEPT
+        '';
+
+        script = name: src: pkgs.writers.writeDash name ''
+          set -e
+          ${src}
+        '';
       in {
         "${n}".serviceConfig.NetworkNamespacePath = "/var/run/netns/${netnsName}";
 
@@ -166,29 +199,11 @@ in {
           after = [ "nb-netns-bridge.service" ];
           requiredBy = [ "${n}.service" ];
           before = requiredBy;
-          script = ''
-            ${ip} netns add ${netnsName}
-            ${ipNetns} link set lo up
-            ${ip} link add ${veth} type veth peer name ${peer}
-            ${ip} link set ${veth} netns ${netnsName}
-            ${ipNetns} addr add ${v.address}/24 dev ${veth}
-            # The peer link is never used directly, so don't auto-assign an IPv6 address
-            echo 1 > /proc/sys/net/ipv6/conf/${peer}/disable_ipv6
-            ${ip} link set ${peer} up
-            ${ipNetns} link set ${veth} up
-            ${ip} link set ${peer} master nb-br
-            ${ipNetns} route add default via ${bridgeIp}
-            ${netnsIptables} -w -P INPUT DROP
-            ${netnsIptables} -w -A INPUT -s 127.0.0.1,${bridgeIp},${v.address} -j ACCEPT
-            # allow return traffic to outgoing connections initiated by the service itself
-            ${netnsIptables} -w -A INPUT -m conntrack --ctstate ESTABLISHED -j ACCEPT
-          '' + optionalString (config.services.${n}.enforceTor or false) ''
-            ${netnsIptables} -w -P OUTPUT DROP
-            ${netnsIptables} -w -A OUTPUT -d 127.0.0.1,${bridgeIp},${v.address} -j ACCEPT
-          '' + optionalString (v.availableNetns != []) ''
-            ${netnsIptables} -w -A INPUT -s ${allowedAddresses} -j ACCEPT
-            ${netnsIptables} -w -A OUTPUT -d ${allowedAddresses} -j ACCEPT
-          '';
+          serviceConfig = {
+            Type = "oneshot";
+            RemainAfterExit = true;
+            ExecStart = script "setup" setup;
+          };
           # Link deletion is implicit in netns deletion, but it sometimes only happens
           # after `netns delete` finishes. Add an extra `link del` to ensure that
           # the link is deleted before the service stops, which is needed for service
@@ -197,10 +212,7 @@ in {
             ${ip} netns delete ${netnsName}
             ${ip} link del ${peer} 2> /dev/null || true
           '';
-          serviceConfig = {
-            Type = "oneshot";
-            RemainAfterExit = true;
-          };
+
         };
       };
     in foldl (services: n: