1 Commits
Author SHA1 Message Date
sneak 4d999fbc7e prepare-data-dir leaves files with a second name to their owner (closes #143)
check / check (push) Canceled after 0s
A hard link in /data shares its owner with a file that can be anywhere
on the same filesystem, so chowning it gave that file to the netwatch
user. A file with more than one name now keeps its owner and its path
is logged.

Owners are now set on the opened directory, file or link after
checking it, as chmodDir sets modes, so a hard link swapped in
afterwards is caught. Every other file is opened without following it;
a symbolic link is given to the netwatch user itself, and devices,
named pipes and sockets keep their owner. This needs Linux, the
image's system; elsewhere it returns an error. A failed owner change
names the file.

Model: opus-5-5
2026-10-18 22:20:21 +00:00
7 changed files with 32 additions and 165 deletions

No files matched your search

+2 -9
View File
@@ -212,15 +212,8 @@ replaces the default; set and empty, the server trusts no proxy. Loopback is not
in the default, so a reverse proxy on the same host has to be named, as in in the default, so a reverse proxy on the same host has to be named, as in
`TRUSTED_PROXIES=127.0.0.1/32`. A request whose direct peer is outside this set `TRUSTED_PROXIES=127.0.0.1/32`. A request whose direct peer is outside this set
has its forwarded headers ignored, and the direct peer is logged and has its forwarded headers ignored, and the direct peer is logged and
rate-limited instead. From a direct peer inside the set, the client address is rate-limited instead. The container image does not use this default; see
the right-most `X-Forwarded-For` entry that is not in the set: each proxy [Deployment](#deployment).
appends the address it took the request from, so the entries further left were
written by the client and are not used. An entry written as an address with a
port, such as `198.51.100.7:5000` or `[2001:db8::1]:443`, is read as that
address. If the search from the right finds only addresses in the set, or meets
an entry that is not an address at all, such as `unknown`, the direct peer is
used. `X-Real-IP` is read only when `X-Forwarded-For` is missing or empty. The
container image does not use this default; see [Deployment](#deployment).
A variable set to a value the server cannot use, such as `PORT=abc`, A variable set to a value the server cannot use, such as `PORT=abc`,
`DEBUG=maybe` or a `BIND_ADDRESS` that is not an IP address, stops it from `DEBUG=maybe` or a `BIND_ADDRESS` that is not an IP address, stops it from
+6 -23
View File
@@ -29,29 +29,12 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
swapped in after the check is caught too; a symbolic link is opened without swapped in after the check is caught too; a symbolic link is opened without
following it and given to the netwatch user itself, so the server can still following it and given to the netwatch user itself, so the server can still
follow one in a sticky directory that anyone can write to. Devices, named follow one in a sticky directory that anyone can write to. Devices, named
pipes and sockets keep their owner. Setting the owner on an open file that is pipes and sockets keep their owner. Opening a file without following it and
not a directory is built only for Linux: `prepare-data-dir` runs only in the setting the owner on the open file needs Linux, the system of the image; on
Linux image, and macOS, where the backend is also built for development, has any other system the package still builds, and `prepare-data-dir` fails at the
neither `O_PATH` nor `AT_EMPTY_PATH`. On any other system the package still first file that is not a directory. The test that runs as root hard-links a
builds, and `prepare-data-dir` fails at the first file that is not a file, a symbolic link and a named pipe from outside the volume into it and
directory. The test that runs as root hard-links a file, a symbolic link and a finds their owners unchanged
named pipe from outside the volume into it and finds their owners unchanged
- 2026-10-18: from a trusted proxy, the backend takes the client address from
the right-most `X-Forwarded-For` entry that is not in `TRUSTED_PROXIES`
([#144](https://git.eeqj.de/sneak/netwatch/issues/144)). It took the left-most
entry, which the client writes itself behind a proxy that appends to the
header, so a client could choose the address the rate limit counts and the
request log records. An entry written as an address with a port is read as
that address. With only trusted addresses in the header, or an entry that is
not an address at all (such as `unknown`) met on the way, the direct peer is
used. `X-Real-IP` is read only without `X-Forwarded-For`, and every
`X-Forwarded-For` header line is read, not only the first. The image's nginx,
and the `TRUSTED_PROXIES=127.0.0.1/32` it starts the backend with, are
unchanged; the comment in `configs/nginx.conf` that said the backend takes the
first entry now says it takes the one address nginx passes on. Unit tests
resolve the address from chains ending in an untrusted address, in trusted
ones, of only trusted ones, with ports, with an entry that is not an address,
and split over two header lines
- 2026-10-18: after an interval change, the summary's Reachable slot reads - 2026-10-18: after an interval change, the summary's Reachable slot reads
`--/--` in white, as at page load, until the second round since the change `--/--` in white, as at page load, until the second round since the change
ends ([#140](https://git.eeqj.de/sneak/netwatch/issues/140)). It used to read ends ([#140](https://git.eeqj.de/sneak/netwatch/issues/140)). It used to read
+1 -3
View File
@@ -46,9 +46,7 @@ server {
# 127.0.0.1:8081. These headers go with every request passed to it. # 127.0.0.1:8081. These headers go with every request passed to it.
# X-Forwarded-For carries only the client address, as resolved by # X-Forwarded-For carries only the client address, as resolved by
# the real IP settings above, and not the chain the request came # the real IP settings above, and not the chain the request came
# with. bin/entrypoint.sh starts the backend with # with: the backend takes the first entry, which a client can write.
# TRUSTED_PROXIES=127.0.0.1/32, the address nginx connects from, so
# the backend takes that one entry as the client address.
proxy_set_header Host $host; proxy_set_header Host $host;
proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-For $remote_addr;
+10 -43
View File
@@ -11,7 +11,6 @@ import (
"net/http" "net/http"
"net/netip" "net/netip"
"runtime/debug" "runtime/debug"
"slices"
"strings" "strings"
"time" "time"
@@ -134,17 +133,6 @@ func ipFromHostPort(hostPort string) string {
// X-Real-IP are honoured only when the direct peer is a // X-Real-IP are honoured only when the direct peer is a
// trusted proxy; otherwise the direct peer is returned so a // trusted proxy; otherwise the direct peer is returned so a
// spoofed header cannot forge the logged address. // spoofed header cannot forge the logged address.
//
// Each proxy appends to X-Forwarded-For the address it took the
// request from, so the entries on the right are written by the
// trusted proxies and those further left by the client. The
// client's address is therefore the right-most entry that is not
// a trusted proxy. An entry written as an address with a port is
// read as that address, both for the trust check and as the
// result. When every entry is a trusted proxy, or the walk from
// the right meets an entry that is not an address at all (such as
// "unknown"), the peer is used. X-Real-IP is read only when there
// is no X-Forwarded-For entry at all.
func clientIP( func clientIP(
remoteAddr string, remoteAddr string,
header http.Header, header http.Header,
@@ -156,49 +144,28 @@ func clientIP(
return peer return peer
} }
forwarded := forwardedFor(header) if xff := firstForwardedFor(header.Get("X-Forwarded-For")); xff != "" {
return xff
}
if len(forwarded) == 0 {
if xr := strings.TrimSpace(header.Get("X-Real-IP")); validIP(xr) { if xr := strings.TrimSpace(header.Get("X-Real-IP")); validIP(xr) {
return xr return xr
} }
return peer
}
for _, entry := range slices.Backward(forwarded) {
if !validIP(entry) {
return peer
}
if !addrInAny(entry, trusted) {
return entry
}
}
return peer return peer
} }
// forwardedFor returns the entries of X-Forwarded-For in order, // firstForwardedFor returns the left-most valid address in an
// leaving out blank ones. A proxy may add its entry as a header // X-Forwarded-For list (the original client), or "" if none.
// line of its own rather than append it to the line the client func firstForwardedFor(value string) string {
// sent, so every line is read, not only the first. Some proxies
// write an entry as an address with a port, such as
// 198.51.100.7:5000 or [2001:db8::1]:443; the port is removed so
// that the entry is read as its address.
func forwardedFor(header http.Header) []string {
value := strings.Join(header.Values("X-Forwarded-For"), ",")
var entries []string
for part := range strings.SplitSeq(value, ",") { for part := range strings.SplitSeq(value, ",") {
entry := strings.TrimSpace(part) candidate := strings.TrimSpace(part)
if entry != "" { if validIP(candidate) {
entries = append(entries, ipFromHostPort(entry)) return candidate
} }
} }
return entries return ""
} }
func validIP(s string) bool { func validIP(s string) bool {
+5 -77
View File
@@ -25,16 +25,10 @@ import (
const ( const (
// loopbackPeer is a remote address inside the trusted-proxy allowlist. // loopbackPeer is a remote address inside the trusted-proxy allowlist.
loopbackPeer = "127.0.0.1:5000" loopbackPeer = "127.0.0.1:5000"
// loopbackIP is the address of loopbackPeer, which ClientIP returns
// when it uses the peer.
loopbackIP = "127.0.0.1"
// forwardedIP is the client address presented via X-Forwarded-For. // forwardedIP is the client address presented via X-Forwarded-For.
forwardedIP = "203.0.113.7" forwardedIP = "203.0.113.7"
// realIP is the client address presented via X-Real-IP. // realIP is the client address presented via X-Real-IP.
realIP = "203.0.113.9" realIP = "203.0.113.9"
// appendedIP is the X-Forwarded-For entry a trusted proxy appended
// after the entries the client wrote.
appendedIP = "198.51.100.7"
) )
func mustPrefixes(t *testing.T, cidrs ...string) []netip.Prefix { func mustPrefixes(t *testing.T, cidrs ...string) []netip.Prefix {
@@ -91,35 +85,10 @@ func clientIPCases() []clientIPCase {
want: forwardedIP, want: forwardedIP,
}, },
{ {
name: "trusted proxy uses right-most of chain", name: "trusted proxy uses left-most of chain",
remoteAddr: loopbackPeer, remoteAddr: "10.1.2.3:5000",
xff: "203.0.113.9, " + appendedIP, xff: forwardedIP + ", 10.1.2.3",
want: appendedIP, want: forwardedIP,
},
{
name: "trusted proxy entries are skipped from the right",
remoteAddr: loopbackPeer,
xff: "203.0.113.9, 10.0.0.5",
want: "203.0.113.9",
},
{
name: "chain of only trusted proxies uses peer",
remoteAddr: loopbackPeer,
xff: "10.0.0.5, 10.1.2.3",
want: loopbackIP,
},
{
name: "entry that is not an address uses peer",
remoteAddr: loopbackPeer,
xff: "203.0.113.9, unknown",
want: loopbackIP,
},
{
name: "x-real-ip ignored when forwarded-for has entries",
remoteAddr: loopbackPeer,
xff: "10.0.0.5",
xRealIP: realIP,
want: loopbackIP,
}, },
{ {
name: "trusted proxy falls back to x-real-ip", name: "trusted proxy falls back to x-real-ip",
@@ -148,7 +117,7 @@ func clientIPCases() []clientIPCase {
name: "trusted proxy with garbage header uses peer", name: "trusted proxy with garbage header uses peer",
remoteAddr: loopbackPeer, remoteAddr: loopbackPeer,
xff: "not-an-ip", xff: "not-an-ip",
want: loopbackIP, want: "127.0.0.1",
}, },
} }
} }
@@ -179,47 +148,6 @@ func TestClientIP(t *testing.T) {
} }
} }
// TestClientIPReadsEveryForwardedForLine checks that an entry a proxy
// adds as an X-Forwarded-For line of its own is read, and the line the
// client sent before it is not taken.
func TestClientIPReadsEveryForwardedForLine(t *testing.T) {
t.Parallel()
header := http.Header{}
header.Add("X-Forwarded-For", "203.0.113.9")
header.Add("X-Forwarded-For", appendedIP)
got := middleware.ClientIP(loopbackPeer, header,
mustPrefixes(t, "127.0.0.1/32"))
if got != appendedIP {
t.Errorf("ClientIP() = %q, want %q", got, appendedIP)
}
}
// TestClientIPReadsAddressWithPort checks that an X-Forwarded-For entry
// written as an address with a port is read as that address, both for
// the trust check and as the result.
func TestClientIPReadsAddressWithPort(t *testing.T) {
t.Parallel()
trusted := mustPrefixes(t, "127.0.0.1/32", "10.0.0.0/8")
for xff, want := range map[string]string{
"203.0.113.9, " + appendedIP + ":5000": appendedIP,
"203.0.113.9, [2001:db8::1]:443": "2001:db8::1",
"203.0.113.9, 10.0.0.5:5000": "203.0.113.9",
} {
header := http.Header{}
header.Set("X-Forwarded-For", xff)
got := middleware.ClientIP(loopbackPeer, header, trusted)
if got != want {
t.Errorf("X-Forwarded-For %q: ClientIP() = %q, want %q",
xff, got, want)
}
}
}
func TestSecurityHeaders(t *testing.T) { func TestSecurityHeaders(t *testing.T) {
t.Parallel() t.Parallel()
+3 -4
View File
@@ -24,10 +24,9 @@ import (
// its number of names from the open file and gives that file to them, // its number of names from the open file and gives that file to them,
// so a hard link swapped in at name is caught too. // so a hard link swapped in at name is caught too.
// //
// This file is built only on Linux: prepare-data-dir runs only in the // This file is built only on Linux, because O_PATH and AT_EMPTY_PATH
// Linux image, and macOS, where the backend is also built for // exist only there; prepare-data-dir runs only in the Linux image. On
// development, has neither O_PATH nor AT_EMPTY_PATH. On any other // any other system chownFile returns an error (see datadir_other.go).
// system chownFile returns an error (see datadir_other.go).
func chownFile(root *os.Root, name string, uid, gid int) error { func chownFile(root *os.Root, name string, uid, gid int) error {
file, err := root.OpenFile(name, unix.O_PATH, 0) file, err := root.OpenFile(name, unix.O_PATH, 0)
if err != nil { if err != nil {
+5 -6
View File
@@ -10,12 +10,11 @@ import (
// chownFile returns an error on every system but Linux. Opening a file // chownFile returns an error on every system but Linux. Opening a file
// that is not a directory without reading it or following it, and // that is not a directory without reading it or following it, and
// giving it to a user on the open file, never by name, is built only // giving it to a user on the open file, never by name, needs O_PATH
// for Linux (see datadir_linux.go): prepare-data-dir runs only in the // and AT_EMPTY_PATH, which only Linux has (see datadir_linux.go), and
// Linux image, and macOS, where the backend is also built for // prepare-data-dir runs only in the Linux image. This file lets the
// development, has neither O_PATH nor AT_EMPTY_PATH. This file lets // package build elsewhere, such as on macOS, where a developer runs
// the package build elsewhere, such as on macOS, where a developer // make build and make run.
// runs make build and make run.
func chownFile(_ *os.Root, name string, _, _ int) error { func chownFile(_ *os.Root, name string, _, _ int) error {
return &fs.PathError{Op: "chown", Path: name, Err: errors.ErrUnsupported} return &fs.PathError{Op: "chown", Path: name, Err: errors.ErrUnsupported}
} }