Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- **Credentials on the command line.** The production (slim) flavors refuse `-s3AccessKey`, `-s3SecretKey` and the four `-listener*` secret flags with exit 64; development builds warn, now for the S3 flags too and even when the env var is also set. Use the `_FILE` variables.
- **`_FILE` secrets are checked.** A missing or unreadable `*_FILE` path, a non-regular file, or one readable by group or others stops startup with exit 64. It used to be ignored silently (listener tokens) or logged (S3 keys), leaving the credential empty. Mount secrets mode `0400`.
- **The debug writers are build-time opt-ins.** The netlink capture writer (`-writeFiles` / `-capturePath`) is compiled in only with `-tags capture`, which no flavor or image sets; use `nix build .#xtcp2-capture`. The `file:` destination needs `-tags dest_file` and is in the full (development) flavor only, so `xtcp2-<dest>` slim flavors and their images no longer have it. Asking for either on a binary without it exits 64 at startup with the tag to rebuild with; `ConfigService.Set` refuses it with `FailedPrecondition`. A capture build reports `+capture` in its version line, and when armed logs a warning and sets `xtcp2_debug_capture_enabled 1`.
- **`/ip` and the daemons carry no netlink write verbs.** goip's write verbs and `pkg/xtcpnl`'s write builders compile only with `-tags goip_write`, which only `goip` and `goip-parity` are built with. `/ip` on xtcp2 and go-link-monitor answers a write verb 400 as before; an untagged `goip` answers one `ErrWriteNotCompiled` (exit 1), so a `go build ./cmd/goip` that needs them must add the tag. The Nix `goip` and `goip-parity` store names gain `-goip_write`; their binary paths are unchanged. `checks.no-write-encoders` proves the daemons link no write encoder and import no `os/exec` (bar minio-go's credentials provider).

### New

Expand Down
10 changes: 6 additions & 4 deletions cmd/goip/main.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
// Command goip is a read-only Go reimplementation of a subset of ip(8).
// Command goip is a Go reimplementation of a subset of ip(8).
//
// It exists as a coverage test for pkg/xtcpnl, not as an `ip` replacement.
// Cloning `ip` forces the library to exercise every message and attribute a
// real tool needs, and comparing the netlink traffic the two produce turns
// "did we cover this?" into a diff that can fail a build. See
// docs/netlink/coverage-expansion.md.
//
// goip never creates, deletes or sets anything: pkg/xtcpnl's request encoder
// rejects every message type outside RTM_GET* and the NLMSG_* control types,
// so the read-only property is enforced in code rather than by convention.
// goip never applies a write. Its write verbs (add, set, delete, flush,
// restore, …) build ip's request as a dry run, written to GOIP_WRITE_OUT,
// and compile only with -tags goip_write, which the Nix goip build sets.
// Without the tag they are ErrWriteNotCompiled, as they always are in the
// daemons that serve /ip; see docs/build-flavors.md.
package main

import (
Expand Down
12 changes: 12 additions & 0 deletions docs/build-flavors.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,16 @@ nix build .#xtcp2client
nix build .#xtcp2_kafka_client
```

### goip write verbs

`goip` and `goip-parity` are built with `-tags goip_write` (`nix/binaries.nix` → `extraTagsByName`); no other binary is. The tag compiles in goip's write verbs (`ip link add`, `route flush`, `xfrm state add`, …, all dry runs written to `GOIP_WRITE_OUT`) and `pkg/xtcpnl`'s write builders (`BuildWriteRequest`, `BuildGenlWriteRequest`, …). `xtcp2` and `go-link-monitor`, which serve `goip show` at `/ip`, are built without it: `/ip` refuses a write verb by name with 400 as before, and behind that refusal the verb is now `ErrWriteNotCompiled`, with no encoder in the binary to reach. An untagged `go build ./cmd/goip` answers every write verb `ErrWriteNotCompiled`, with exit 1.

One exemption: the `treatment` tag (the full-destination builds, which embed the route treatment agent, and `xtcp2-treatment-agent`/`xtcp2-treatmentctl`) compiles `internal/goip/actuate` and, of the write builders, only the route and rule ones it uses (`BuildWriteRequest`, `BuildRouteWriteRequest`, `BuildRuleWriteRequest`). Link, address, nsid, genl and xfrm builders stay `goip_write`-only, and the slim flavors and `go-link-monitor` carry none.

`nix flake check` runs `no-write-encoders`, which fails if a daemon's (`xtcp2-debug`, `xtcp2-min`, `go-link-monitor`) `go list -deps` names a write file beyond treatment's route and rule files, or `go tool nm` finds a write builder, an `Encode*InfoData` or a goip write arm in it, or if `goip` lacks `BuildWriteRequest`. It also fails if a package in either daemon's deps imports `os/exec`, bar the one it lists (minio-go's credentials provider, in the s3parquet destination).

To test the write path: `go test -tags goip_write ./internal/goip/... ./internal/goipparity/... ./pkg/nlparity/... ./pkg/xtcpnl/...`, or `nix build .#test-focused-goip`. Untagged, `internal/goip/notag_test.go` checks that every write verb is `ErrWriteNotCompiled` and sends nothing.

## OCI images

The three "fat" images carry every cmd binary; the slim images carry only the single matching `xtcp2-<flavor>` binary.
Expand Down Expand Up @@ -251,6 +261,8 @@ The absent ones are published as `0` deliberately: a binary built without the ta
| `destinations_core.go`, `destinations_null.go`, `destinations_stderr.go`, `destinations_udp.go`, `destinations_unix.go`, `destinations_unixgram.go` | (none) | always |
| `destinations_file.go` | `//go:build dest_file` | only with `-tags dest_file` (full flavor) |
| `netlinker_capture.go` / `netlinker_nocapture.go` | `//go:build capture` / `!capture` | the capture writer, or its no-op stub |
| `internal/goip` write files, `write_ops_goip_write.go`; `pkg/xtcpnl` write builders; `pkg/nlparity/nlparity_write.go` | `//go:build goip_write` | `goip` and `goip-parity` only ([goip write verbs](#goip-write-verbs)) |
| `pkg/xtcpnl` route/rule builders (`xtcpnl_{route,rule}_write.go`, `xtcpnl_write_request.go`) | `//go:build goip_write \|\| treatment` | as above, plus `treatment` builds ([goip write verbs](#goip-write-verbs)) |
| `destinations_kafka.go` | `//go:build dest_kafka` | only with `-tags dest_kafka` |
| `destinations_nats.go` | `//go:build dest_nats` | only with `-tags dest_nats` |
| `destinations_nsq.go` | `//go:build dest_nsq` | only with `-tags dest_nsq` |
Expand Down
11 changes: 11 additions & 0 deletions docs/netlink/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,17 @@ add|change|replace|delete|flush`, `goip rule add|delete|flush|restore`,
now do too, and refuse with
`ErrWriteNotApplied` when the hook is unset.

Every write verb, and every write builder in `pkg/xtcpnl`, compiles only with
`-tags goip_write`, which `cmd/goip` and `cmd/goip-parity` alone are built
with. The daemons that serve `goip show` over HTTP (`xtcp2` and
`go-link-monitor`, at `/ip`) are built without it: `/ip` refuses a write verb
by name with 400, and behind that refusal the verb is `ErrWriteNotCompiled`,
with no encoder in the binary. The exception is the `treatment` tag, which
compiles the route and rule builders for the treatment actuator
(`internal/goip/actuate`) and nothing else of the write set.
`checks.no-write-encoders` proves that with `go list -deps` and
`go tool nm`; see [build flavors](../build-flavors.md#goip-write-verbs).

Supported families have expanded beyond those initial paths; the
[coverage tracker](coverage-status.md) records current supported operations.
Coverage of parsing and dry-run requests does not establish live mutation
Expand Down
2 changes: 1 addition & 1 deletion docs/security-hardening/focused-implementation-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ Non-blocking shortfalls found while implementing F1; none blocks R1.
- [x] `internal/goip`: write verbs (`*_write.go`, `*_flush.go`, `*_restore.go`, `link_property.go`, `stats_set.go`, the `obj_*` setters) behind `//go:build goip_write`; `cmd/goip` builds with the tag, `cmd/xtcp2` and `cmd/go-link-monitor` without. *Done as:* a seam first. Every `runX` calls its write arm through `writeOps`, an untagged table of stubs that return `ErrWriteNotCompiled` (an `ErrWriteVerb`); `write_ops_goip_write.go` swaps the real arms in from `init()`. The helpers the read path shares with the write files moved to untagged files first (FP-8a, a pure move), and the `obj_*` setters to `obj_<x>_write.go`. `goip-parity` builds with the tag too.
- [x] `pkg/xtcpnl`: `BuildWriteRequest`, `IsAdmittedWriteType` and the write half of `xtcpnl_rtattr_encode.go` behind the same tag; `pkg/nlparity` write paths likewise. *Done as:* the write builders (`Build*WriteRequest`, `BuildSetNsidRequest`) and their files are tagged; the admission predicates (`IsAdmitted*`) and the `nlparity` classifiers (`IsWrite`, `ParseWriteRequests`) stay untagged, because they classify and encode nothing. The per-kind `Encode*InfoData` stay beside their decoders, unreachable without goip's link write; the nm check proves the linker drops them.
- [x] `goip.ErrWriteVerb` handling in `goiphttp` stays (a write verb in a URL is still a 400), implemented without linking the verbs. Unchanged: `/ip` refuses a write verb by name before it runs.
- [x] Structural test in CI: `go list -deps ./cmd/xtcp2` has no file matching the write set; `go tool nm` on the daemon finds no `BuildWriteRequest`; no `os/exec`. *Done as:* `checks.no-write-encoders`, for xtcp2 and go-link-monitor with each binary's own tags, each proof with a positive control (the listing with `goip_write` names write files; tagged `goip` has `BuildWriteRequest`). A manual negative run (`BuildWriteRequest` untagged and referenced from `goip.Show`) fails it.
- [x] Structural test in CI: `go list -deps ./cmd/xtcp2` has no file matching the write set; `go tool nm` on the daemon finds no `BuildWriteRequest`; no `os/exec`. *Done as:* `checks.no-write-encoders`, for xtcp2 and go-link-monitor with each binary's own tags, each proof with a positive control (the listing with `goip_write` names write files; tagged `goip` has `BuildWriteRequest`). A manual negative run (`BuildWriteRequest` untagged and referenced from `goip.Show`) fails it. After #282 the `treatment` tag (fat builds, the standalone agent and watchdog) compiles `internal/goip/actuate` and the route/rule builders it needs (`goip_write || treatment`); the check exempts exactly those three files and symbols under that tag, fails if any is missing (stale list), and covers `xtcp2-min` (no tag) for files and `no_exec`.

**Tests:** FD `no_write_encoders`, `no_exec`; goip's own suites run with the tag. *Done as:* `internal/goip/notag_test.go` (untagged) runs every write verb against a socket that refuses requests and expects `ErrWriteNotCompiled`, exit 1 and no request; `TestWriteOpsUntagged`/`TestWriteOpsTagged` pin the table either way. Mixed tests split into `*_write_test.go`; shared tables skip a write row untagged. `test-focused-goip` runs goip, goipparity, nlparity and xtcpnl with the tag.
**Status:** open as #297 (FP-8a, moves), #298 (FP-8b, seam, tags, Nix) and #299 (FP-8c, the check and docs), stacked. No `goip_write` leg in `go-test-flavors`, whose legs test `pkg/xtcp` only; `test-focused-goip` carries the tagged tests, and `internal/goipparity`'s tests moved there from the untagged `go test ./...`.
Expand Down
11 changes: 11 additions & 0 deletions nix/checks/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
binaries,
xdp2,
lintBaselineMeasure,
linkmonitor,
}:

let
Expand Down Expand Up @@ -99,6 +100,16 @@ in

# Slim flavors must not carry the debug writers (dest_file, capture).
flavor-tags = import ./flavor-tags.nix { inherit pkgs lib binaries; };
# The daemons carry no netlink write encoders (goip_write) and no os/exec.
no-write-encoders = import ./no-write-encoders.nix {
inherit
pkgs
lib
vendoredSource
binaries
linkmonitor
;
};

# The lint ratchet, and the only check here that is GREEN at baseline. The
# three tier checks above report a tier's whole finding list, which has been
Expand Down
163 changes: 163 additions & 0 deletions nix/checks/no-write-encoders.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
# nix/checks/no-write-encoders.nix
#
# The daemons, xtcp2 and go-link-monitor, must not be able to build a netlink
# mutation: goip's write verbs and pkg/xtcpnl's write builders compile only
# with the goip_write tag, which cmd/goip and cmd/goip-parity alone carry.
#
# One exemption: a build with the `treatment` tag embeds the route treatment
# actuator (internal/goip/actuate), which encodes routes and rules. It may
# carry exactly the route and rule builders (treatmentFiles, treatmentSyms)
# and nothing else of the write set. go-link-monitor and the slim xtcp2
# flavors build without the tag and get no exemption.
#
# Three proofs, each with its positive control so the check cannot pass by
# testing nothing:
#
# files `go list -deps` of each daemon, with the tags its binary was
# built with, names no write file of internal/goip, pkg/xtcpnl or
# pkg/nlparity, bar treatmentFiles under `treatment` (which must
# all be named, or the list is stale). The same listing with
# goip_write must name more.
# symbols `go tool nm` on xtcp2-debug (unstripped, treatment) and
# go-link-monitor finds no write builder, no Encode*InfoData and
# no goip write arm, bar treatmentSyms under `treatment`. goip,
# built with goip_write, must have BuildWriteRequest. The slim
# xtcp2 is stripped, so it gets the files and no_exec proofs only.
# no_exec no package in either daemon's deps imports os/exec, bar the
# third-party ones in execAllowed: the daemons never run a
# program, and a new importer has to be argued for here.
#
{
pkgs,
lib,
vendoredSource,
binaries,
linkmonitor,
}:

let
versions = import ../versions.nix { inherit pkgs; };

daemons = [
{
name = "xtcp2";
pkg = "./cmd/xtcp2";
bin = "${binaries.xtcp2-debug}/bin/xtcp2";
nm = true;
}
{
name = "xtcp2-min";
pkg = "./cmd/xtcp2";
bin = "${binaries.xtcp2-min}/bin/xtcp2";
nm = false;
}
{
name = "go-link-monitor";
pkg = "./cmd/go-link-monitor";
bin = "${linkmonitor}/bin/go-link-monitor";
nm = true;
}
];

# What `treatment` may compile and link of the write set: the route and rule
# builders internal/goip/actuate uses, and the BuildWriteRequest gate under
# them. Link, address, nsid, genl and xfrm writes stay goip_write-only.
treatmentFiles = map (f: "github.com/randomizedcoder/xtcp2/pkg/xtcpnl/${f}") [
"xtcpnl_route_write.go"
"xtcpnl_rule_write.go"
"xtcpnl_write_request.go"
];
treatmentSyms = "pkg/xtcpnl[.](BuildWriteRequest|BuildRouteWriteRequest|BuildRuleWriteRequest)$";

# The files goip_write gates, in the packages that hold them.
writePkgs = "^github.com/randomizedcoder/xtcp2/(internal/goip|pkg/xtcpnl|pkg/nlparity)$";
writeFiles = "(_write[a-z0-9_]*|_flush|_restore|link_property|stats_set)[.]go$";

# minio-go's process credential provider (the s3parquet destination) runs
# credential_process from an AWS config; xtcp2 builds static credentials
# (credentials.NewStaticV4), so that provider is never constructed.
execAllowed = [ "github.com/minio/minio-go/v7/pkg/credentials" ];

writeSyms = lib.concatStringsSep "|" [
"pkg/xtcpnl\\.Build[A-Za-z]*WriteRequest"
"pkg/xtcpnl\\.BuildSetNsidRequest"
"pkg/xtcpnl\\.Encode[A-Za-z0-9]*InfoData"
"internal/goip\\.(linkWrite|routeWrite|addrWrite)$"
];
in
pkgs.runCommand "xtcp2-no-write-encoders"
{
nativeBuildInputs = [ versions.go ];
inherit vendoredSource;
}
''
cp -r $vendoredSource ./xtcp2 && chmod -R +w ./xtcp2
cd ./xtcp2
export HOME=$(mktemp -d) GOTOOLCHAIN=local GOFLAGS=-mod=vendor CGO_ENABLED=0
export GOCACHE=$TMPDIR/go-cache
fail=0

tags_of() {
go version -m "$1" | sed -n 's/^[[:space:]]*build[[:space:]]*-tags=//p'
}
# write_files PKG TAGS: the write files in PKG's deps under TAGS.
write_files() {
go list -deps -tags "$2" -f '{{.ImportPath}} {{join .GoFiles " "}}' "$1" \
| awk -v p='${writePkgs}' -v f='${writeFiles}' \
'$1 ~ p { for (i = 2; i <= NF; i++) if ($i ~ f) print $1 "/" $i }'
}

# has_tag TAGS TAG: TAGS (comma-separated, as go version -m prints them)
# includes TAG.
has_tag() {
case ",$1," in *",$2,"*) return 0 ;; esac
return 1
}

${lib.concatMapStrings (d: ''
t=$(tags_of ${d.bin})
all=$(write_files ${d.pkg} "$t")
w=$all
if has_tag "$t" treatment; then
w=$(printf '%s\n' "$all" | grep -vxF ${
lib.concatMapStringsSep " " (f: "-e ${f}") treatmentFiles
} || true)
for f in ${lib.concatStringsSep " " treatmentFiles}; do
printf '%s\n' "$all" | grep -qxF "$f" \
|| { echo "FAIL: ${d.name} (tags: $t) does not compile $f; treatmentFiles is stale"; fail=1; }
done
fi
if [ -n "$w" ]; then
echo "FAIL: ${d.name} (tags: $t) compiles write files:"; echo "$w"; fail=1
fi
if [ "$(write_files ${d.pkg} "''${t:+$t,}goip_write" | wc -l)" -le "$(printf '%s' "$all" | grep -c . || true)" ]; then
echo "FAIL: ${d.name} with goip_write names no further write file; the pattern is stale"; fail=1
fi

${lib.optionalString d.nm ''
s=$(go tool nm ${d.bin} | grep -E '${writeSyms}' || true)
if has_tag "$t" treatment; then
s=$(printf '%s\n' "$s" | grep -vE '${treatmentSyms}' || true)
fi
if [ -n "$s" ]; then
echo "FAIL: ${d.name} links write encoders:"; echo "$s"; fail=1
fi
''}

e=$(go list -deps -tags "$t" -f '{{.ImportPath}} {{join .Imports " "}}' ${d.pkg} \
| awk '{ for (i = 2; i <= NF; i++) if ($i == "os/exec") print $1 }' \
| grep -vxF ${lib.concatMapStringsSep " " (p: "-e ${p}") execAllowed} || true)
if [ -n "$e" ]; then
echo "FAIL: ${d.name} (tags: $t) imports os/exec in:"; echo "$e"; fail=1
fi
'') daemons}

if [ -z "$(go tool nm ${binaries.byVariant.debug.goip}/bin/goip | grep -E 'pkg/xtcpnl[.]BuildWriteRequest$' || true)" ]; then
echo "FAIL: goip lacks xtcpnl.BuildWriteRequest; goip_write is not wired"; fail=1
fi

[ "$fail" = 0 ] || exit 1
echo "ok: ${
lib.concatMapStringsSep " " (d: d.name) daemons
} carry no write encoders (bar treatment's route and rule builders) and no unlisted os/exec; goip has them" > $out
''
1 change: 1 addition & 0 deletions nix/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ let
xdp2
lintBaselineMeasure
;
linkmonitor = linkmonitorPackages.go-link-monitor;
};

# Behavioral test runners
Expand Down