A peer that never connected has no timestamp, and names are readable

Two faults in the VPN tab, both found by running it against the real machines.

omitempty does nothing for a zero time.Time, so a peer that has never handshaken
was serialised as the year one and the page would have shown mbp14 as last seen
on 1 January 0001 instead of "never connected". The field is a pointer now, and
a test pins the JSON.

And peer names on a wg-quick host were never going to appear: /etc/wireguard is
root-only because it holds private keys, and the file was being opened as the
daemon's own user. It now falls back to reading it as root, the same way the
kernel state beside it is already read. Only comments and public keys are taken
from it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Mitchell-Johnson
2026-09-27 22:01:13 -07:00
co-authored by Claude Opus 5
parent 6f634bbe11
commit acb83d3a4a
2 changed files with 67 additions and 13 deletions
+42 -11
View File
@@ -33,9 +33,11 @@ type Peer struct {
// Endpoint is where its packets come from, empty until it has been heard from. // Endpoint is where its packets come from, empty until it has been heard from.
Endpoint string `json:"endpoint,omitempty"` Endpoint string `json:"endpoint,omitempty"`
AllowedIPs []string `json:"allowed_ips"` AllowedIPs []string `json:"allowed_ips"`
// Handshake is the last completed handshake. Zero means never: a peer that is configured // Handshake is the last completed handshake, absent when there has never been one — a peer
// but has not connected since this tunnel came up. // that is configured but has not connected since this tunnel came up. A pointer because
Handshake time.Time `json:"handshake,omitempty"` // omitempty does nothing for a zero time.Time, and a peer that has never been heard from
// would otherwise be reported as having last spoken in the year one.
Handshake *time.Time `json:"handshake,omitempty"`
// Online is a handshake inside the last three minutes. WireGuard is silent when idle, so // Online is a handshake inside the last three minutes. WireGuard is silent when idle, so
// this means "was talking recently", not "is connected" — there is no such thing here. // this means "was talking recently", not "is connected" — there is no such thing here.
Online bool `json:"online"` Online bool `json:"online"`
@@ -108,7 +110,7 @@ func Look(ctx context.Context) Report {
for j := range t.Peers { for j := range t.Peers {
p := &t.Peers[j] p := &t.Peers[j]
p.Name = names[p.PublicKey] p.Name = names[p.PublicKey]
p.Online = !p.Handshake.IsZero() && time.Since(p.Handshake) < onlineWithin p.Online = p.Handshake != nil && time.Since(*p.Handshake) < onlineWithin
} }
t.Kind, t.Exit = classify(*t) t.Kind, t.Exit = classify(*t)
sort.SliceStable(t.Peers, func(a, b int) bool { sort.SliceStable(t.Peers, func(a, b int) bool {
@@ -116,8 +118,8 @@ func Look(ctx context.Context) Report {
if pa.Online != pb.Online { if pa.Online != pb.Online {
return pa.Online return pa.Online
} }
if !pa.Handshake.Equal(pb.Handshake) { if ha, hb := shookAt(pa), shookAt(pb); !ha.Equal(hb) {
return pa.Handshake.After(pb.Handshake) return ha.After(hb)
} }
return pa.Name < pb.Name return pa.Name < pb.Name
}) })
@@ -168,13 +170,22 @@ func parseDump(out string) []Tunnel {
p.AllowedIPs = strings.Split(f[4], ",") p.AllowedIPs = strings.Split(f[4], ",")
} }
if hs > 0 { if hs > 0 {
p.Handshake = time.Unix(hs, 0) at := time.Unix(hs, 0)
p.Handshake = &at
} }
tunnels[i].Peers = append(tunnels[i].Peers, p) tunnels[i].Peers = append(tunnels[i].Peers, p)
} }
return tunnels return tunnels
} }
// shookAt is a peer's last handshake, or the zero time when there has never been one.
func shookAt(p Peer) time.Time {
if p.Handshake == nil {
return time.Time{}
}
return *p.Handshake
}
// off turns wg's placeholders into an empty string. // off turns wg's placeholders into an empty string.
func off(v string) string { func off(v string) string {
if v == "(none)" || v == "off" { if v == "(none)" || v == "off" {
@@ -256,11 +267,11 @@ func confNames() map[string]string {
out := map[string]string{} out := map[string]string{}
files, _ := filepath.Glob(filepath.Join(confDir, "*.conf")) files, _ := filepath.Glob(filepath.Join(confDir, "*.conf"))
for _, path := range files { for _, path := range files {
f, err := os.Open(path) text, err := readMaybeRoot(path)
if err != nil { if err != nil {
continue // unreadable without root, which is the normal case; the kernel still has the keys continue
} }
sc := bufio.NewScanner(f) sc := bufio.NewScanner(strings.NewReader(text))
// last is the most recent comment line; when a [Peer] block opens, that comment becomes // last is the most recent comment line; when a [Peer] block opens, that comment becomes
// its pending name. Keeping the two apart is what lets each block take the comment that // its pending name. Keeping the two apart is what lets each block take the comment that
// introduces it and ignore any comment written inside it. // introduces it and ignore any comment written inside it.
@@ -286,11 +297,31 @@ func confNames() map[string]string {
pending = "" pending = ""
} }
} }
f.Close()
} }
return out return out
} }
// readMaybeRoot reads a file, falling back to root when it is not ours to read — which
// /etc/wireguard always is, since it holds private keys. Only the comments and public keys in
// it are looked at; nothing else in the file is kept or returned.
func readMaybeRoot(path string) (string, error) {
b, err := os.ReadFile(path)
if err == nil {
return string(b), nil
}
if !os.IsPermission(err) || !priv.Available() {
return "", err
}
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
args := priv.Wrap("cat", path)
out, cerr := exec.CommandContext(ctx, args[0], args[1:]...).Output()
if cerr != nil {
return "", cerr
}
return string(out), nil
}
// uciNames reads OpenWrt's configuration: each peer is a section with a description and a // uciNames reads OpenWrt's configuration: each peer is a section with a description and a
// public key. `uci show` is used rather than the file, so includes and overrides are applied. // public key. `uci show` is used rather than the file, so includes and overrides are applied.
func uciNames() map[string]string { func uciNames() map[string]string {
+25 -2
View File
@@ -1,8 +1,10 @@
package vpn package vpn
import ( import (
"encoding/json"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"testing" "testing"
"time" "time"
) )
@@ -48,11 +50,11 @@ func TestParseServerDump(t *testing.T) {
if p.Rx != 134192164 || p.Tx != 415977404 { if p.Rx != 134192164 || p.Tx != 415977404 {
t.Errorf("counters = %d/%d", p.Rx, p.Tx) t.Errorf("counters = %d/%d", p.Rx, p.Tx)
} }
if p.Handshake.Unix() != 1790565744 { if p.Handshake == nil || p.Handshake.Unix() != 1790565744 {
t.Errorf("handshake = %v", p.Handshake) t.Errorf("handshake = %v", p.Handshake)
} }
// The second peer has never connected: no endpoint, no handshake, nothing moved. // The second peer has never connected: no endpoint, no handshake, nothing moved.
if q := w.Peers[1]; q.Endpoint != "" || !q.Handshake.IsZero() || q.Rx != 0 { if q := w.Peers[1]; q.Endpoint != "" || q.Handshake != nil || q.Rx != 0 {
t.Errorf("second peer = %+v, want one that has never been heard from", q) t.Errorf("second peer = %+v, want one that has never been heard from", q)
} }
if kind, exit := classify(w); kind != "server" || exit != "" { if kind, exit := classify(w); kind != "server" || exit != "" {
@@ -149,3 +151,24 @@ func TestOnlineIsRecentHandshake(t *testing.T) {
} }
} }
} }
// A peer that has never handshaken must carry no time at all, not the zero one. omitempty
// does nothing for a time.Time, so this is the difference between the page saying "never
// connected" and saying the device was last seen in the year one.
func TestNeverConnectedHasNoTimestamp(t *testing.T) {
w := parseDump(gateDump)[0]
b, err := json.Marshal(w.Peers[1])
if err != nil {
t.Fatal(err)
}
if strings.Contains(string(b), "handshake") {
t.Errorf("json = %s, want no handshake field at all", b)
}
if strings.Contains(string(b), "0001-01-01") {
t.Errorf("json = %s, want no year-one timestamp", b)
}
// and the one that has connected still carries its time
if b, _ := json.Marshal(w.Peers[0]); !strings.Contains(string(b), "handshake") {
t.Errorf("json = %s, want the handshake of a peer that has one", b)
}
}