firewall: use watch-outputs to track changes in zone->interface map

includes a horrible hack to work around (claimed (by me)) deficiencies
in the nftables parser

THOUGHTS.txt

@@ -7020,3 +7020,92 @@ Sun Feb  9 21:33:57 GMT 2025
 nft update set @lan
 
 echo 'flush set table-ip lan;  add element table-ip lan { eth0,lo }' | nft -f -
+
+Tue Feb 11 18:30:09 GMT 2025
+
+outstanding for 1.0:
+
+1) security audit fedback
+
+a) ask ROS if I can ship their report, with a response doc
+ showing the commits that address each finding/non-finding
+b) firewall rules: icmp rate limit, DNS, doc for icmpv6 packet dropping
+c) look over env var inputs and parse them properly instead of
+  string glommeration
+
+2) docs:
+ - for each device, add "finishedness" status and link to build status
+ - generally read them over and spruce up
+ - porting guide
+
+3) some kconfig magic to generate minimal kconfig files so that
+device modules don't end up as copy-pastes of the openwrt defconfig
+
+
+---
+
+apparently 5% of available bandwidth is a reasonable rate limit for
+icmp
+
+% nft add rule filter input limit rate over 10 mbytes/second drop
+
+but nftables has no way to get interface bandwidth and indeed nor does
+the device generally: the 1000Mb/s ethernet interface might be
+connected to a 70Mb/s pppoe upstream and how would it know?  So the
+site operator needs to say somewhere what the upstream bandwidth is.
+
+Sun Feb 16 22:16:29 GMT 2025
+
+we probably didn't need to write that service, we could have used the
+thing that makes templated config files _and_ if we somehow contrive
+to write the interface bandwidth as an interface output we could get
+that the same way
+
+if only I could remember how it worked :-)
+
+----
+
+* watch-output watches only _one_ service and is called with a list of
+outputs inside that service, so not exactly what we need. we can
+extend it easily enough to watch multiple services using poll() if we
+can figure out the syntax we want. Luckily all the places that call it
+go through modules/secrets/subscriber.nix so it's easy enough to change
+existing uses
+
+we could do
+watch-outputs -r foo /nix/store/blah/.outputs/ifname   /nix/store/eee/.outputs/ifname  ...
+
+or
+watch-outputs -r foo /nix/store/blah:ifname   /nix/store/eee:ifname  /nix/store/eee:bandwidth
+
+or
+
+watch-outputs -r foo /nix/store/blah:ifname   /nix/store/eee:ifname:bandwidth
+
+which I quite like insofar as it's shorter but has no other real merit
+
+then we need to decide how to represent an output reference in a firewall rule.
+Since each rule is basically text already, might just put the handlebars straight in
+
+let qq = builtins.toJSON ;
+in "icmp6 limit rate over {{ tonumber(output(${qq (intf "service")}, ${qq (intf "bandwidth")})) / 20 }} bytes/second drop"
+
+probably we should do a separate rule for each interface in the wan zone
+
+Sun Feb 23 00:34:34 GMT 2025
+
+looks like we have no tests for anything involving watched services or subscribers,
+or if we do I can't see what
+
+Thu Feb 27 20:47:03 GMT 2025
+
+- use output-template to write firewall rule file
+- wrap firewall in svc.secrets.subscriber.build (c.f. e745991) with zones as
+   watched services
+- put the handlebars in the firewall config
+
+we have uncommitted changes to watch-outputs that I'm relunctant to
+commit until I have some way to see if they're working. the pppoe test
+will check both firewall zones so _should_ start to fail with the
+current watch-outputs (because only one service) and then pass when we
+put the new one in

modules/firewall/default-rules.nix

@@ -18,10 +18,16 @@ in
       (drop "ip6 saddr 2001:db8::/32") # documentation addresses
       (drop "ip6 daddr 2001:db8::/32")
 
-      # I think this means "check FIB for (saddr, iif) to see if we
-      # could route a packet to that address using that interface",
-      # and if we can't then it was an inapproppriate source address
-      # for packets received _from_ said interface
+      # Reverse path filtering: drop packet if it's not coming from
+      # the same interface that we'd use to send a reply.  Works by
+      # doing a lookup in the FIB to find how we'd route a packet _to_
+      # saddr through iif, and then checking the output interface
+      # returned by the lookup. if oif is 0, that means no route was
+      # found for that address with that interface, so the packet can
+      # be dropped
+      #
+      #  https://wiki.nftables.org/wiki-nftables/index.php/Matching_routing_information#fib
+      #  https://thr3ads.net/netfilter-buglog/2018/01/2843000-Bug-1220-New-Reverse-path-filtering-using-fib-needs-better-documentation
       (drop "fib saddr . iif oif eq 0")
 
       (drop "icmpv6 type router-renumbering")
@@ -49,8 +55,6 @@ in
       (drop "ip6 daddr ::FFFF:0:0/96")
       (drop "ip6 saddr fe80::/10") # link-local unicast
       (drop "ip6 daddr fe80::/10")
-      (drop "ip6 saddr fc00::/7") # unique-local addresses
-      (drop "ip6 daddr fc00::/7")
       (drop "ip6 saddr 2001:10::/28") # ORCHID [RFC4843].
       (drop "ip6 daddr 2001:10::/28")
 
@@ -196,9 +200,7 @@ in
     type = "filter";
     family = "ip";
 
-    rules = [
-      (accept "udp sport 53")
-    ];
+    rules = [];
   };
 
   input-ip4 = {

modules/firewall/service.nix

@@ -5,6 +5,9 @@
   nftables,
   writeFennel,
   anoia,
+  svc,
+  lua,
+  output-template,
   lualinux,
   linotify,
 }:
@@ -18,14 +21,18 @@ let
   inherit (lib.attrsets) mapAttrs' nameValuePair mapAttrsToList;
   inherit (lib.strings) concatStringsSep;
   inherit (lib.lists) flatten;
+  inherit (builtins) concatLists attrValues;
+  inherit (liminix) outputRef;
   mkSet =
     family: name:
     nameValuePair "${name}-set-${family}" {
       kind = "set";
       inherit name family;
       type = "ifname";
+      elements = map (s: "{{ output(${builtins.toJSON s}, \"ifname\", \"\") }}") zones.${name};
     };
-  sets = (mapAttrs' (n: _: mkSet "ip" n) zones) // (mapAttrs' (n: _: mkSet "ip6" n) zones);
+  sets = (mapAttrs' (n: _: mkSet "ip" n) zones) //
+         (mapAttrs' (n: _: mkSet "ip6" n) zones);
   allRules = lib.recursiveUpdate extraRules (lib.recursiveUpdate (builtins.trace sets sets) rules);
   script = firewallgen "firewall1.nft" allRules;
   ifwatch = writeFennel "ifwatch" {
@@ -37,13 +44,26 @@ let
     mainFunction = "run";
   } ./ifwatch.fnl;
   watchArg = z: intfs: map (i: "${z}:${i}/.outputs") intfs;
-in
-longrun {
   name = "firewall";
-  run = ''
-    ${script}
-    PATH=${nftables}/bin:$PATH
-    ${ifwatch} ${concatStringsSep " " (flatten (mapAttrsToList watchArg zones))}
-  '';
-  finish = "${nftables}/bin/nft flush ruleset";
+  service = longrun {
+    inherit name;
+    run = ''
+      mkdir -p /run/${name}; in_outputs ${name}
+      # exec > /dev/console 2>&1
+      echo RESTARTING FIREWALL >/dev/console
+      PATH=${nftables}/bin:${lua}/bin:$PATH
+      ${output-template}/bin/output-template '{{' '}}' < ${script} | lua -e 'for x in io.lines() do if not string.match(x, "elements = {%s+}") then print(x) end; end'   > /run/${name}/fw.nft
+      # cat /run/${name}/fw.nft > /dev/console
+      nft -f /run/${name}/fw.nft
+      while sleep 86400 ; do : ; done
+    '';
+    finish = "${nftables}/bin/nft flush ruleset";
+  };
+in
+svc.secrets.subscriber.build {
+  watch =
+    concatLists
+      (mapAttrsToList (_zone : services : map (s: outputRef s "ifname") services) zones);
+
+  inherit service;
 }

modules/secrets/subscriber.nix

@@ -13,12 +13,11 @@
 }:
 let
   inherit (liminix.services) oneshot longrun;
-  inherit (builtins) length head toString;
-  inherit (lib) unique optional optionals;
+  inherit (builtins) map length head toString;
+  inherit (lib) unique optional optionals concatStringsSep;
   inherit (service) name;
 
   watched-services = unique (map (f: f "service") watch);
-  paths = unique (map (f: f "path") watch);
 
   restart-flag =
     {
@@ -35,17 +34,11 @@ let
     }
     .${action};
 
-  watched-service =
-    if length watched-services == 0 then
-      null
-    else if length watched-services == 1 then
-      head watched-services
-    else
-      throw "cannot subscribe to more than one source service for secrets";
-
   watcher =
     let
       name' = "restart-${name}";
+      refs = concatStringsSep " "
+        (map (s: "${s "service"}:${s "path"}") watch);
     in
     longrun {
       name = name';
@@ -55,16 +48,16 @@ let
         if test -e $dir/notification-fd; then flag="-U"; else flag="-u"; fi
         ${s6}/bin/s6-svwait $flag /run/service/${name} || exit
         PATH=${s6-rc}/bin:${s6}/bin:$PATH
-        ${watch-outputs}/bin/watch-outputs ${restart-flag} ${name} ${watched-service.name} ${lib.concatStringsSep " " paths}
+        ${watch-outputs}/bin/watch-outputs ${restart-flag} ${name} ${refs}
       '';
     };
 in
 service.overrideAttrs (o: {
-  buildInputs = (lim.orEmpty o.buildInputs) ++ optional (watched-service != null) watcher;
+  buildInputs = (lim.orEmpty o.buildInputs) ++ optional (watch != []) watcher;
   dependencies =
     (lim.orEmpty o.dependencies)
-    ++ optionals (watched-service != null) [
-      watcher
-      watched-service
-    ];
+    # ++ optionals
+    #   (watch != [])
+      #   ([ watcher ] ++ watched-services);
+      ;
 })

pkgs/firewallgen/default.nix

@@ -61,7 +61,7 @@ let
     ''
       set ${name}  {
         type ${type}
-        ${if elements != [ ] then "elements = { ${concatStringsSep ", " elements} }" else ""}
+        ${if elements != [ ] then "elements = { ${concatStringsSep ", " (builtins.trace elements elements)} }" else ""}
       }
     '';
 

pkgs/output-template/output-template.fnl

@@ -25,9 +25,9 @@
         myenv {
                : string
                :output
-               (fn [service-path path]
+               (fn [service-path path default]
                  (let [s (assert (svc.open (.. service-path "/.outputs")))]
-                   (s:output path)))
+                   (or (s:output path) default)))
                :lua_quote #(string.format "%q" %1)
                :json_quote (fn [x] (.. "\"" (json-escape x) "\""))
                }]

pkgs/watch-outputs/Makefile

@@ -1,3 +1,3 @@
 check:
-	./output-template  '{{' '}}' < example.ini > output
+	fennelrepl ./test.fnl  '{{' '}}' < example.ini > output
 	diff -u output example.ini.expected

pkgs/watch-outputs/default.nix

@@ -2,6 +2,7 @@
   fetchurl,
   writeFennel,
   fennel,
+  fennelrepl,
   runCommand,
   lua,
   anoia,
@@ -17,9 +18,10 @@ stdenv.mkDerivation {
   src = ./.;
 
   buildInputs = [ lua ];
-  #  doCheck = true;
+  nativeBuildInputs = [ fennelrepl ] ;
 
   buildPhase = ''
+    fennelrepl --test ./watch-outputs.fnl
     cp -p ${
       writeFennel name {
         packages = [
@@ -28,11 +30,13 @@ stdenv.mkDerivation {
           linotify
           fennel
         ];
+        macros = [ anoia.dev ];
         mainFunction = "run";
       } ./watch-outputs.fnl
     } ${name}
+    make check
   '';
-  #  checkPhase = "make check";
+
   installPhase = ''
     install -D ${name} $out/bin/${name}
   '';

pkgs/watch-outputs/output-template.fnl

@@ -28,7 +28,7 @@
                (fn [service-path path]
                  (let [s (assert (svc.open (.. service-path "/.outputs")))]
                    (s:output path)))
-               :lua_quote #(string.format "%q" %1)
+               :lua_quote #(string.format "%q" $1)
                :json_quote (fn [x] (.. "\"" (json-escape x) "\""))
                }]
     (string.gsub text delim

pkgs/watch-outputs/watch-outputs.fnl

@@ -1,10 +1,18 @@
 (local { : %% : system : assoc : split : table= : dig } (require :anoia))
 (local svc (require :anoia.svc))
-(local { : kill } (require :lualinux))
+(local { : kill &as ll} (require :lualinux))
+(import-macros { : define-tests : expect : expect= } :anoia.assert)
 
-(fn split-paths [paths]
-  (icollect [_ path (ipairs paths)]
-    (split "/" path)))
+(local { : view } (require :fennel))
+
+(fn output-refs [outputs]
+  (let [result {}]
+    (each [_ v (ipairs outputs)]
+      (let [[service path] (split ":" v)
+            paths (or (. result service) [])]
+        (table.insert paths (split "/" path ))
+        (tset result service paths)))
+    result))
 
 (fn parse-args [args]
   (match args
@@ -17,37 +25,73 @@
     ["-s" signal service & rest] (assoc (parse-args rest)
                                         :controlled-service service
                                         :action [:signal signal])
-    [watched-service & paths] { : watched-service
-                                :paths (split-paths paths)
-                                }))
+    outputs { :output-references (output-refs outputs) } ))
+
+(define-tests
+  (expect= (parse-args ["-r" "daemon"
+                        "/nix/store/s1:out1"
+                        "/nix/store/s2:out1" "/nix/store/s2:out2/ifname"])
+           {:action "restart"
+            :controlled-service "daemon"
+            :output-references
+            {"/nix/store/s1" [["out1"]]
+             "/nix/store/s2" [["out1"] ["out2" "ifname"]]}}
+           ))
+
 
 (fn changed? [paths old-tree new-tree]
   (accumulate [changed? false
                _ path (ipairs paths)]
     (or changed? (not (table= (dig old-tree path) (dig new-tree path))))))
 
+(define-tests
+  (expect (changed? [["ifname"]] {:ifname "true"} {:ifindex 2}))
+  (expect (changed? [["ifname"]] {:ifname "true"} {:ifname "false"}))
+  (expect (not (changed? [["ifname"]] {:ifname "true"} {:ifname "true"})))
+  (expect (not (changed? [["mtu"]] {:ifname "true"} {:ifname "false"})))
+  (expect (not (changed? [["mtu"]] {:ifname "true"} {:ifname "false"})))
+  )
+
+
+
 (fn do-action [action service]
   (case action
     :restart (system (%% "s6-svc -r /run/service/%s" service))
     :restart-all (system (%% "s6-rc -b -d %q; s6-rc-up-tree %q" service service))
     [:signal n] (system (%% "s6-svc -s %d /run/service/%s" n service))))
 
+(local POLLIN  0x0001)
+(local POLLHUP 0x0010)
+
+(fn wait-for-change [services]
+  (let [pollfds (collect [s _p (ipairs services)]
+                  (bor (lshift (s:fileno) 32)
+                       (lshift (bor POLLIN POLLHUP) 16)))]
+    (ll.poll pollfds)))
+
+(fn open-services [output-references]
+  (collect [s p (pairs output-references)]
+    (values (assert (svc.open (.. s "/.outputs"))) p)))
+
 (fn run []
-  (let [{
+  (let [trees {}
+        {
+         : output-references
          : controlled-service
          : action
          : watched-service
-         : paths } (parse-args arg)
-        dir (.. watched-service "/.outputs")
-        service (assert (svc.open dir))]
-    (print (%% "watching %q" watched-service))
-    (accumulate [tree (service:output ".")
-                 v (service:events)]
-      (let [new-tree (service:output ".")]
-        (when (changed? paths tree new-tree)
-          (print "watched path event:"  action controlled-service)
-          (do-action action controlled-service))
-        new-tree))))
+         : paths } (parse-args arg)]
+    (while true
+      (let [services (open-services output-references)
+            trees (collect [s _ (pairs services)]
+                    (values s (s:output ".")))]
+        (wait-for-change services)
+        (each [service paths (pairs services)]
+          (let [new-tree (service:output ".")]
+            (when (changed? paths (. trees service) new-tree)
+              (print "watched path event:"  action controlled-service)
+              (do-action action controlled-service))))))))
+
 
 
 { : run }

tests/pppoe/configuration.nix

@@ -15,6 +15,7 @@ rec {
     ../../modules/ppp
     ../../modules/dnsmasq
     ../../modules/network
+    ../../modules/firewall
   ];
 
   services.pppoe = svc.pppoe.build {
@@ -23,6 +24,13 @@ rec {
     password = "NotReallyTheSecret";
   };
 
+  services.firewall = svc.firewall.build {
+    zones = {
+      wan = [ services.pppoe ];
+      lan = [ services.lan4 ];
+    };
+  };
+
   services.defaultroute4 = svc.network.route.build {
     via = "$(output ${services.pppoe} address)";
     target = "default";
@@ -39,5 +47,5 @@ rec {
     domain = "fake.liminix.org";
   };
 
-  defaultProfile.packages = [ pkgs.hello ];
+  defaultProfile.packages = with pkgs; [ nftables hello ];
 }

tests/pppoe/getaddress.expect

@@ -1,18 +1,36 @@
 set timeout 60
 
 spawn socat unix-connect:vm/console -
+expect "s6-linux-init"
 send "\r\n"
 expect "#"
+send "PS1=\$(echo 'I1JFQURZIyA=' | base64 -d); stty -echo\n"
+expect "#READY#"
 set FINISHED 0
 set EXIT "1" 
 while { $FINISHED < 10 } {
-  send "ip address show dev ppp0 | grep ppp0\r\n"
+  send "ip address show dev ppp0 | grep ppp0\n"
   expect {
     "192.168.100.1" { set FINISHED 20; set EXIT 0; }
-    "can't find device" { send_user "waiting ..." ; send "\r\n"; sleep 3 }
-    "DOWN" { send_user "waiting ..." ; send "\r\n"; sleep 3 }
+    "can't find device" { send_user "waiting ..." ; sleep 3 }
+    "DOWN" { send_user "waiting ..." ; sleep 3 }
   }
   set FINISHED [ expr $FINISHED + 1 ]
 }
+expect "#READY#"
+send "nft list set ip table-ip wan || touch /non/existent\n"
+expect {
+  "ppp0" { puts "ppp0 found " }
+  "{ }" { puts "missing ifname"; exit 1 }
+  "No such file or directory" { exit 1 }
+}  
+expect "#READY#"
 
+send "nft list set ip table-ip lan || touch /non/existent\n"
+expect {
+  "lan" { puts "lan found" }
+  "{ }" { puts "missing ifname"; exit 1 }
+  "No such file or directory" { exit 1 }
+}  
+expect "#READY#"
 exit $EXIT