From 30503c21767224a0cb6eec287084b9e72d66e646 Mon Sep 17 00:00:00 2001 From: Andrey Larin Date: Fri, 2 Oct 2026 20:38:21 -0400 Subject: [PATCH 1/2] Say adopted servers have no keys, and flag a tunnel left on a destroyed one --- README.md | 11 ++++++++++- internal/cli/destroy.go | 8 +++++++- internal/cli/provision.go | 4 ++-- internal/cli/rotate.go | 10 ++++++++-- internal/cli/tun.go | 24 +++++++++++++++++++++++ internal/cli/tun_test.go | 37 +++++++++++++++++++++++++++++++++++ internal/client/client.go | 6 +++++- internal/config/config.go | 4 ++-- internal/provider/provider.go | 6 +++--- 9 files changed, 98 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 3f35ba4..9d66ee9 100644 --- a/README.md +++ b/README.md @@ -216,9 +216,18 @@ vpncli sync `sync` treats the provider as the source of truth. Rows for servers that no longer exist are dropped, drifted addresses and statuses are corrected, and servers tagged `vpncli` that local state has never seen are adopted - which is -how a second machine, or a run that died mid-provision, is picked back up. +how a run that died mid-provision is picked back up, and how a second machine +gets to see, rotate and destroy what the first one made. Untagged servers are left alone, because that listing covers the whole account. +Adopting a server is not the same as being able to connect to it. Its REALITY +keys were generated by the machine that bootstrapped it and live only in that +machine's state file, so on any other machine an adopted server has no keys. +`vpncli server bootstrap` would give it some, but they are new keys and they +replace the old ones, which cuts off every client the first machine set up. +To use a server from two machines, connect from the one that made it and +import the link on the other. + Create a server from those answers: ```sh diff --git a/internal/cli/destroy.go b/internal/cli/destroy.go index 4939c46..8121529 100644 --- a/internal/cli/destroy.go +++ b/internal/cli/destroy.go @@ -37,7 +37,13 @@ Requires DIGITALOCEAN_TOKEN or DIGITALOCEAN_ACCESS_TOKEN to be set.`, if err != nil { return fmt.Errorf("%q is not a server id: `vpncli server list` shows them", args[0]) } - return runDestroy(cmd.Context(), cmd.InOrStdin(), cmd.OutOrStdout(), openProvider, id, yes) + if err := runDestroy(cmd.Context(), cmd.InOrStdin(), cmd.OutOrStdout(), openProvider, id, yes); err != nil { + return err + } + if t, err := newTunnel(); err == nil { + noteDeadTunnel(cmd.Context(), cmd.OutOrStdout(), t, id) + } + return nil }, } diff --git a/internal/cli/provision.go b/internal/cli/provision.go index b48c1c8..0a6d070 100644 --- a/internal/cli/provision.go +++ b/internal/cli/provision.go @@ -29,8 +29,8 @@ wait: a server that exists but is not in state is invisible and still billed. So an interrupted wait leaves something ` + "`vpncli server destroy`" + ` can clean up, and ` + "`vpncli sync`" + ` picks it up on any machine. -The server comes up as a stock OS image with the configured SSH key installed. -Installing Xray-core and the REALITY camouflage is v0.8.0. +Once it has booted, the bootstrap installs Xray-core and the REALITY +camouflage over SSH. If that fails halfway, ` + "`vpncli server bootstrap`" + ` finishes it. Requires DIGITALOCEAN_TOKEN or DIGITALOCEAN_ACCESS_TOKEN to be set.`, Args: cobra.NoArgs, diff --git a/internal/cli/rotate.go b/internal/cli/rotate.go index 0ebbe4c..bf0da3a 100644 --- a/internal/cli/rotate.go +++ b/internal/cli/rotate.go @@ -48,8 +48,14 @@ Requires DIGITALOCEAN_TOKEN or DIGITALOCEAN_ACCESS_TOKEN to be set.`, if err != nil { return fmt.Errorf("%q is not a server id: `vpncli server list` shows them", args[0]) } - return runRotate(cmd.Context(), cmd.InOrStdin(), cmd.OutOrStdout(), - openProvider, dialSSH, reality.Check, id, yes) + if err := runRotate(cmd.Context(), cmd.InOrStdin(), cmd.OutOrStdout(), + openProvider, dialSSH, reality.Check, id, yes); err != nil { + return err + } + if t, err := newTunnel(); err == nil { + noteDeadTunnel(cmd.Context(), cmd.OutOrStdout(), t, id) + } + return nil }, } diff --git a/internal/cli/tun.go b/internal/cli/tun.go index e3c28a8..8dabc7b 100644 --- a/internal/cli/tun.go +++ b/internal/cli/tun.go @@ -244,6 +244,30 @@ func runTunStatus(ctx context.Context, out io.Writer, t *tunnel) error { return nil } +// noteDeadTunnel says so when the tunnel on this machine runs through a server +// that is no longer in state. It stays up and carries nothing, which looks +// exactly like the network having gone. Best effort: a check that fails +// should not turn a destroy that worked into an error. +func noteDeadTunnel(ctx context.Context, out io.Writer, t *tunnel, id int64) { + _, running, err := t.running(ctx) + if err != nil || running.Server != id { + return + } + + store, err := openStore() + if err != nil { + return + } + defer store.Close() + + if _, err := store.Get(ctx, id); !errors.Is(err, state.ErrNotFound) { + return + } + + fmt.Fprintf(out, "The tunnel on this machine still goes through it and carries nothing: "+ + "`vpncli tun down`, then `vpncli tun up`.\n") +} + // chooseServer resolves the id a tunnel is for. With none given it takes the // most recently configured server, which after a provision or a rotation is // the one meant. diff --git a/internal/cli/tun_test.go b/internal/cli/tun_test.go index a5c081a..2f7b993 100644 --- a/internal/cli/tun_test.go +++ b/internal/cli/tun_test.go @@ -530,3 +530,40 @@ func TestOlder(t *testing.T) { } } } + +func TestNoteDeadTunnel(t *testing.T) { + for _, tc := range []struct { + name string + through int64 + gone int64 + want bool + }{ + {"through the destroyed server", 99, 99, true}, + {"through a server still in state", 1, 1, false}, + {"through some other server", 1, 99, false}, + } { + t.Run(tc.name, func(t *testing.T) { + tn, f := tunneling(t) + if err := tn.save(record{Server: tc.through, Started: time.Now()}); err != nil { + t.Fatalf("save: %v", err) + } + f.live = []int{4242} + + var out bytes.Buffer + noteDeadTunnel(context.Background(), &out, tn, tc.gone) + if got := strings.Contains(out.String(), "vpncli tun down"); got != tc.want { + t.Errorf("note = %q, want a note: %v", out.String(), tc.want) + } + }) + } +} + +func TestNoteDeadTunnelWithNothingRunning(t *testing.T) { + tn, _ := tunneling(t) + + var out bytes.Buffer + noteDeadTunnel(context.Background(), &out, tn, 99) + if out.Len() != 0 { + t.Errorf("note = %q, want nothing", out.String()) + } +} diff --git a/internal/client/client.go b/internal/client/client.go index 5e96f16..7f1fb54 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -103,7 +103,11 @@ func usable(srv state.Server) error { case srv.IPv4 == "": return fmt.Errorf("server %d has no address yet: `vpncli sync` picks one up once it has booted", srv.ID) case !srv.Bootstrapped() || !srv.Credentials.Complete(): - return fmt.Errorf("%w: `vpncli server bootstrap %d` configures it", ErrNotConfigured, srv.ID) + // A server adopted by sync lands here too: its keys are in the state + // file of the machine that bootstrapped it, and nowhere else. + return fmt.Errorf("%w here: if another machine set it up, connect from that one; "+ + "`vpncli server bootstrap %d` configures it, and replaces any keys it already has", + ErrNotConfigured, srv.ID) } return nil } diff --git a/internal/config/config.go b/internal/config/config.go index fe22ecb..a427090 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -152,8 +152,8 @@ func LoadFrom(path string) (Config, error) { } // Save writes the config to the default path, creating the directory. The file -// is written 0600 and the directory 0700: no secrets live here today, but -// v0.8.0 adds REALITY key material and the permissions should already be right. +// is written 0600 and the directory 0700. It holds no secrets, but it names +// the account's servers and the key that logs in to them. func (c *Config) Save() error { path, err := Path() if err != nil { diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 4fac566..574cd7b 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -1,6 +1,6 @@ // Package provider defines the VPS provider abstraction. Every supported -// cloud (DigitalOcean first, Hetzner/Vultr/Linode later) implements -// VPSProvider so the rest of vpncli never learns which one it is talking to. +// cloud implements VPSProvider so the rest of vpncli never learns which one +// it is talking to. package provider import ( @@ -63,7 +63,7 @@ type CreateOptions struct { Image string // SSHKeyIDs are provider-side identifiers of keys to install for root. - // Bootstrap (v0.8.0) connects over SSH, so at least one is required. + // The bootstrap connects over SSH, so at least one is required. SSHKeyIDs []string // IPv6 asks for a routable IPv6 address as well as the IPv4 one. A From ebe6682fd89153a893417317333dfeabbba9fd0f Mon Sep 17 00:00:00 2001 From: Andrey Larin Date: Fri, 2 Oct 2026 20:41:00 -0400 Subject: [PATCH 2/2] Show the tunnel's interface in tun status --- README.md | 14 ++++++- internal/cli/tun.go | 45 +++++++++++++++++++--- internal/cli/tun_test.go | 79 ++++++++++++++++++++++++++++++++++++++- internal/cli/tunnel.go | 54 ++++++++++++++++++++++++++ internal/client/client.go | 5 +++ 5 files changed, 189 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 9d66ee9..f481d2c 100644 --- a/README.md +++ b/README.md @@ -405,10 +405,20 @@ when you interrupt it, so there is no file or process left to remember. ```sh vpncli tun up 3 --detach -vpncli tun status # up through vpncli-ams3-0a910d (203.0.113.10, ams3) for 8m +vpncli tun status vpncli tun down ``` +``` +up through vpncli-ams3-0a910d (203.0.113.10, ams3) for 8m +interface utun5: 172.19.0.1/30, fdfe:dcba:9876::1/126, mtu 9000, up +``` + +The interface is found by the tunnel's own address, because its name is the +system's choice. A sing-box running with no interface carrying that address is +reported as routing nothing - which is what one started without root looks +like, and otherwise looks exactly like a tunnel that works. + With no id it takes the most recently configured server, which after a provision or a rotation is the one you meant. @@ -523,7 +533,7 @@ vpncli server rotate 3 replace one with a fresh server vpncli server destroy 3 delete one and forget it vpncli tun up 3 route this machine through a server -vpncli tun status whether the tunnel is up, and since when +vpncli tun status whether the tunnel is up, since when, and on which interface vpncli tun down stop it vpncli sync reconcile local state against the provider diff --git a/internal/cli/tun.go b/internal/cli/tun.go index 8dabc7b..ddf201c 100644 --- a/internal/cli/tun.go +++ b/internal/cli/tun.go @@ -7,6 +7,7 @@ import ( "io" "os" "strconv" + "strings" "time" "github.com/spf13/cobra" @@ -102,11 +103,15 @@ func newTunStatusCommand() *cobra.Command { return &cobra.Command{ Use: "status", Short: "Say whether the tunnel is up", - Long: `Report whether a tunnel is running, through which server, and since when. - -It reads local state and one process, so it is instant and works offline. It -does not check that traffic is actually flowing - for that, ask something on -the internet where it thinks you are.`, + Long: `Report whether a tunnel is running, through which server, and since when, +and the network interface it created: its name, addresses, MTU and whether it +is up. + +It reads local state, the process list and the interface table, so it is +instant and works offline. A sing-box with no interface is reported as routing +nothing, which is what one started without root does. It does not check that +traffic is actually flowing - for that, ask something on the internet where it +thinks you are.`, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { t, err := newTunnel() @@ -217,6 +222,14 @@ func runTunStatus(ctx context.Context, out io.Writer, t *tunnel) error { return err } + if err := printTunServer(ctx, out, t, running); err != nil { + return err + } + return printTunInterface(out, t) +} + +// printTunServer says which server the tunnel goes through, and since when. +func printTunServer(ctx context.Context, out io.Writer, t *tunnel, running record) error { if running.Server == 0 { // Running, but not started by this command - so there is nothing to // say about which server or since when. @@ -244,6 +257,28 @@ func runTunStatus(ctx context.Context, out io.Writer, t *tunnel) error { return nil } +// printTunInterface describes the interface sing-box created. A running +// sing-box with no interface is the one failure status can actually see: it +// was started without root, and is routing nothing. +func printTunInterface(out io.Writer, t *tunnel) error { + l, err := t.run.Interface(client.TunAddr()) + if errors.Is(err, ErrNoInterface) { + fmt.Fprintf(out, "but no interface has %s on it, so nothing is being routed: sing-box needs root to create one\n", + client.TunAddr()) + return nil + } + if err != nil { + return err + } + + how := "up" + if !l.Up { + how = "down, so nothing is being routed" + } + fmt.Fprintf(out, "interface %s: %s, mtu %d, %s\n", l.Name, strings.Join(l.Addrs, ", "), l.MTU, how) + return nil +} + // noteDeadTunnel says so when the tunnel on this machine runs through a server // that is no longer in state. It stays up and carries nothing, which looks // exactly like the network having gone. Best effort: a check that fails diff --git a/internal/cli/tun_test.go b/internal/cli/tun_test.go index 2f7b993..b85c16b 100644 --- a/internal/cli/tun_test.go +++ b/internal/cli/tun_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "io" + "net/netip" "os" "strings" "testing" @@ -30,10 +31,18 @@ type fakeRunner struct { // version is what the client prints; versionErr is it failing to answer. version string versionErr error + + // link is the interface the tunnel's address is on; linkErr is there + // being none. + link link + linkErr error } func newFakeRunner() *fakeRunner { - return &fakeRunner{version: "sing-box version 1.14.0\n\nEnvironment: go1.26.7 darwin/arm64\n"} + return &fakeRunner{ + version: "sing-box version 1.14.0\n\nEnvironment: go1.26.7 darwin/arm64\n", + link: link{Name: "utun9", MTU: 9000, Up: true, Addrs: []string{"172.19.0.1/30"}}, + } } func (f *fakeRunner) Version(context.Context) (string, error) { @@ -63,6 +72,8 @@ func (f *fakeRunner) Start(_ context.Context, _ io.Writer, name string, args ... return nil } +func (f *fakeRunner) Interface(netip.Addr) (link, error) { return f.link, f.linkErr } + func (f *fakeRunner) Matching(context.Context, string) ([]int, error) { return f.live, nil } func (f *fakeRunner) Stop(_ context.Context, _ io.Writer, pids []int) error { @@ -567,3 +578,69 @@ func TestNoteDeadTunnelWithNothingRunning(t *testing.T) { t.Errorf("note = %q, want nothing", out.String()) } } + +func TestTunStatusNamesTheInterface(t *testing.T) { + tn, _ := tunneling(t) + if _, err := up(t, tn, 1, true); err != nil { + t.Fatalf("tun up: %v", err) + } + + var out bytes.Buffer + if err := runTunStatus(context.Background(), &out, tn); err != nil { + t.Fatalf("tun status: %v", err) + } + want := "interface utun9: 172.19.0.1/30, mtu 9000, up" + if !strings.Contains(out.String(), want) { + t.Errorf("status = %q, want it to say %q", out.String(), want) + } +} + +// sing-box run without root starts, logs, and creates nothing. That is the +// one failure status can see without sending traffic anywhere. +func TestTunStatusWithNoInterface(t *testing.T) { + tn, f := tunneling(t) + if _, err := up(t, tn, 1, true); err != nil { + t.Fatalf("tun up: %v", err) + } + f.linkErr = ErrNoInterface + + var out bytes.Buffer + if err := runTunStatus(context.Background(), &out, tn); err != nil { + t.Fatalf("tun status: %v", err) + } + if !strings.Contains(out.String(), "nothing is being routed") { + t.Errorf("status = %q, want it to say nothing is routed", out.String()) + } +} + +func TestTunStatusWithTheInterfaceDown(t *testing.T) { + tn, f := tunneling(t) + if _, err := up(t, tn, 1, true); err != nil { + t.Fatalf("tun up: %v", err) + } + f.link.Up = false + + var out bytes.Buffer + if err := runTunStatus(context.Background(), &out, tn); err != nil { + t.Fatalf("tun status: %v", err) + } + if !strings.Contains(out.String(), "utun9") || !strings.Contains(out.String(), "down, so nothing") { + t.Errorf("status = %q, want the interface reported as down", out.String()) + } +} + +// The status reads the real interface table, so it has to find whatever the +// tunnel address is on. Loopback is on every machine. +func TestSystemInterfaceFindsAnAddress(t *testing.T) { + l, err := system{}.Interface(netip.MustParseAddr("127.0.0.1")) + if err != nil { + t.Fatalf("Interface: %v", err) + } + if l.Name == "" || !l.Up { + t.Errorf("link = %+v, want the loopback interface, up", l) + } + + if _, err := (system{}).Interface(netip.MustParseAddr("192.0.2.255")); !errors.Is(err, ErrNoInterface) { + t.Errorf("err = %v, want ErrNoInterface for an address nothing has", err) + } +} diff --git a/internal/cli/tunnel.go b/internal/cli/tunnel.go index 413732f..e4e1307 100644 --- a/internal/cli/tunnel.go +++ b/internal/cli/tunnel.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" "io" + "net" + "net/netip" "os" "os/exec" "path/filepath" @@ -60,6 +62,20 @@ type runner interface { Matching(ctx context.Context, needle string) ([]int, error) // Stop ends processes that are running as root. Stop(ctx context.Context, out io.Writer, pids []int) error + // Interface finds the network interface carrying addr, or reports + // ErrNoInterface. + Interface(addr netip.Addr) (link, error) +} + +// ErrNoInterface is returned when no interface carries the tunnel's address. +var ErrNoInterface = errors.New("no interface carries the tunnel's address") + +// link is the interface sing-box created, as the system sees it. +type link struct { + Name string + MTU int + Up bool + Addrs []string } // tunnel is the client process and the files that describe it. @@ -250,6 +266,44 @@ func (system) Stop(ctx context.Context, out io.Writer, pids []int) error { return cmd.Run() } +func (system) Interface(addr netip.Addr) (link, error) { + ifaces, err := net.Interfaces() + if err != nil { + return link{}, fmt.Errorf("listing interfaces: %w", err) + } + + for _, iface := range ifaces { + addrs, err := iface.Addrs() + if err != nil { + continue + } + + found := false + var shown []string + for _, a := range addrs { + prefix, err := netip.ParsePrefix(a.String()) + if err != nil { + continue + } + found = found || prefix.Addr() == addr + // Link-local addresses come with every interface and say + // nothing about the tunnel. + if !prefix.Addr().IsLinkLocalUnicast() { + shown = append(shown, prefix.String()) + } + } + if found { + return link{ + Name: iface.Name, + MTU: iface.MTU, + Up: iface.Flags&net.FlagUp != 0, + Addrs: shown, + }, nil + } + } + return link{}, ErrNoInterface +} + // usable checks that the client is installed and new enough to read what this // generates. // diff --git a/internal/client/client.go b/internal/client/client.go index 7f1fb54..9d3294a 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -11,6 +11,7 @@ import ( "encoding/json" "fmt" "net" + "net/netip" "net/url" "strconv" @@ -53,6 +54,10 @@ const ( tunIPv6 = "fdfe:dcba:9876::1/126" ) +// TunAddr is this machine's end of the Tun mode interface. The interface name +// is the system's choice, so this is how it is found again once it is up. +func TunAddr() netip.Addr { return netip.MustParsePrefix(tunIPv4).Addr() } + // tunResolver is where Tun mode sends DNS, through the tunnel. Left to the // system, lookups would go out over the local network in plain sight and undo // most of the point of tunneling the traffic that follows them.