amadan

carlos / platform Public

Merge branch 'fix-probe-resolver-ports' into main

Probe: resolver checks no longer need netlink; the control service reads local addresses from /proc (#126)
claude 3de0b0fca09e7022cfee5d397ffd3f30fdf34578 parent 1da67e6parent ad9139c
10 files changed, +484 −72
  • docs/superpowers/specs/2026-09-11-isolated-exec-design.md +11 −4
  • internal/instancectl/hardening.go +52 −17
  • internal/instancectl/hardening_test.go +163 −6
  • internal/instancectl/localaddrs.go +127 −0
  • internal/instancectl/server.go +16 −1
  • internal/instanceexec/families_test.go +107 −0
  • internal/instanceexec/probe.go +0 −20
  • internal/instanceexec/probe_hardening.go +6 −12
  • internal/instanceexec/probe_hardening_test.go +2 −8
  • internal/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
- },
}
}