From 96e5b7b85990168a60408526d2d044e10ff28c8e Mon Sep 17 00:00:00 2001 From: Nils Stinnesbeck Date: Tue, 27 Oct 2026 15:00:24 +0100 Subject: [PATCH] Derive LAN subnets and validate addressing at build time Add an IPv4 helper lib (parse, netmask, cidr, overlap) and derive router.lans..ipv4.subnet from address and prefixLength instead of requiring it twice. New assertions reject invalid addresses, DHCP pools that are reversed, outside the subnet or contain the router address, and overlapping LANs. Add `lib` unit tests and an `assertions` check that evaluates broken configs and compares the exact error messages. Co-Authored-By: Claude Sonnet 5.5 --- PLAN.md | 4 +- flake.nix | 2 +- modules/router/assertions.nix | 89 +++++++++++++++++++++++++++- modules/router/lib.nix | 49 +++++++++++++++ modules/router/options.nix | 93 ++++++++++++++++------------- tests/assertions.nix | 108 ++++++++++++++++++++++++++++++++++ tests/basic.nix | 2 +- tests/default.nix | 2 + tests/lib.nix | 60 +++++++++++++++++++ 9 files changed, 362 insertions(+), 47 deletions(-) create mode 100644 modules/router/lib.nix create mode 100644 tests/assertions.nix create mode 100644 tests/lib.nix diff --git a/PLAN.md b/PLAN.md index 23c27d8..3a6c95e 100644 --- a/PLAN.md +++ b/PLAN.md @@ -70,8 +70,8 @@ Learned along the way: - The default policy now explicitly answers ICMP echo on LANs only; the WAN gets only ICMP error types and ND. - In NixOS tests `fail(...)` also passes on "command not found": install probe tools explicitly and add a positive control per negative check. - Unbound answers `.test` locally (RFC 6761) and validates DNSSEC; tests using fake domains need both `local-zone ... transparent` and `domain-insecure` (both verified necessary; set in the test, not the module). -- Subnets are given explicitly (`subnet` + `address`) for now; a small CIDR helper lib is needed before IPv6 PD / VLAN subnet-ID allocation. -- Not yet covered: IPv6, VLANs, WireGuard, formal zones, a test for the assertions themselves (verified manually via `nix eval`). +- `modules/router/lib.nix` has IPv4 helpers (parse/format/netmask/cidr/overlap). `ipv4.subnet` is now derived from `address` + `prefixLength` (read-only). Assertions also cover pool bounds, pool containing the router address, and overlapping LANs. IPv6 helpers (prefix arithmetic for PD subnet-IDs) are still to do. +- Checks: `basic` (VM test), `lib` (helper unit tests), `assertions` (broken configs must fail with exact messages). Not yet covered: IPv6, VLANs, WireGuard, formal zones. ## Open items diff --git a/flake.nix b/flake.nix index 53957b6..50a5361 100644 --- a/flake.nix +++ b/flake.nix @@ -41,6 +41,6 @@ program = "${lab}/bin/router-lab"; }; - formatter = forAllSystems (pkgs: pkgs.nixfmt-rfc-style); + formatter = forAllSystems (pkgs: pkgs.nixfmt); }; } diff --git a/modules/router/assertions.nix b/modules/router/assertions.nix index 9ba8baa..0223d39 100644 --- a/modules/router/assertions.nix +++ b/modules/router/assertions.nix @@ -1,7 +1,68 @@ { config, lib, ... }: let cfg = config.router; - lans = lib.attrValues cfg.lans; + ipv4 = (import ./lib.nix { inherit lib; }).ipv4; + + lansByName = cfg.lans; + lans = lib.attrValues lansByName; + + validIp = s: ipv4.parse s != null; + + # Assertions about a single LAN; skipped (reported once) when its addresses + # do not even parse, so we never feed null into the integer helpers. + perLan = + name: lan: + let + where = "router.lans.${name}"; + addrOk = validIp lan.ipv4.address; + startOk = validIp lan.dhcp.pool.start; + endOk = validIp lan.dhcp.pool.end; + addr = ipv4.parse lan.ipv4.address; + start = ipv4.parse lan.dhcp.pool.start; + end = ipv4.parse lan.dhcp.pool.end; + net = { + address = addr; + inherit (lan.ipv4) prefixLength; + }; + parsed = addrOk && startOk && endOk; + in + [ + { + assertion = addrOk; + message = "router: ${where}.ipv4.address `${lan.ipv4.address}` is not a valid IPv4 address."; + } + ] + ++ lib.optionals lan.dhcp.enable ( + [ + { + assertion = startOk && endOk; + message = "router: ${where}.dhcp.pool.start/end must be valid IPv4 addresses."; + } + ] + ++ lib.optionals parsed [ + { + assertion = start <= end; + message = "router: ${where}.dhcp.pool starts after it ends."; + } + { + assertion = ipv4.inSubnet net start && ipv4.inSubnet net end; + message = "router: ${where}.dhcp.pool is not inside ${lan.ipv4.subnet}."; + } + { + assertion = !(start <= addr && addr <= end); + message = "router: ${where}.dhcp.pool contains the router's own address ${lan.ipv4.address}."; + } + ] + ); + + overlapPairs = lib.concatLists ( + lib.imap0 ( + i: a: + map (b: { + inherit a b; + }) (lib.drop (i + 1) (lib.attrNames lansByName)) + ) (lib.attrNames lansByName) + ); in { config = lib.mkIf cfg.enable { @@ -26,6 +87,30 @@ in lib.length ifaces == lib.length (lib.unique ifaces); message = "router: two LANs share the same interface."; } - ]; + ] + ++ lib.concatLists (lib.mapAttrsToList perLan lansByName) + ++ map ( + { a, b }: + let + la = lansByName.${a}; + lb = lansByName.${b}; + in + { + # Only comparable when both addresses parse; the per-LAN assertion reports the rest. + assertion = + !(validIp la.ipv4.address && validIp lb.ipv4.address) + || !(ipv4.overlaps + { + address = ipv4.parse la.ipv4.address; + inherit (la.ipv4) prefixLength; + } + { + address = ipv4.parse lb.ipv4.address; + inherit (lb.ipv4) prefixLength; + } + ); + message = "router: LANs `${a}` (${la.ipv4.subnet}) and `${b}` (${lb.ipv4.subnet}) overlap."; + } + ) overlapPairs; }; } diff --git a/modules/router/lib.nix b/modules/router/lib.nix new file mode 100644 index 0000000..20bdbf5 --- /dev/null +++ b/modules/router/lib.nix @@ -0,0 +1,49 @@ +# Small IPv4 helpers (addresses are plain 32-bit integers inside Nix). +# Parsing returns null on malformed input so callers can turn that into an +# assertion message instead of an evaluation crash. +{ lib }: +let + pow2 = n: if n == 0 then 1 else 2 * pow2 (n - 1); +in +rec { + ipv4 = rec { + parse = + s: + let + m = builtins.match "([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})" s; + octets = map lib.toInt m; + in + if m == null || lib.any (o: o > 255) octets then + null + else + lib.foldl' (acc: o: acc * 256 + o) 0 octets; + + format = + n: + lib.concatMapStringsSep "." (shift: toString (lib.mod (n / pow2 shift) 256)) [ + 24 + 16 + 8 + 0 + ]; + + netMask = prefixLength: 4294967295 - (pow2 (32 - prefixLength) - 1); + + # Network address (integer) of `address` under `prefixLength`. + networkOf = address: prefixLength: builtins.bitAnd address (netMask prefixLength); + + # "192.168.10.1" /24 -> "192.168.10.0/24" + cidr = address: prefixLength: "${format (networkOf (parse address) prefixLength)}/${toString prefixLength}"; + + inSubnet = + { address, prefixLength }: ip: networkOf ip prefixLength == networkOf address prefixLength; + + # Whether two subnets share any address (one contains the other's network). + overlaps = + a: b: + let + p = lib.min a.prefixLength b.prefixLength; + in + networkOf a.address p == networkOf b.address p; + }; +} diff --git a/modules/router/options.nix b/modules/router/options.nix index 1ffd4aa..a5b799d 100644 --- a/modules/router/options.nix +++ b/modules/router/options.nix @@ -1,6 +1,7 @@ { lib, ... }: let inherit (lib) mkOption mkEnableOption types; + rlib = import ./lib.nix { inherit lib; }; in { options.router = { @@ -18,55 +19,65 @@ in default = { }; description = "LAN segments served by this router, keyed by name."; type = types.attrsOf ( - types.submodule { - options = { - interface = mkOption { - type = types.str; - example = "eth1"; - description = "Interface (or bridge/VLAN device) carrying this LAN."; - }; - - ipv4 = { - address = mkOption { + types.submodule ( + { config, ... }: + { + options = { + interface = mkOption { type = types.str; - example = "192.168.10.1"; - description = "Router address on this LAN."; + example = "eth1"; + description = "Interface (or bridge/VLAN device) carrying this LAN."; }; - prefixLength = mkOption { - type = types.ints.between 8 30; - default = 24; - }; - subnet = mkOption { - type = types.str; - example = "192.168.10.0/24"; - description = "Network in CIDR notation; must contain `address`."; - }; - }; - dhcp = { - enable = mkOption { + ipv4 = { + address = mkOption { + type = types.str; + example = "192.168.10.1"; + description = "Router address on this LAN."; + }; + prefixLength = mkOption { + type = types.ints.between 8 30; + default = 24; + }; + subnet = mkOption { + type = types.str; + readOnly = true; + description = "Network in CIDR notation, derived from `address` and `prefixLength`."; + }; + }; + + dhcp = { + enable = mkOption { + type = types.bool; + default = true; + }; + pool = { + start = mkOption { + type = types.str; + example = "192.168.10.100"; + }; + end = mkOption { + type = types.str; + example = "192.168.10.199"; + }; + }; + }; + + allowManagement = mkOption { type = types.bool; default = true; - }; - pool = { - start = mkOption { - type = types.str; - example = "192.168.10.100"; - }; - end = mkOption { - type = types.str; - example = "192.168.10.199"; - }; + description = "Whether clients on this LAN may reach SSH on the router."; }; }; - allowManagement = mkOption { - type = types.bool; - default = true; - description = "Whether clients on this LAN may reach SSH on the router."; - }; - }; - } + # "invalid" is only ever seen when assertions.nix is already failing. + config.ipv4.subnet = + if rlib.ipv4.parse config.ipv4.address == null then + "invalid" + else + rlib.ipv4.cidr config.ipv4.address config.ipv4.prefixLength; + } + ) ); }; diff --git a/tests/assertions.nix b/tests/assertions.nix new file mode 100644 index 0000000..6a24f2e --- /dev/null +++ b/tests/assertions.nix @@ -0,0 +1,108 @@ +# Evaluation-time tests: broken configs must fail with exactly the expected +# assertion messages, and a valid one with none. +{ self, pkgs }: +let + inherit (pkgs) lib; + + evalRouter = + router: + (import "${pkgs.path}/nixos/lib/eval-config.nix" { + system = "x86_64-linux"; + modules = [ + self.nixosModules.router + { + boot.loader.grub.enable = false; + fileSystems."/" = { + device = "x"; + fsType = "ext4"; + }; + system.stateVersion = "25.05"; + router = { + enable = true; + wan.interface = "eth0"; + } // router; + } + ]; + }).config; + + lan = overrides: lib.recursiveUpdate { + interface = "eth1"; + ipv4.address = "192.168.10.1"; + dhcp.pool = { start = "192.168.10.100"; end = "192.168.10.199"; }; + } overrides; + + failed = router: map (a: a.message) (lib.filter (a: !a.assertion) (evalRouter router).assertions); + + cases = { + valid = { router.lans.main = lan { }; expect = [ ]; }; + noManagement = { + router.lans.main = lan { allowManagement = false; }; + expect = [ "locked out" ]; + }; + wanIsLan = { + router.lans.main = lan { interface = "eth0"; }; + expect = [ "WAN interface `eth0` is also used by a LAN" ]; + }; + sharedInterface = { + router.lans = { + a = lan { }; + b = lan { ipv4.address = "192.168.20.1"; dhcp.pool = { start = "192.168.20.100"; end = "192.168.20.199"; }; }; + }; + expect = [ "two LANs share the same interface" ]; + }; + badAddress = { + router.lans.main = lan { ipv4.address = "192.168.10.300"; dhcp.enable = false; }; + expect = [ "`192.168.10.300` is not a valid IPv4 address" ]; + }; + poolOutsideSubnet = { + router.lans.main = lan { dhcp.pool = { start = "192.168.11.100"; end = "192.168.11.199"; }; }; + expect = [ "pool is not inside 192.168.10.0/24" ]; + }; + poolReversed = { + router.lans.main = lan { dhcp.pool = { start = "192.168.10.199"; end = "192.168.10.100"; }; }; + expect = [ "starts after it ends" ]; + }; + poolContainsRouter = { + router.lans.main = lan { dhcp.pool = { start = "192.168.10.1"; end = "192.168.10.50"; }; }; + expect = [ "contains the router's own address 192.168.10.1" ]; + }; + overlappingLans = { + router.lans = { + a = lan { }; + b = lan { interface = "eth2"; ipv4.address = "192.168.10.2"; }; + }; + expect = [ "`a` (192.168.10.0/24) and `b` (192.168.10.0/24) overlap" ]; + }; + nonOverlappingLans = { + router.lans = { + a = lan { }; + b = lan { interface = "eth2"; ipv4.address = "192.168.20.1"; dhcp.pool = { start = "192.168.20.100"; end = "192.168.20.199"; }; }; + }; + expect = [ ]; + }; + }; + + problems = lib.concatLists ( + lib.mapAttrsToList ( + name: c: + let + got = failed c.router; + missing = lib.filter (e: !lib.any (m: lib.hasInfix e m) got) c.expect; + in + lib.optional (missing != [ ] || lib.length got != lib.length c.expect) + "${name}: expected ${builtins.toJSON c.expect}, got ${builtins.toJSON got}" + ) cases + ); + + subnet = (evalRouter { lans.main = lan { ipv4.prefixLength = 20; ipv4.address = "172.16.37.1"; dhcp.enable = false; }; }).router.lans.main.ipv4.subnet; + subnetProblem = lib.optional (subnet != "172.16.32.0/20") "derived subnet: expected 172.16.32.0/20, got ${subnet}"; +in +pkgs.runCommand "router-assertion-tests" { } ( + if problems ++ subnetProblem == [ ] then + "touch $out" + else + '' + printf '%s\n' ${lib.escapeShellArgs (problems ++ subnetProblem)} >&2 + exit 1 + '' +) diff --git a/tests/basic.nix b/tests/basic.nix index 63b80b5..d2bdedf 100644 --- a/tests/basic.nix +++ b/tests/basic.nix @@ -62,7 +62,7 @@ pkgs.testers.runNixOSTest { wan.interface = "eth1"; lans.main = { interface = "eth2"; - ipv4 = { address = "192.168.10.1"; subnet = "192.168.10.0/24"; }; + ipv4 = { address = "192.168.10.1"; }; dhcp.pool = { start = "192.168.10.100"; end = "192.168.10.199"; }; }; dns.forwarders = [ "203.0.113.1" ]; diff --git a/tests/default.nix b/tests/default.nix index f148b6b..722a724 100644 --- a/tests/default.nix +++ b/tests/default.nix @@ -1,4 +1,6 @@ { self, pkgs }: { basic = import ./basic.nix { inherit self pkgs; }; + lib = import ./lib.nix { inherit pkgs; }; + assertions = import ./assertions.nix { inherit self pkgs; }; } diff --git a/tests/lib.nix b/tests/lib.nix new file mode 100644 index 0000000..e8d5486 --- /dev/null +++ b/tests/lib.nix @@ -0,0 +1,60 @@ +# Unit tests for modules/router/lib.nix, run at evaluation time. +{ pkgs }: +let + inherit (pkgs) lib; + r = import ../modules/router/lib.nix { inherit lib; }; + inherit (r) ipv4; + ip = ipv4.parse; + + failures = lib.runTests { + testParse = { expr = ip "192.168.10.1"; expected = 3232238081; }; + testParseZero = { expr = ip "0.0.0.0"; expected = 0; }; + testParseMax = { expr = ip "255.255.255.255"; expected = 4294967295; }; + testParseBadOctet = { expr = ip "192.168.10.256"; expected = null; }; + testParseShort = { expr = ip "192.168.10"; expected = null; }; + testParseJunk = { expr = ip "192.168.10.x"; expected = null; }; + testFormatRoundTrip = { expr = ipv4.format (ip "10.20.30.40"); expected = "10.20.30.40"; }; + testFormatZero = { expr = ipv4.format 0; expected = "0.0.0.0"; }; + testNetMask24 = { expr = ipv4.format (ipv4.netMask 24); expected = "255.255.255.0"; }; + testNetMask20 = { expr = ipv4.format (ipv4.netMask 20); expected = "255.255.240.0"; }; + testNetMask8 = { expr = ipv4.format (ipv4.netMask 8); expected = "255.0.0.0"; }; + testCidr24 = { expr = ipv4.cidr "192.168.10.77" 24; expected = "192.168.10.0/24"; }; + testCidr20 = { expr = ipv4.cidr "172.16.37.1" 20; expected = "172.16.32.0/20"; }; + testCidr30 = { expr = ipv4.cidr "10.0.0.6" 30; expected = "10.0.0.4/30"; }; + testInSubnetYes = { + expr = ipv4.inSubnet { address = ip "192.168.10.1"; prefixLength = 24; } (ip "192.168.10.200"); + expected = true; + }; + testInSubnetNo = { + expr = ipv4.inSubnet { address = ip "192.168.10.1"; prefixLength = 24; } (ip "192.168.11.1"); + expected = false; + }; + testOverlapNested = { + expr = ipv4.overlaps + { address = ip "10.0.0.1"; prefixLength = 8; } + { address = ip "10.1.2.3"; prefixLength = 24; }; + expected = true; + }; + testOverlapSame = { + expr = ipv4.overlaps + { address = ip "192.168.10.1"; prefixLength = 24; } + { address = ip "192.168.10.9"; prefixLength = 24; }; + expected = true; + }; + testOverlapDisjoint = { + expr = ipv4.overlaps + { address = ip "192.168.10.1"; prefixLength = 24; } + { address = ip "192.168.11.1"; prefixLength = 24; }; + expected = false; + }; + }; +in +pkgs.runCommand "router-lib-tests" { } ( + if failures == [ ] then + "touch $out" + else + '' + echo ${lib.escapeShellArg (lib.generators.toPretty { } failures)} >&2 + exit 1 + '' +)