amadan
Isolation probe: check a resolver is not the box's own address root-side, from procfs (#126)
After v0.25.0 rolled, the isolation probe reported ready: false on every dev box, so nested-by-default never fired. The one failing line was resolver-ports fail interface addresses: route ip+net: netlinkrib: address family not supported by protocol The probe listed the box's interface addresses with net.InterfaceAddrs, which is netlink, and carlos-instance-probe.service (the same namespace text as every instance unit) has RestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX. The listing could never succeed there. The rule it carried, "an allowed resolver must not be one of the box's own addresses", now lives where the allow is written: the control service (resolverAllow, allowResolvers in internal/instancectl). That service is AF_UNIX only, so it does not use netlink either: it reads /proc/net/fib_trie (every IPv4 LOCAL route, including whole local ranges such as 127.0.0.0/8) and /proc/net/if_inet6. A nameserver inside a local prefix is left out of IPAddressAllow= and reported as a failing resolver-local line folded into the probe report, so the box stays not ready, as before. If the addresses cannot be read, the allow is reset to empty and resolver-allow fails: an allow that might name the box is never written. The probe's resolver-ports line keeps what it can test from inside: an allowed resolver does not answer TCP 22. No unit changes: neither the instance templates nor the probe unit gain AF_NETLINK, and the control unit stays AF_UNIX. The same failure cannot hit instance units at runtime: a new test walks every platform package instanceexec builds from (the probe, the trampoline and its exit hook, and instancectl) and refuses net.Interfaces/InterfaceAddrs/InterfaceBy*, syscall.NetlinkRIB and any AF_* constant outside AF_INET, AF_INET6, AF_UNIX. It fails on main (probe.go: net.InterfaceAddrs). Third-party code was checked by hand: the only such call is google/uuid's net.Interfaces, reached only for v1/v6 UUIDs, which nothing here makes. The binary is CGO_ENABLED=0, so DNS does not go through glibc's netlink use either.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
10 files changed,
+484
−72
docs/superpowers/specs/2026-09-11-isolated-exec-design.md+11 −4internal/instancectl/hardening.go+52 −17internal/instancectl/hardening_test.go+163 −6internal/instancectl/localaddrs.go+127 −0internal/instancectl/server.go+16 −1internal/instanceexec/families_test.go+107 −0internal/instanceexec/probe.go+0 −20internal/instanceexec/probe_hardening.go+6 −12internal/instanceexec/probe_hardening_test.go+2 −8internal/instanceexec/probe_test.go+0 −4
CI passed — run details
diff --git a/docs/superpowers/specs/2026-09-11-isolated-exec-design.md b/docs/superpowers/specs/2026-09-11-isolated-exec-design.md| index f1a1cb222..e6e8670bb 100644 |
| --- a/docs/superpowers/specs/2026-09-11-isolated-exec-design.md |
| +++ b/docs/superpowers/specs/2026-09-11-isolated-exec-design.md |
| @@ -2101,10 +2101,17 @@ has a probe line, so `ready` reflects it): |
| service re-opens the nameservers in that same file with |
| `IPAddressAllow=` on `carlos-instances.slice`. Link-local resolvers |
| are refused except 169.254.169.253: an allow carries no zone, so |
| - `fe80::1` would open that address on every link. The probe's `dns` |
| - and `resolver-ports` lines check that each allowed resolver is |
| - reachable, is not an address of the box, and does not answer on |
| - port 22. *Residuals:* an allowed upstream is open on every port (it |
| + `fe80::1` would open that address on every link. A nameserver that |
| + is one of the box's own addresses is never opened either: the |
| + control service reads them from procfs (`/proc/net/fib_trie` LOCAL |
| + routes, `/proc/net/if_inet6`) as it writes the allow, closes every |
| + resolver if it cannot, and reports a refusal as a failing |
| + `resolver-local` line. The probe's `dns` and `resolver-ports` lines |
| + check from inside that each allowed resolver is reachable and does |
| + not answer on port 22. (The probe made the local-address check |
| + itself until v0.25.0, with `net.InterfaceAddrs`; that is netlink, |
| + which the unit's `RestrictAddressFamilies=` refuses, so no box was |
| + ever ready; #126.) *Residuals:* an allowed upstream is open on every port (it |
| is a remote resolver, not the box), and a resolver change reaches |
| running instances only at their next start. Loopback inside one instance is |
| closed too, so a family that talks to itself over TCP loopback must |
diff --git a/internal/instancectl/hardening.go b/internal/instancectl/hardening.go| index 17f452979..2653c9902 100644 |
| --- a/internal/instancectl/hardening.go |
| +++ b/internal/instancectl/hardening.go |
| @@ -156,9 +156,10 @@ func ParseResolvers(b []byte) []netip.Addr { |
| // refused and the probe's dns line says so. |
| // |
| // Everything else is opened as a single address: the VPC resolver (VPC |
| -// base + 2, inside RFC 1918), fd00:ec2::253, or a public one. That the |
| -// address is not one of the box's own is checked by the probe, which can |
| -// see the interfaces; this service cannot (AF_UNIX only). |
| +// base + 2, inside RFC 1918), fd00:ec2::253, or a public one, unless it |
| +// is one of the box's own addresses, which only the box can say: |
| +// resolverAllow refuses those against localPrefixes, here, where the |
| +// allow is written (#126). |
| func ResolverAllowed(a netip.Addr) (bool, string) { |
| switch { |
| case !a.IsValid(): |
| @@ -181,22 +182,27 @@ func ResolverAllowed(a netip.Addr) (bool, string) { |
| // resolverAllow is the IPAddressAllow= value for a resolv.conf: each |
| // allowed nameserver as a single-address prefix, sorted and deduplicated |
| -// so the drop-in systemd writes does not churn. |
| -func resolverAllow(b []byte) []string { |
| +// so the drop-in systemd writes does not churn. A nameserver inside one |
| +// of the box's local prefixes is left out, and named in refused. |
| +func resolverAllow(b []byte, local []netip.Prefix) (allow, refused []string) { |
| seen := map[string]bool{} |
| - var out []string |
| for _, a := range ParseResolvers(b) { |
| if ok, _ := ResolverAllowed(a); !ok { |
| continue |
| } |
| p := netip.PrefixFrom(a, a.BitLen()).String() |
| - if !seen[p] { |
| - seen[p] = true |
| - out = append(out, p) |
| + if seen[p] { |
| + continue |
| + } |
| + seen[p] = true |
| + if lp, ok := isLocal(a, local); ok { |
| + refused = append(refused, fmt.Sprintf("%s is an address of this box (%s)", a, lp)) |
| + continue |
| } |
| + allow = append(allow, p) |
| } |
| - sort.Strings(out) |
| - return out |
| + sort.Strings(allow) |
| + return allow, refused |
| } |
| // ResolvedConf is systemd-resolved's list of its upstreams. When it |
| @@ -221,21 +227,50 @@ func (s *Server) resolverFile() string { |
| // a resolver that left the file is closed again. It runs before every |
| // probe, so the allow is in place before the first start and follows a |
| // resolver change within one probe period; the probe then checks the |
| -// result from inside. |
| -func (s *Server) allowResolvers(ctx context.Context) error { |
| +// result from inside. It returns the nameservers it refused for being |
| +// the box's own. When the box's addresses cannot be read, every resolver |
| +// is closed and the error returned: an allow that might name the box |
| +// itself is never written. |
| +func (s *Server) allowResolvers(ctx context.Context) (refused []string, err error) { |
| b, err := os.ReadFile(s.resolverFile()) |
| if err != nil { |
| - return err |
| + return nil, err |
| + } |
| + var allow []string |
| + local, lerr := s.localPrefixes() |
| + if lerr == nil { |
| + allow, refused = resolverAllow(b, local) |
| } |
| args := []string{"set-property", "--runtime", InstancesSlice, "IPAddressAllow="} |
| - if allow := resolverAllow(b); len(allow) > 0 { |
| + if len(allow) > 0 { |
| args = append(args, "IPAddressAllow="+strings.Join(allow, " ")) |
| } |
| out, err := s.Systemctl(ctx, args...) |
| if err != nil { |
| - return fmt.Errorf("%s: %s", strings.TrimSpace(out), err) |
| + return nil, fmt.Errorf("%s: %s", strings.TrimSpace(out), err) |
| } |
| - return nil |
| + if lerr != nil { |
| + return nil, fmt.Errorf("this box's addresses: %v; every resolver is closed", lerr) |
| + } |
| + return refused, nil |
| +} |
| + |
| +// localPrefixes is LocalAddrs, refusing when there is none. |
| +func (s *Server) localPrefixes() ([]netip.Prefix, error) { |
| + if s.LocalAddrs == nil { |
| + return nil, errors.New("no way to list them") |
| + } |
| + return s.LocalAddrs() |
| +} |
| + |
| +// resolverLocalLine is the verdict on allowResolvers' refusals, folded |
| +// into the probe's report: the probe cannot make this check from inside |
| +// its unit, which cannot list the box's addresses (#126). |
| +func resolverLocalLine(refused []string) VerdictLine { |
| + if len(refused) > 0 { |
| + return VerdictLine{Check: "resolver-local", Verdict: "fail", Detail: strings.Join(refused, "; ")} |
| + } |
| + return VerdictLine{Check: "resolver-local", Verdict: "ok"} |
| } |
| // The data disk (#214 item 1). Every instance's directory, the registry, |
diff --git a/internal/instancectl/hardening_test.go b/internal/instancectl/hardening_test.go| index 0a0db345c..6048a214a 100644 |
| --- a/internal/instancectl/hardening_test.go |
| +++ b/internal/instancectl/hardening_test.go |
| @@ -2,6 +2,7 @@ package instancectl |
| import ( |
| "context" |
| + "errors" |
| "fmt" |
| "net/http" |
| "net/netip" |
| @@ -68,10 +69,13 @@ func TestResolverAllowPolicy(t *testing.T) { |
| conf := []byte("nameserver 172.31.0.2\nnameserver 127.0.0.53\nnameserver 127.0.0.54\nnameserver 127.0.0.1\nnameserver ::1\n" + |
| "nameserver 169.254.169.254\nnameserver 169.254.169.253\nnameserver 169.254.1.1\nnameserver fd00:ec2::253\n" + |
| "nameserver fe80::1%eth0\nnameserver 0.0.0.0\nnameserver 172.31.0.2\noptions edns0\n") |
| - got := strings.Join(resolverAllow(conf), " ") |
| - if want := "169.254.169.253/32 172.31.0.2/32 fd00:ec2::253/128"; got != want { |
| + allow, refused := resolverAllow(conf, boxPrefixes) |
| + if got, want := strings.Join(allow, " "), "169.254.169.253/32 172.31.0.2/32 fd00:ec2::253/128"; got != want { |
| t.Errorf("allow = %q, want %q", got, want) |
| } |
| + if len(refused) != 0 { |
| + t.Errorf("refused = %q", refused) |
| + } |
| // review F1: resolved's stubs are loopback, and an allow on them is |
| // an allow on every wildcard listener of the box. |
| for _, stub := range []string{"127.0.0.53", "127.0.0.54"} { |
| @@ -92,8 +96,8 @@ func TestAllowFollowsTheFileInstancesSee(t *testing.T) { |
| t.Fatal(err) |
| } |
| ctl := &fakeCtl{} |
| - s := &Server{Systemctl: ctl.run, Logf: t.Logf, ResolvConf: etc, ResolvedConf: up} |
| - if err := s.allowResolvers(context.Background()); err != nil { |
| + s := &Server{Systemctl: ctl.run, Logf: t.Logf, ResolvConf: etc, ResolvedConf: up, LocalAddrs: boxAddrs} |
| + if _, err := s.allowResolvers(context.Background()); err != nil { |
| t.Fatal(err) |
| } |
| if got := ctl.joined(); got != "set-property --runtime carlos-instances.slice IPAddressAllow=" { |
| @@ -104,7 +108,7 @@ func TestAllowFollowsTheFileInstancesSee(t *testing.T) { |
| } |
| ctl2 := &fakeCtl{} |
| s.Systemctl = ctl2.run |
| - if err := s.allowResolvers(context.Background()); err != nil { |
| + if _, err := s.allowResolvers(context.Background()); err != nil { |
| t.Fatal(err) |
| } |
| if got := ctl2.joined(); got != "set-property --runtime carlos-instances.slice IPAddressAllow= IPAddressAllow=10.0.0.2/32" { |
| @@ -121,7 +125,11 @@ func TestProbeRunReopensTheResolversFirst(t *testing.T) { |
| t.Fatal(err) |
| } |
| ctl := &fakeCtl{} |
| - s, _ := newServer(t, ctl, func(s *Server) { s.ResolvConf = resolv; s.ExeRoot = filepath.Join(dir, "exe") }) |
| + s, _ := newServer(t, ctl, func(s *Server) { |
| + s.ResolvConf = resolv |
| + s.ExeRoot = filepath.Join(dir, "exe") |
| + s.LocalAddrs = boxAddrs |
| + }) |
| s.RunProbe(context.Background()) |
| calls := ctl.joined() |
| want := "set-property --runtime carlos-instances.slice IPAddressAllow= IPAddressAllow=10.0.0.2/32\nstart carlos-instance-probe.service" |
| @@ -133,6 +141,155 @@ func TestProbeRunReopensTheResolversFirst(t *testing.T) { |
| } |
| } |
| +// A box on EC2: its own VPC address, loopback as a local range, and the |
| +// v6 loopback and link-local. |
| +var boxPrefixes = []netip.Prefix{ |
| + netip.MustParsePrefix("127.0.0.0/8"), netip.MustParsePrefix("172.31.7.19/32"), |
| + netip.MustParsePrefix("::1/128"), netip.MustParsePrefix("fe80::4a1:2ff:fe3b:5c7d/128"), |
| +} |
| + |
| +func boxAddrs() ([]netip.Prefix, error) { return boxPrefixes, nil } |
| + |
| +// The box's own address is never opened as a resolver, however it got |
| +// into resolv.conf (#214 item 3, review F1). The control service makes |
| +// this check where it writes the allow, from procfs: the probe used to |
| +// make it from inside its unit, where listing interfaces takes netlink |
| +// and is refused (#126). |
| +func TestResolverAllowRefusesTheBoxsOwnAddress(t *testing.T) { |
| + conf := []byte("nameserver 172.31.7.19\nnameserver 172.31.0.2\nnameserver 10.9.0.53\n") |
| + local := append([]netip.Prefix{netip.MustParsePrefix("10.9.0.0/24")}, boxPrefixes...) |
| + allow, refused := resolverAllow(conf, local) |
| + if got := strings.Join(allow, " "); got != "172.31.0.2/32" { |
| + t.Errorf("allow = %q", got) |
| + } |
| + if got := strings.Join(refused, "; "); got != "172.31.7.19 is an address of this box (172.31.7.19/32); 10.9.0.53 is an address of this box (10.9.0.0/24)" { |
| + t.Errorf("refused = %q", got) |
| + } |
| +} |
| + |
| +// A refused resolver is a failing line in the report, so the box is not |
| +// ready, whatever the probe says; and the allow written leaves it out. |
| +func TestProbeRunFailsOnALocalResolver(t *testing.T) { |
| + dir := t.TempDir() |
| + resolv := filepath.Join(dir, "resolv.conf") |
| + if err := os.WriteFile(resolv, []byte("nameserver 172.31.7.19\n"), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + ctl := &fakeCtl{} |
| + s, _ := newServer(t, ctl, func(s *Server) { s.ResolvConf = resolv; s.LocalAddrs = boxAddrs }) |
| + probeOK(t, s) |
| + if got := ctl.joined(); !strings.HasPrefix(got, "set-property --runtime carlos-instances.slice IPAddressAllow=\n") { |
| + t.Errorf("calls:\n%s", got) |
| + } |
| + rep, ready := s.Ready() |
| + if ready || rep.Ready { |
| + t.Errorf("ready with a local resolver: %+v", rep) |
| + } |
| + if l := rep.Lines[len(rep.Lines)-1]; l.Check != "resolver-local" || l.Verdict != "fail" || !strings.Contains(l.Detail, "172.31.7.19 is an address of this box") { |
| + t.Errorf("last line = %+v", l) |
| + } |
| + |
| + // And a good resolver is an ok line. |
| + if err := os.WriteFile(resolv, []byte("nameserver 172.31.0.2\n"), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + probeOK(t, s) |
| + rep, ready = s.Ready() |
| + if l := rep.Lines[len(rep.Lines)-1]; !ready || l.Check != "resolver-local" || l.Verdict != "ok" { |
| + t.Errorf("ready=%v report=%+v", ready, rep) |
| + } |
| +} |
| + |
| +// When the box's addresses cannot be read, nothing is opened: the allow |
| +// is reset to empty and the probe does not run. |
| +func TestAllowClosesWhenTheBoxsAddressesCannotBeRead(t *testing.T) { |
| + dir := t.TempDir() |
| + resolv := filepath.Join(dir, "resolv.conf") |
| + if err := os.WriteFile(resolv, []byte("nameserver 172.31.0.2\n"), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + for name, local := range map[string]func() ([]netip.Prefix, error){ |
| + "error": func() ([]netip.Prefix, error) { return nil, errors.New("no procfs") }, |
| + "nil": nil, |
| + } { |
| + ctl := &fakeCtl{} |
| + s, _ := newServer(t, ctl, func(s *Server) { s.ResolvConf = resolv; s.LocalAddrs = local }) |
| + probeOK(t, s) |
| + if got := ctl.joined(); got != "set-property --runtime carlos-instances.slice IPAddressAllow=" { |
| + t.Errorf("%s: calls:\n%s", name, got) |
| + } |
| + rep, ready := s.Ready() |
| + if ready || len(rep.Lines) != 1 || rep.Lines[0].Check != "resolver-allow" || !strings.Contains(rep.Lines[0].Detail, "every resolver is closed") { |
| + t.Errorf("%s: ready=%v report=%+v", name, ready, rep) |
| + } |
| + } |
| +} |
| + |
| +func TestParseTheBoxsAddresses(t *testing.T) { |
| + dir := t.TempDir() |
| + fib := filepath.Join(dir, "fib_trie") |
| + v6 := filepath.Join(dir, "if_inet6") |
| + // An EC2 box: /proc/net/fib_trie (Main and Local tables) and |
| + // /proc/net/if_inet6, trimmed. |
| + if err := os.WriteFile(fib, []byte(`Main: |
| + +-- 0.0.0.0/0 3 0 5 |
| + |-- 0.0.0.0 |
| + /0 universe UNICAST |
| + +-- 127.0.0.0/8 2 0 2 |
| + +-- 127.0.0.0/31 1 0 0 |
| + |-- 127.0.0.0 |
| + /8 host LOCAL |
| + |-- 127.0.0.1 |
| + /32 host LOCAL |
| + |-- 127.255.255.255 |
| + /32 link BROADCAST |
| + +-- 172.31.0.0/20 2 0 2 |
| + |-- 172.31.0.0 |
| + /20 link UNICAST |
| + |-- 172.31.7.19 |
| + /32 host LOCAL |
| + |-- 172.31.15.255 |
| + /32 link BROADCAST |
| +Local: |
| + +-- 0.0.0.0/0 3 0 5 |
| + |-- 172.31.7.19 |
| + /32 host LOCAL |
| +`), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.WriteFile(v6, []byte("00000000000000000000000000000001 01 80 10 80 lo\nfe8000000000000004a102fffe3b5c7d 02 40 20 80 ens5\n"), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + got, err := localPrefixes(fib, v6) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if s := fmt.Sprint(got); s != "[127.0.0.0/8 127.0.0.1/32 172.31.7.19/32 172.31.7.19/32 ::1/128 fe80::4a1:2ff:fe3b:5c7d/128]" { |
| + t.Errorf("local = %s", s) |
| + } |
| + // The resolver, the subnet's broadcast and network addresses are not |
| + // local; anything on loopback is. |
| + for a, want := range map[string]bool{"172.31.0.2": false, "172.31.15.255": false, "172.31.0.0": false, "127.0.0.53": true, "172.31.7.19": true} { |
| + if _, ok := isLocal(netip.MustParseAddr(a), got); ok != want { |
| + t.Errorf("isLocal(%s) = %v", a, ok) |
| + } |
| + } |
| + // No IPv6 in the kernel is no v6 address; a fib_trie with no LOCAL |
| + // route is not one this parser knows, and is refused. |
| + if _, err := localPrefixes(fib, filepath.Join(dir, "absent")); err != nil { |
| + t.Errorf("no if_inet6: %v", err) |
| + } |
| + if err := os.WriteFile(fib, []byte("Main:\n +-- 0.0.0.0/0 3 0 5\n"), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := localPrefixes(fib, v6); err == nil { |
| + t.Error("a fib_trie with no LOCAL route was read as no addresses") |
| + } |
| + if _, err := localPrefixes(filepath.Join(dir, "absent"), v6); err == nil { |
| + t.Error("no fib_trie was read as no addresses") |
| + } |
| +} |
| + |
| func TestDiskFloor(t *testing.T) { |
| if f := DiskFloor(8 << 30); f != 1<<30 { |
| t.Errorf("8 GiB: %d", f) |
diff --git a/internal/instancectl/localaddrs.go b/internal/instancectl/localaddrs.go| new file mode 100644 |
| index 000000000..7b152dd00 |
| --- /dev/null |
| +++ b/internal/instancectl/localaddrs.go |
| @@ -0,0 +1,127 @@ |
| +package instancectl |
| + |
| +import ( |
| + "bufio" |
| + "bytes" |
| + "encoding/hex" |
| + "errors" |
| + "fmt" |
| + "io/fs" |
| + "net/netip" |
| + "os" |
| + "strconv" |
| + "strings" |
| +) |
| + |
| +// The box's own addresses, for the resolver allow (#214 item 3, #126). |
| +// An allowed resolver must not be one of them: the allow opens an address |
| +// on every port, and on the box's own address that is every listener |
| +// bound to all addresses. |
| +// |
| +// They are read from procfs, not asked of the kernel over netlink, which |
| +// is what net.InterfaceAddrs does: this service runs with |
| +// RestrictAddressFamilies=AF_UNIX, and the probe unit, which used to make |
| +// this check, with AF_INET AF_INET6 AF_UNIX, so netlink is refused in |
| +// both (the v0.25.0 roll: "netlinkrib: address family not supported by |
| +// protocol" on every box, and no box ever ready). A file read needs no |
| +// socket at all, and widening either unit's families for one listing was |
| +// the alternative. |
| + |
| +// Where the box's addresses are read; tests point them at fixtures. |
| +const ( |
| + fibTriePath = "/proc/net/fib_trie" // IPv4: the routing tables, LOCAL entries |
| + ifInet6Path = "/proc/net/if_inet6" // IPv6: one line per interface address |
| +) |
| + |
| +// localPrefixes is everything that routes to this box: each IPv4 LOCAL |
| +// route (an interface address, or a whole local range such as |
| +// 127.0.0.0/8), and each IPv6 interface address as a /128. |
| +func localPrefixes(fibTrie, ifInet6 string) ([]netip.Prefix, error) { |
| + b, err := os.ReadFile(fibTrie) |
| + if err != nil { |
| + return nil, err |
| + } |
| + out, err := parseFibTrie(b) |
| + if err != nil { |
| + return nil, fmt.Errorf("%s: %w", fibTrie, err) |
| + } |
| + b, err = os.ReadFile(ifInet6) |
| + switch { |
| + case errors.Is(err, fs.ErrNotExist): |
| + // No IPv6 in this kernel: no v6 address can be the box's. |
| + return out, nil |
| + case err != nil: |
| + return nil, err |
| + } |
| + v6, err := parseIfInet6(b) |
| + if err != nil { |
| + return nil, fmt.Errorf("%s: %w", ifInet6, err) |
| + } |
| + return append(out, v6...), nil |
| +} |
| + |
| +// parseFibTrie reads /proc/net/fib_trie: a leaf line "|-- <addr>" is |
| +// followed by one line per route at that address, "/<len> <scope> |
| +// <type>". A type of LOCAL is a destination this box answers for itself. |
| +// A file with no LOCAL entry at all is refused rather than read as "no |
| +// addresses": every box has at least 127.0.0.1, so it is not the file |
| +// this parser knows. |
| +func parseFibTrie(b []byte) ([]netip.Prefix, error) { |
| + var out []netip.Prefix |
| + var leaf netip.Addr |
| + sc := bufio.NewScanner(bytes.NewReader(b)) |
| + for sc.Scan() { |
| + line := strings.TrimSpace(sc.Text()) |
| + if a, ok := strings.CutPrefix(line, "|-- "); ok { |
| + leaf, _ = netip.ParseAddr(strings.TrimSpace(a)) |
| + continue |
| + } |
| + f := strings.Fields(line) |
| + if len(f) < 3 || !strings.HasPrefix(f[0], "/") || f[len(f)-1] != "LOCAL" || !leaf.IsValid() { |
| + continue |
| + } |
| + bits, err := strconv.Atoi(f[0][1:]) |
| + if err != nil { |
| + continue |
| + } |
| + if p, err := leaf.Prefix(bits); err == nil { |
| + out = append(out, p) |
| + } |
| + } |
| + if err := sc.Err(); err != nil { |
| + return nil, err |
| + } |
| + if len(out) == 0 { |
| + return nil, errors.New("no LOCAL route") |
| + } |
| + return out, nil |
| +} |
| + |
| +// parseIfInet6 reads /proc/net/if_inet6: each line starts with the |
| +// address as 32 hex digits. |
| +func parseIfInet6(b []byte) ([]netip.Prefix, error) { |
| + var out []netip.Prefix |
| + sc := bufio.NewScanner(bytes.NewReader(b)) |
| + for sc.Scan() { |
| + f := strings.Fields(sc.Text()) |
| + if len(f) == 0 { |
| + continue |
| + } |
| + raw, err := hex.DecodeString(f[0]) |
| + if err != nil || len(raw) != 16 { |
| + return nil, fmt.Errorf("%q is not an address", f[0]) |
| + } |
| + out = append(out, netip.PrefixFrom(netip.AddrFrom16([16]byte(raw)), 128)) |
| + } |
| + return out, sc.Err() |
| +} |
| + |
| +// isLocal names the local prefix a holds, if any. |
| +func isLocal(a netip.Addr, local []netip.Prefix) (netip.Prefix, bool) { |
| + for _, p := range local { |
| + if p.Contains(a) { |
| + return p, true |
| + } |
| + } |
| + return netip.Prefix{}, false |
| +} |
diff --git a/internal/instancectl/server.go b/internal/instancectl/server.go| index edc87d99a..a0fbd8ccd 100644 |
| --- a/internal/instancectl/server.go |
| +++ b/internal/instancectl/server.go |
| @@ -10,6 +10,7 @@ import ( |
| "log" |
| "net" |
| "net/http" |
| + "net/netip" |
| "os" |
| "os/exec" |
| "path/filepath" |
| @@ -60,6 +61,10 @@ type Server struct { |
| // (#214 item 3). |
| ResolvConf string |
| ResolvedConf string |
| + // LocalAddrs lists what routes to this box itself; a nameserver |
| + // inside it is never opened (#126). Production reads procfs |
| + // (localPrefixes); nil closes every resolver. |
| + LocalAddrs func() ([]netip.Prefix, error) |
| // DataRoot is the instance data tree, and Statfs reports its free and |
| // total bytes; a nil Statfs skips the disk floors (#214 item 1). |
| DataRoot string |
| @@ -117,6 +122,7 @@ func Run(args []string) { |
| ExeRoot: filepath.Join(controlsock.Dir, "exe"), |
| ResolvConf: "/etc/resolv.conf", |
| ResolvedConf: ResolvedConf, |
| + LocalAddrs: func() ([]netip.Prefix, error) { return localPrefixes(fibTriePath, ifInet6Path) }, |
| DataRoot: "/data/instances", |
| Statfs: statfsBytes, |
| DiskEvery: 30 * time.Second, |
| @@ -654,11 +660,14 @@ func (s *Server) RunProbe(ctx context.Context) { |
| return |
| } |
| } |
| + var resolverLines []VerdictLine |
| if s.ResolvConf != "" { |
| - if err := s.allowResolvers(ctx); err != nil { |
| + refused, err := s.allowResolvers(ctx) |
| + if err != nil { |
| set(ProbeReport{Lines: []VerdictLine{{Check: "resolver-allow", Verdict: "fail", Detail: err.Error()}}}) |
| return |
| } |
| + resolverLines = append(resolverLines, resolverLocalLine(refused)) |
| } |
| if out, err := s.Systemctl(ctx, "start", probeUnit); err != nil { |
| set(ProbeReport{Lines: []VerdictLine{{Check: "probe-unit", Verdict: "fail", Detail: strings.TrimSpace(out + " " + err.Error())}}}) |
| @@ -674,6 +683,12 @@ func (s *Server) RunProbe(ctx context.Context) { |
| set(ProbeReport{Lines: []VerdictLine{{Check: "probe-output", Verdict: "fail", Detail: err.Error()}}}) |
| return |
| } |
| + for _, l := range resolverLines { |
| + r.Lines = append(r.Lines, l) |
| + if l.Verdict == "fail" { |
| + r.Ready = false |
| + } |
| + } |
| for _, l := range r.Lines { |
| if l.Verdict != "ok" { |
| s.logf("instance-control: probe: %s %s %s", l.Check, l.Verdict, l.Detail) |
diff --git a/internal/instanceexec/families_test.go b/internal/instanceexec/families_test.go| new file mode 100644 |
| index 000000000..937b5c945 |
| --- /dev/null |
| +++ b/internal/instanceexec/families_test.go |
| @@ -0,0 +1,107 @@ |
| +package instanceexec |
| + |
| +import ( |
| + "go/ast" |
| + "go/build" |
| + "go/parser" |
| + "go/token" |
| + "path" |
| + "path/filepath" |
| + "runtime" |
| + "sort" |
| + "strconv" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +// Every carlos process inside an instance unit (the probe, the trampoline |
| +// and its exit hook, for all three templates) runs under |
| +// RestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX. A socket of any other |
| +// family is refused, so code that asks for one fails on every box: the |
| +// probe's net.InterfaceAddrs, which is netlink, kept every box on v0.25.0 |
| +// from ever being ready (#126). This walks the platform packages this one |
| +// builds from, as linux sees them, and refuses the calls that open one. |
| +// Third-party code is outside the walk; at the time of writing its one |
| +// such call is google/uuid's net.Interfaces, reached only for v1 and v6 |
| +// UUIDs, which nothing here makes. |
| +func TestInstanceCodeOpensOnlyTheUnitsAddressFamilies(t *testing.T) { |
| + const module = "github.com/carlosframework/platform" |
| + root, err := filepath.Abs(filepath.Join("..", "..")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + ctx := build.Default |
| + ctx.GOOS, ctx.GOARCH, ctx.CgoEnabled = "linux", runtime.GOARCH, false |
| + |
| + refused := map[string]map[string]bool{ |
| + "net": {"Interfaces": true, "InterfaceAddrs": true, "InterfaceByName": true, "InterfaceByIndex": true}, |
| + "syscall": {"NetlinkRIB": true}, |
| + "golang.org/x/sys/unix": {}, |
| + } |
| + allowedAF := map[string]bool{"AF_INET": true, "AF_INET6": true, "AF_UNIX": true, "AF_UNSPEC": true} |
| + |
| + var found []string |
| + seen := map[string]bool{} |
| + queue := []string{module + "/internal/instanceexec"} |
| + for len(queue) > 0 { |
| + ip := queue[0] |
| + queue = queue[1:] |
| + if seen[ip] { |
| + continue |
| + } |
| + seen[ip] = true |
| + dir := filepath.Join(root, strings.TrimPrefix(ip, module)) |
| + pkg, err := ctx.ImportDir(dir, 0) |
| + if err != nil { |
| + t.Fatalf("%s: %v", ip, err) |
| + } |
| + for _, imp := range pkg.Imports { |
| + if strings.HasPrefix(imp, module+"/") { |
| + queue = append(queue, imp) |
| + } |
| + } |
| + for _, name := range pkg.GoFiles { |
| + file := filepath.Join(dir, name) |
| + f, err := parser.ParseFile(token.NewFileSet(), file, nil, parser.SkipObjectResolution) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + local := map[string]string{} |
| + for _, is := range f.Imports { |
| + p, _ := strconv.Unquote(is.Path.Value) |
| + n := path.Base(p) |
| + if is.Name != nil { |
| + n = is.Name.Name |
| + } |
| + local[n] = p |
| + } |
| + ast.Inspect(f, func(n ast.Node) bool { |
| + sel, ok := n.(*ast.SelectorExpr) |
| + if !ok { |
| + return true |
| + } |
| + x, ok := sel.X.(*ast.Ident) |
| + if !ok { |
| + return true |
| + } |
| + p := local[x.Name] |
| + names, watched := refused[p] |
| + if !watched { |
| + return true |
| + } |
| + af := strings.HasPrefix(sel.Sel.Name, "AF_") && !allowedAF[sel.Sel.Name] |
| + if names[sel.Sel.Name] || af { |
| + found = append(found, strings.TrimPrefix(file, root+string(filepath.Separator))+": "+p+"."+sel.Sel.Name) |
| + } |
| + return true |
| + }) |
| + } |
| + } |
| + sort.Strings(found) |
| + for _, f := range found { |
| + t.Errorf("%s needs an address family the instance units refuse", f) |
| + } |
| + if !seen[module+"/internal/instancectl"] { |
| + t.Errorf("the walk never reached instancectl: %v", seen) |
| + } |
| +} |
diff --git a/internal/instanceexec/probe.go b/internal/instanceexec/probe.go| index 6317577b2..3e32d42af 100644 |
| --- a/internal/instanceexec/probe.go |
| +++ b/internal/instanceexec/probe.go |
| @@ -8,7 +8,6 @@ import ( |
| "flag" |
| "fmt" |
| "net" |
| - "net/netip" |
| "os" |
| "path/filepath" |
| "sort" |
| @@ -70,9 +69,6 @@ type probeEnv struct { |
| selfCgroup func() (string, error) |
| // ioEnforced says whether io.weight has a controller behind it. |
| ioEnforced func(selfCgroup string) bool |
| - // localAddrs are this box's interface addresses (the network |
| - // namespace is the box's). |
| - localAddrs func() ([]netip.Addr, error) |
| } |
| // cgroupMountpoint is the fixed system path the cgroup-ro-mount check |
| @@ -124,25 +120,9 @@ func productionProbeEnv() probeEnv { |
| abstractScoped: realAbstractCheck, |
| selfCgroup: ownCgroupDir, |
| ioEnforced: ioWeightEnforced, |
| - localAddrs: interfaceAddrs, |
| } |
| } |
| -// interfaceAddrs lists the box's addresses, zones and prefixes dropped. |
| -func interfaceAddrs() ([]netip.Addr, error) { |
| - addrs, err := net.InterfaceAddrs() |
| - if err != nil { |
| - return nil, err |
| - } |
| - out := make([]netip.Addr, 0, len(addrs)) |
| - for _, a := range addrs { |
| - if p, err := netip.ParsePrefix(a.String()); err == nil { |
| - out = append(out, p.Addr().WithZone("").Unmap()) |
| - } |
| - } |
| - return out, nil |
| -} |
| - |
| // ownCgroupDir is /sys/fs/cgroup plus this process's unified-hierarchy |
| // path from /proc/self/cgroup. |
| func ownCgroupDir() (string, error) { |
diff --git a/internal/instanceexec/probe_hardening.go b/internal/instanceexec/probe_hardening.go| index 438b8f110..4437a6eb2 100644 |
| --- a/internal/instanceexec/probe_hardening.go |
| +++ b/internal/instanceexec/probe_hardening.go |
| @@ -74,20 +74,14 @@ func hardeningChecks(e probeEnv, add func(check, verdict, detail string), pass f |
| pass("dns", strings.Join(dnsFail, "; "), len(dnsFail) == 0) |
| // Item 3: an allowed resolver opens only a resolver. The allow is per |
| - // address, both ways and every port, so it must not be an address of |
| - // this box, where a tenant could reach a listener bound to all |
| - // addresses, and a non-DNS port on it must not answer. |
| + // address, both ways and every port, so a non-DNS port on it must not |
| + // answer. That it is not an address of this box is the control |
| + // service's check (resolver-local), made where the allow is written: |
| + // listing the box's addresses takes netlink, which this unit's |
| + // RestrictAddressFamilies= refuses, as it does every instance's |
| + // (#126, the v0.25.0 roll). |
| var portFail []string |
| - local, lerr := e.localAddrs() |
| - if lerr != nil { |
| - portFail = append(portFail, "interface addresses: "+lerr.Error()) |
| - } |
| for _, a := range allowed { |
| - for _, l := range local { |
| - if l == a { |
| - portFail = append(portFail, a.String()+" is an address of this box") |
| - } |
| - } |
| if err := e.connectTCP(netip.AddrPortFrom(a, resolverProbePort).String(), 2*time.Second); err == nil { |
| portFail = append(portFail, fmt.Sprintf("%s: port %d answered", a, resolverProbePort)) |
| } |
diff --git a/internal/instanceexec/probe_hardening_test.go b/internal/instanceexec/probe_hardening_test.go| index 0f83241a2..59b9a8546 100644 |
| --- a/internal/instanceexec/probe_hardening_test.go |
| +++ b/internal/instanceexec/probe_hardening_test.go |
| @@ -5,7 +5,6 @@ package instanceexec |
| import ( |
| "errors" |
| "net" |
| - "net/netip" |
| "os" |
| "path/filepath" |
| "runtime" |
| @@ -183,18 +182,13 @@ func TestScopeAbstractUnixRefusesAnOutsideName(t *testing.T) { |
| } |
| // An allowed resolver must open only a resolver (#214 item 3, review F1): |
| -// one that is an address of this box, or that answers on a non-DNS port, |
| -// fails resolver-ports. |
| +// one that answers on a non-DNS port fails resolver-ports. One that is an |
| +// address of this box is the control service's refusal (resolver-local). |
| func TestProbeResolverPortsLine(t *testing.T) { |
| e := goodEnv(t) |
| if r := runChecks(e); verdict(r, "resolver-ports") != "ok" { |
| t.Fatalf("good fixture: resolver-ports = %s", verdict(r, "resolver-ports")) |
| } |
| - e.localAddrs = func() ([]netip.Addr, error) { return []netip.Addr{netip.MustParseAddr("10.0.0.2")}, nil } |
| - if r := runChecks(e); r.Ready || verdict(r, "resolver-ports") != "fail" { |
| - t.Errorf("resolver on a box address yet ready=%v", r.Ready) |
| - } |
| - e = goodEnv(t) |
| e.connectTCP = func(addr string, _ time.Duration) error { |
| if addr == "10.0.0.2:22" { |
| return nil |
diff --git a/internal/instanceexec/probe_test.go b/internal/instanceexec/probe_test.go| index cef018748..b1ead4c5a 100644 |
| --- a/internal/instanceexec/probe_test.go |
| +++ b/internal/instanceexec/probe_test.go |
| @@ -5,7 +5,6 @@ package instanceexec |
| import ( |
| "errors" |
| "net" |
| - "net/netip" |
| "os" |
| "path/filepath" |
| "strings" |
| @@ -85,9 +84,6 @@ func goodEnv(t *testing.T) probeEnv { |
| abstractScoped: func() error { return nil }, |
| selfCgroup: func() (string, error) { return cg, nil }, |
| ioEnforced: func(string) bool { return true }, |
| - localAddrs: func() ([]netip.Addr, error) { |
| - return []netip.Addr{netip.MustParseAddr("127.0.0.1"), netip.MustParseAddr("10.0.3.17")}, nil |
| - }, |
| } |
| } |