peer: rotate samizdat credentials hourly (closes engineering#3437)

Currently peer.Client builds the libbox inbound exactly once per Start
and holds the same X25519 keypair / shortID / masquerade for the
entire peer process lifetime — a leaked credential (logs, telemetry,
support bundles, the route_id leakage in engineering#3440) remains
usable for hours or days.

This adds a credRotationLoop goroutine on a 1h tick. On each tick:

  1. Re-register with lantern-cloud against the same (address, port)
     tuple — same router-side mapping, fresh server-side row, fresh
     samizdat creds.
  2. Patch the new options for VPN bypass.
  3. Build a new libbox service.
  4. Close the old box (releases the listening port).
  5. Start the new box (re-binds the same port with new creds).
  6. Atomic swap of c.box, c.routeID.
  7. Best-effort deregister of the prior route_id so the bandit
     stops handing the old (now-invalid) creds to clients within
     ~immediately rather than waiting up-to-TTL for the row to
     expire.

Steps 4-5 leave a brief (~hundreds of ms) window where the port is
unbound; samizdat clients see TCP RST and reconnect via the bandit.
That's the trade-off vs. the security cost of holding the same cred
for the peer process lifetime — caps blast radius from cred leakage
to ~1h regardless of how long the peer has been running.

Rotation is best-effort: a single failure logs and waits for the
next tick. The current box and creds remain serving in the failure
case so a transient register error doesn't kill the session.

Config gains CredRotationInterval (defaults to peerCredRotationInterval
= 1h) so tests drive the loop without a 1h sleep — see
TestClient_RotatesCredentialsAtInterval.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: 2 people

peer/peer.go

@@ -54,14 +54,15 @@ type Status struct {
 }
 
 // Config plumbs in dependencies. Zero-valued fields fall back to production
-// defaults; HeartbeatInterval and HeartbeatTimeout exist so tests can drive
-// the loop without sleeping a full minute.
+// defaults; HeartbeatInterval, HeartbeatTimeout, and CredRotationInterval
+// exist so tests can drive the loops without sleeping a full minute / hour.
 type Config struct {
-	API               *API
-	NewForwarder      func(ctx context.Context) (portForwarder, error)
-	BuildBoxService   boxFactory
-	HeartbeatInterval time.Duration
-	HeartbeatTimeout  time.Duration
+	API                  *API
+	NewForwarder         func(ctx context.Context) (portForwarder, error)
+	BuildBoxService      boxFactory
+	HeartbeatInterval    time.Duration
+	HeartbeatTimeout     time.Duration
+	CredRotationInterval time.Duration
 }
 
 // Client orchestrates one peer-proxy session: open UPnP port → register with
@@ -92,8 +93,38 @@ type Client struct {
 	forwarder portForwarder
 	box       boxService
 	routeID   string
+	// externalPort / internalPort persist the port mapping picked at
+	// Start so the cred-rotation loop can re-register against the same
+	// (address, port) tuple without re-probing UPnP / re-mapping. The
+	// router-side mapping itself stays put across rotations; only the
+	// samizdat creds and route_id rotate.
+	externalPort uint16
+	internalPort uint16
+	// boxOptions is the fresh options string passed to BuildBoxService,
+	// kept for diagnostics and so the rotation path doesn't need to
+	// re-derive it from the (also-stored) box reference.
+	boxOptions string
+	// runCtx is captured here for the cred-rotation goroutine to bind
+	// the new libbox lifetime to the same context as the original Start.
+	// Stop's cancelRun() teardown still applies to the rebuilt box.
+	runCtx context.Context
 }
 
+// peerCredRotationInterval bounds how long a leaked samizdat
+// credential remains usable. At each tick the peer re-registers with
+// lantern-cloud (new route_id, new keypair, new shortID), rebuilds the
+// libbox service against the new options, and deregisters the prior
+// route. Caps blast radius from credential leakage (logs, telemetry,
+// memory dumps, the H2 leakage path in engineering#3440) to ~1h
+// regardless of peer process lifetime.
+//
+// Cost per rotation: one API.Register + Deregister round trip, one
+// libbox build + start + close cycle. Brief (~hundreds-of-ms) port-
+// rebind window during the swap; samizdat clients see TCP RST and
+// reconnect via the bandit. Acceptable trade-off vs. holding the same
+// cred for the full peer process lifetime.
+const peerCredRotationInterval = 1 * time.Hour
+
 // peerCleanupTimeout caps how long Start's rollback path waits for
 // Deregister / UnmapPort. Cleanup uses a fresh Background context (not the
 // caller's ctx) so an already-canceled or expired Start ctx doesn't skip
@@ -242,6 +273,10 @@ func (c *Client) Start(ctx context.Context) error {
 	c.forwarder = fwd
 	c.box = box
 	c.routeID = regResp.RouteID
+	c.externalPort = mapping.ExternalPort
+	c.internalPort = mapping.InternalPort
+	c.boxOptions = options
+	c.runCtx = runCtx
 	c.cancelRun = cancelRun
 	c.runDone = runDone
 	c.status = Status{
@@ -254,8 +289,14 @@ func (c *Client) Start(ctx context.Context) error {
 	statusSnapshot := c.status
 	c.mu.Unlock()
 
+	rotation := c.cfg.CredRotationInterval
+	if rotation == 0 {
+		rotation = peerCredRotationInterval
+	}
+
 	fwd.StartRenewal(runCtx)
 	go c.heartbeatLoop(runCtx, heartbeat, runDone)
+	go c.credRotationLoop(runCtx, rotation)
 
 	slog.Info("peer client started",
 		"external_ip", externalIP,
@@ -306,6 +347,10 @@ func (c *Client) Stop(ctx context.Context) error {
 	c.forwarder = nil
 	c.box = nil
 	c.routeID = ""
+	c.externalPort = 0
+	c.internalPort = 0
+	c.boxOptions = ""
+	c.runCtx = nil
 	c.status = Status{}
 	c.mu.Unlock()
 
@@ -397,6 +442,141 @@ func isNotRegistered(err error) bool {
 	return errors.As(err, &apiErr) && apiErr.Status == 404
 }
 
+// credRotationLoop periodically rotates the peer's samizdat credentials
+// (X25519 keypair, shortID, masquerade) by re-registering with
+// lantern-cloud, rebuilding the libbox inbound, and deregistering the
+// prior route. Caps blast radius from credential leakage to ~interval
+// regardless of peer process lifetime — see peerCredRotationInterval.
+//
+// Closes done is the responsibility of heartbeatLoop; this loop just
+// exits when ctx is cancelled. We deliberately don't add another close
+// channel: heartbeatLoop's done already gates Stop, and rotation
+// failures are non-fatal (log + retry next tick), so there's nothing
+// the Stop path needs to wait on from this goroutine.
+func (c *Client) credRotationLoop(ctx context.Context, interval time.Duration) {
+	t := time.NewTicker(interval)
+	defer t.Stop()
+	for {
+		select {
+		case <-ctx.Done():
+			return
+		case <-t.C:
+			if err := c.rotateCreds(ctx); err != nil {
+				// Don't kill the loop on a single failure — current
+				// box / route is still serving. Try again next tick.
+				slog.Warn("peer cred rotation failed; current creds remain in use", "err", err)
+			}
+		}
+	}
+}
+
+// rotateCreds atomically swaps the peer's samizdat credentials. On
+// success: a fresh route_id and keypair are in use, the libbox inbound
+// has been rebuilt against the new options, the prior route is
+// deregistered server-side, and the FlutterEvent stream sees no gap.
+// On failure: the prior creds and box continue serving — rotation is
+// best-effort. The router-side port mapping is preserved across the
+// rotation; only the in-process samizdat state changes.
+//
+// Sequence:
+//  1. Re-register with the same (externalIP, externalPort) as Start.
+//  2. Patch the new server-supplied options for VPN bypass.
+//  3. Build a new libbox service against the new options.
+//  4. Close the old box (releases the listening port).
+//  5. Start the new box (re-binds the same port, now with new creds).
+//  6. Atomic swap: c.box, c.routeID, c.boxOptions point at the new box.
+//  7. Best-effort deregister of the prior route_id so the bandit
+//     catalog stops handing the old (now-invalid) creds to clients.
+//
+// Steps 4-5 leave a brief (~hundreds of ms) window where the port
+// isn't bound; samizdat clients see TCP RST and reconnect. Acceptable
+// trade-off vs. the security cost of holding the same cred for the
+// peer process lifetime.
+func (c *Client) rotateCreds(ctx context.Context) error {
+	c.mu.Lock()
+	if !c.active {
+		c.mu.Unlock()
+		return errors.New("not active")
+	}
+	fwd := c.forwarder
+	extPort := c.externalPort
+	intPort := c.internalPort
+	oldRouteID := c.routeID
+	oldBox := c.box
+	c.mu.Unlock()
+
+	if fwd == nil || oldBox == nil {
+		return errors.New("rotateCreds: client state inconsistent")
+	}
+
+	externalIP, err := fwd.ExternalIP(ctx)
+	if err != nil {
+		return fmt.Errorf("get external ip: %w", err)
+	}
+	regResp, err := c.cfg.API.Register(ctx, RegisterRequest{
+		ExternalIP:   externalIP,
+		ExternalPort: extPort,
+		InternalPort: intPort,
+	})
+	if err != nil {
+		return fmt.Errorf("re-register: %w", err)
+	}
+	options, err := ensurePeerOutboundsBypassVPN(regResp.ServerConfig)
+	if err != nil {
+		return fmt.Errorf("patch sing-box options: %w", err)
+	}
+
+	c.mu.Lock()
+	runCtx := c.runCtx
+	c.mu.Unlock()
+	if runCtx == nil {
+		// Stop happened between the unlock above and here. Skip the
+		// build to avoid spinning up a libbox tied to a torn-down ctx.
+		// The new register row is harmless — server-side reaper will
+		// deprecate it after TTL since no heartbeat will arrive.
+		return errors.New("client stopped during rotation")
+	}
+	newBox, err := c.cfg.BuildBoxService(runCtx, options)
+	if err != nil {
+		return fmt.Errorf("build new sing-box: %w", err)
+	}
+
+	// Close old, start new. Order matters — both want the same port.
+	// If newBox.Start fails after oldBox.Close, we lost the listener
+	// and the next heartbeat / rotation tick is the recovery point.
+	if closeErr := oldBox.Close(); closeErr != nil {
+		slog.Warn("close old box during rotation", "err", closeErr)
+	}
+	if err := newBox.Start(); err != nil {
+		// Catastrophic: port is now unbound. Leave c.box pointing at
+		// oldBox so a future Stop tries to close it (idempotent on
+		// already-closed); the next rotation tick will try again.
+		return fmt.Errorf("start new sing-box: %w", err)
+	}
+
+	c.mu.Lock()
+	c.box = newBox
+	c.routeID = regResp.RouteID
+	c.boxOptions = options
+	c.status.RouteID = regResp.RouteID
+	c.mu.Unlock()
+
+	// Deregister the prior route so the bandit stops handing the old
+	// (now-invalid) creds to clients. Best-effort: the prior row will
+	// expire from its TTL anyway, but explicit deregister cuts the
+	// stale-creds window from up-to-TTL down to ~immediately.
+	if err := c.cfg.API.Deregister(ctx, oldRouteID); err != nil {
+		slog.Warn("deregister prior route after rotation",
+			"err", err, "old_route_id", oldRouteID)
+	}
+
+	slog.Info("peer cred rotation succeeded",
+		"new_route_id", regResp.RouteID,
+		"old_route_id", oldRouteID,
+	)
+	return nil
+}
+
 // ensurePeerOutboundsBypassVPN guarantees the peer sing-box's outbound dials
 // bind to the physical interface rather than whatever the OS routing table
 // picks. Without this, when the user's own Lantern VPN is up its TUN holds

peer/peer_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"sync"
@@ -139,6 +140,10 @@ type stubServer struct {
 	server             *httptest.Server
 	registerStatus     int
 	registerResp       RegisterResponse
+	// registerRespFn lets a test return a different response per
+	// register call (e.g. cred-rotation tests need a fresh route_id
+	// each time). When non-nil, takes precedence over registerResp.
+	registerRespFn     func() RegisterResponse
 	heartbeatStatus    int
 	deregisterStatus   int
 	registerCount      atomic.Int64
@@ -174,7 +179,11 @@ func newStubServer(t *testing.T) *stubServer {
 			http.Error(w, "register failed", s.registerStatus)
 			return
 		}
-		_ = json.NewEncoder(w).Encode(s.registerResp)
+		resp := s.registerResp
+		if s.registerRespFn != nil {
+			resp = s.registerRespFn()
+		}
+		_ = json.NewEncoder(w).Encode(resp)
 	})
 	mux.HandleFunc("/v1/peer/heartbeat", func(w http.ResponseWriter, r *http.Request) {
 		s.heartbeatCount.Add(1)
@@ -576,6 +585,90 @@ func TestAPIError_StringFormat(t *testing.T) {
 	assert.Contains(t, e.Error(), "could not connect")
 }
 
+// TestClient_RotatesCredentialsAtInterval pins the C2 fix from
+// engineering#3437: the peer client must re-register and rebuild its
+// libbox inbound on a schedule so a leaked credential's blast radius is
+// bounded by CredRotationInterval rather than peer process lifetime.
+//
+// Drives a short rotation interval (50ms) and asserts:
+//   1. Multiple registers happen (start + ≥2 rotations within 250ms).
+//   2. Each rotation deregisters the prior route_id.
+//   3. The peer's exposed RouteID changes — clients freshly assigned
+//      after a rotation see the new ID; the bandit catalog stops
+//      handing out the old one once Deregister lands.
+//   4. Multiple distinct boxes were built (the rotation actually
+//      rebuilt libbox; not just a no-op).
+//   5. The first box was closed (the old listener released its port).
+func TestClient_RotatesCredentialsAtInterval(t *testing.T) {
+	fwd := &fakeForwarder{externalIP: "203.0.113.42"}
+	srv := newStubServer(t)
+
+	// Each rotation needs a register response with a distinct
+	// route_id so we can verify the swap actually changed identifiers
+	// rather than re-registering the same id.
+	var registerSeq atomic.Int64
+	srv.registerRespFn = func() RegisterResponse {
+		n := registerSeq.Add(1)
+		return RegisterResponse{
+			RouteID:                  fmt.Sprintf("00000000-0000-0000-0000-00000000000%d", n),
+			ServerConfig:             `{"inbounds": [{"type":"samizdat","tag":"samizdat-in"}]}`,
+			HeartbeatIntervalSeconds: 60,
+		}
+	}
+
+	// Each BuildBoxService call gets a fresh fakeBoxService so we can
+	// see how many boxes were built and which ones got closed.
+	var (
+		boxesMu sync.Mutex
+		boxes   []*fakeBoxService
+	)
+	c := newTestClient(t, fwd, &fakeBoxService{}, srv, func(cfg *Config) {
+		cfg.CredRotationInterval = 50 * time.Millisecond
+		// Long heartbeat so heartbeat ticks don't compete with the
+		// register/deregister counters that we're asserting on.
+		cfg.HeartbeatInterval = time.Hour
+		cfg.BuildBoxService = func(_ context.Context, options string) (boxService, error) {
+			b := &fakeBoxService{gotConfig: options}
+			boxesMu.Lock()
+			boxes = append(boxes, b)
+			boxesMu.Unlock()
+			return b, nil
+		}
+	})
+
+	require.NoError(t, c.Start(context.Background()))
+	t.Cleanup(func() { _ = c.Stop(context.Background()) })
+
+	// Wait for at least 2 rotations on top of the initial register.
+	require.Eventually(t, func() bool {
+		return srv.registerCount.Load() >= 3
+	}, 1*time.Second, 25*time.Millisecond,
+		"expected ≥3 registers (initial + 2 rotations) within 1s; got %d",
+		srv.registerCount.Load())
+
+	// Each rotation deregisters the prior route — N rotations =>
+	// N deregisters (initial register is not preceded by one).
+	rotations := srv.registerCount.Load() - 1
+	assert.GreaterOrEqual(t, srv.deregisterCount.Load(), rotations-1,
+		"each rotation should deregister the prior route_id (got %d deregs vs %d rotations)",
+		srv.deregisterCount.Load(), rotations)
+
+	// RouteID exposed via Status should reflect the latest rotation.
+	c.mu.Lock()
+	currentRouteID := c.routeID
+	c.mu.Unlock()
+	assert.NotEqual(t, "00000000-0000-0000-0000-000000000001", currentRouteID,
+		"current route_id should have advanced past the initial register")
+
+	// Multiple boxes built; first one closed.
+	boxesMu.Lock()
+	defer boxesMu.Unlock()
+	require.GreaterOrEqual(t, len(boxes), 2,
+		"expected ≥2 libbox builds (initial + ≥1 rotation)")
+	assert.True(t, boxes[0].closed.Load(),
+		"first box should be closed by the first rotation")
+}
+
 // Subscribers (the IPC SSE handler in production) need both edges so the UI
 // can render fresh state without polling.
 func TestClient_StatusEventEmittedOnStartAndStop(t *testing.T) {