From 0e379386330f738979a18cb49153a307c5c52f24 Mon Sep 17 00:00:00 2001 From: matthew-pilot Date: Fri, 29 May 2026 16:10:03 +0000 Subject: [PATCH] fix(ipc): gate handshake approve/reject/revoke on admin token (PILOT-231) The IPC handshake approve/reject/revoke dispatch paths accepted any same-UID caller with no auth check, matching the pre-existing 'same-UID = full daemon authority' model. A compromised app-store package (no trust anchor, PILOT-243) or malicious process could grant tunnel access to arbitrary peers, reject pending requests, or tear down all trusted peer tunnels. Fix: require admin-token verification for the three state-mutation verbs (approve, reject, revoke), matching the existing BroadcastDatagram admin-token gate pattern: - ipc.go: checkHandshakeAdminToken strips the [tokenLen][token] prefix from the payload and verifies via constant-time compare when Config.AdminToken is non-empty; passes through when no token is configured (backward-compatible with pre-token configs). - driver.go: ApproveHandshake, RejectHandshake, RevokeTrust now accept an adminToken parameter and pack it into the IPC payload Closes PILOT-231 --- cmd/pilotctl/main.go | 9 +++++--- pkg/daemon/ipc.go | 41 ++++++++++++++++++++++++++++++++++++ pkg/driver/driver.go | 29 ++++++++++++++++--------- pkg/driver/zz_driver_test.go | 6 +++--- tests/zz_handshake_test.go | 8 +++---- tests/zz_trust_gate_test.go | 4 ++-- 6 files changed, 75 insertions(+), 22 deletions(-) diff --git a/cmd/pilotctl/main.go b/cmd/pilotctl/main.go index f8239afe..ef71bf9a 100644 --- a/cmd/pilotctl/main.go +++ b/cmd/pilotctl/main.go @@ -3960,7 +3960,8 @@ func cmdApprove(args []string) { nodeID := resolveToNodeID(d, args[0]) - result, err := d.ApproveHandshake(nodeID) + adminToken := getAdminToken() + result, err := d.ApproveHandshake(nodeID, adminToken) if err != nil { fatalCode("connection_failed", "approve: %v", err) } @@ -3986,7 +3987,8 @@ func cmdReject(args []string) { reason = args[1] } - result, err := d.RejectHandshake(nodeID, reason) + adminToken := getAdminToken() + result, err := d.RejectHandshake(nodeID, reason, adminToken) if err != nil { fatalCode("connection_failed", "reject: %v", err) } @@ -4006,7 +4008,8 @@ func cmdUntrust(args []string) { defer d.Close() nodeID := resolveToNodeID(d, args[0]) - _, err := d.RevokeTrust(nodeID) + adminToken := getAdminToken() + _, err := d.RevokeTrust(nodeID, adminToken) if err != nil { fatalCode("connection_failed", "untrust: %v", err) } diff --git a/pkg/daemon/ipc.go b/pkg/daemon/ipc.go index b695d3c1..abd9b2f4 100644 --- a/pkg/daemon/ipc.go +++ b/pkg/daemon/ipc.go @@ -4,6 +4,7 @@ package daemon import ( "context" + "crypto/subtle" "encoding/binary" "encoding/json" "errors" @@ -192,6 +193,8 @@ var ErrIPCClosed = errors.New("ipc: connection closed") // code paths. var ErrIPCBackpressure = errors.New("ipc: backpressure (client too slow)") +var errHandshakeAuth = errors.New("ipc: handshake requires admin token") + // IPCEnvelopeHeaderSize is the size of the per-message header that sits // inside the ipcutil length-framed envelope: 1 byte cmd. const IPCEnvelopeHeaderSize = 1 @@ -1232,6 +1235,10 @@ func (s *IPCServer) handleHandshake(conn *ipcConn, reqID uint64, payload []byte) s.ipcWriteHandshakeOK(conn, reqID, data) case SubHandshakeApprove: + rest, err := s.checkHandshakeAdminToken(conn, reqID, rest, "approve") + if err != nil { + return // error already sent + } if len(rest) < 4 { s.sendError(conn, reqID, "handshake approve: missing node_id") return @@ -1248,6 +1255,10 @@ func (s *IPCServer) handleHandshake(conn *ipcConn, reqID uint64, payload []byte) s.ipcWriteHandshakeOK(conn, reqID, data) case SubHandshakeReject: + rest, err := s.checkHandshakeAdminToken(conn, reqID, rest, "reject") + if err != nil { + return // error already sent + } if len(rest) < 4 { s.sendError(conn, reqID, "handshake reject: missing node_id") return @@ -1301,6 +1312,10 @@ func (s *IPCServer) handleHandshake(conn *ipcConn, reqID uint64, payload []byte) s.ipcWriteHandshakeOK(conn, reqID, data) case SubHandshakeRevoke: + rest, err := s.checkHandshakeAdminToken(conn, reqID, rest, "revoke") + if err != nil { + return // error already sent + } if len(rest) < 4 { s.sendError(conn, reqID, "handshake revoke: missing node_id") return @@ -1340,6 +1355,32 @@ func (s *IPCServer) handleHandshake(conn *ipcConn, reqID uint64, payload []byte) } } +// checkHandshakeAdminToken verifies the admin token prefix when the daemon +// has an admin token configured. Handshake approve/reject/revoke are +// privileged state-mutation verbs — they require the same token gate as +// BroadcastDatagram. When no admin token is configured, the check is a +// no-op (backward-compatible with pre-token daemon configs). +func (s *IPCServer) checkHandshakeAdminToken(conn *ipcConn, reqID uint64, rest []byte, verb string) ([]byte, error) { + if len(rest) < 2 { + s.sendError(conn, reqID, fmt.Sprintf("handshake %s: missing admin token header", verb)) + return nil, errHandshakeAuth + } + tokenLen := binary.BigEndian.Uint16(rest[0:2]) + if len(rest) < 2+int(tokenLen) { + s.sendError(conn, reqID, fmt.Sprintf("handshake %s: truncated admin token", verb)) + return nil, errHandshakeAuth + } + payload := rest[2+tokenLen:] + if s.daemon.config.AdminToken != "" { + token := string(rest[2 : 2+tokenLen]) + if subtle.ConstantTimeCompare([]byte(token), []byte(s.daemon.config.AdminToken)) != 1 { + s.sendError(conn, reqID, fmt.Sprintf("handshake %s: invalid admin token", verb)) + return nil, errHandshakeAuth + } + } + return payload, nil +} + func (s *IPCServer) ipcWriteHandshakeOK(conn *ipcConn, reqID uint64, data []byte) { if err := conn.writeReply(CmdHandshakeOK, reqID, data); err != nil { slog.Debug("IPC handshake reply failed", "err", err) diff --git a/pkg/driver/driver.go b/pkg/driver/driver.go index 5d2b6208..c64bc590 100644 --- a/pkg/driver/driver.go +++ b/pkg/driver/driver.go @@ -228,21 +228,27 @@ func (d *Driver) Handshake(nodeID uint32, justification string) (map[string]inte } // ApproveHandshake approves a pending trust handshake request. -func (d *Driver) ApproveHandshake(nodeID uint32) (map[string]interface{}, error) { - msg := make([]byte, 6) +func (d *Driver) ApproveHandshake(nodeID uint32, adminToken string) (map[string]interface{}, error) { + tokenBytes := []byte(adminToken) + msg := make([]byte, 1+1+2+len(tokenBytes)+4) msg[0] = cmdHandshake msg[1] = subHandshakeApprove - binary.BigEndian.PutUint32(msg[2:6], nodeID) + binary.BigEndian.PutUint16(msg[2:4], uint16(len(tokenBytes))) + copy(msg[4:4+len(tokenBytes)], tokenBytes) + binary.BigEndian.PutUint32(msg[4+len(tokenBytes):], nodeID) return d.jsonRPC(msg, cmdHandshakeOK, "approve") } // RejectHandshake rejects a pending trust handshake request. -func (d *Driver) RejectHandshake(nodeID uint32, reason string) (map[string]interface{}, error) { - msg := make([]byte, 1+1+4+len(reason)) +func (d *Driver) RejectHandshake(nodeID uint32, reason string, adminToken string) (map[string]interface{}, error) { + tokenBytes := []byte(adminToken) + msg := make([]byte, 1+1+2+len(tokenBytes)+4+len(reason)) msg[0] = cmdHandshake msg[1] = subHandshakeReject - binary.BigEndian.PutUint32(msg[2:6], nodeID) - copy(msg[6:], reason) + binary.BigEndian.PutUint16(msg[2:4], uint16(len(tokenBytes))) + copy(msg[4:4+len(tokenBytes)], tokenBytes) + binary.BigEndian.PutUint32(msg[4+len(tokenBytes):], nodeID) + copy(msg[4+len(tokenBytes)+4:], reason) return d.jsonRPC(msg, cmdHandshakeOK, "reject") } @@ -274,11 +280,14 @@ func (d *Driver) TrustedPeers() (map[string]interface{}, error) { } // RevokeTrust removes a peer from the trusted set and notifies the registry. -func (d *Driver) RevokeTrust(nodeID uint32) (map[string]interface{}, error) { - msg := make([]byte, 6) +func (d *Driver) RevokeTrust(nodeID uint32, adminToken string) (map[string]interface{}, error) { + tokenBytes := []byte(adminToken) + msg := make([]byte, 1+1+2+len(tokenBytes)+4) msg[0] = cmdHandshake msg[1] = subHandshakeRevoke - binary.BigEndian.PutUint32(msg[2:6], nodeID) + binary.BigEndian.PutUint16(msg[2:4], uint16(len(tokenBytes))) + copy(msg[4:4+len(tokenBytes)], tokenBytes) + binary.BigEndian.PutUint32(msg[4+len(tokenBytes):], nodeID) return d.jsonRPC(msg, cmdHandshakeOK, "revoke") } diff --git a/pkg/driver/zz_driver_test.go b/pkg/driver/zz_driver_test.go index 7d2070f9..f20d0f4c 100644 --- a/pkg/driver/zz_driver_test.go +++ b/pkg/driver/zz_driver_test.go @@ -481,10 +481,10 @@ func TestHandshakeFamilyRoundTrips(t *testing.T) { if _, err := drv.Handshake(99, "please"); err != nil { t.Fatalf("Handshake: %v", err) } - if _, err := drv.ApproveHandshake(100); err != nil { + if _, err := drv.ApproveHandshake(100, ""); err != nil { t.Fatalf("Approve: %v", err) } - if _, err := drv.RejectHandshake(101, "no"); err != nil { + if _, err := drv.RejectHandshake(101, "no", ""); err != nil { t.Fatalf("Reject: %v", err) } if _, err := drv.PendingHandshakes(); err != nil { @@ -493,7 +493,7 @@ func TestHandshakeFamilyRoundTrips(t *testing.T) { if _, err := drv.TrustedPeers(); err != nil { t.Fatalf("Trusted: %v", err) } - if _, err := drv.RevokeTrust(102); err != nil { + if _, err := drv.RevokeTrust(102, ""); err != nil { t.Fatalf("Revoke: %v", err) } diff --git a/tests/zz_handshake_test.go b/tests/zz_handshake_test.go index 324d8c7f..e1143acd 100644 --- a/tests/zz_handshake_test.go +++ b/tests/zz_handshake_test.go @@ -189,7 +189,7 @@ func TestHandshakePendingApproveReject(t *testing.T) { } // B approves A - _, err = drvB.ApproveHandshake(daemonA.NodeID()) + _, err = drvB.ApproveHandshake(daemonA.NodeID(), "") if err != nil { t.Fatalf("approve: %v", err) } @@ -235,7 +235,7 @@ func TestHandshakePendingApproveReject(t *testing.T) { } // B rejects C - _, err = drvB.RejectHandshake(daemonC.NodeID(), "not authorized") + _, err = drvB.RejectHandshake(daemonC.NodeID(), "not authorized", "") if err != nil { t.Fatalf("reject: %v", err) } @@ -409,7 +409,7 @@ func TestHandshakeRevokeTrust(t *testing.T) { } // A revokes trust in B - _, err := drvA.RevokeTrust(daemonB.NodeID()) + _, err := drvA.RevokeTrust(daemonB.NodeID(), "") if err != nil { t.Fatalf("revoke: %v", err) } @@ -531,7 +531,7 @@ func TestHandshakeRejectReason(t *testing.T) { } // B rejects with reason - _, err = drvB.RejectHandshake(daemonA.NodeID(), "not authorized for this network") + _, err = drvB.RejectHandshake(daemonA.NodeID(), "not authorized for this network", "") if err != nil { t.Fatalf("reject: %v", err) } diff --git a/tests/zz_trust_gate_test.go b/tests/zz_trust_gate_test.go index 5a612596..17cbc435 100644 --- a/tests/zz_trust_gate_test.go +++ b/tests/zz_trust_gate_test.go @@ -94,7 +94,7 @@ func TestWaitForTrustFastPathAfterTrust(t *testing.T) { } // B approves → both sides become trusted. - if _, err := b.Driver.ApproveHandshake(a.Daemon.NodeID()); err != nil { + if _, err := b.Driver.ApproveHandshake(a.Daemon.NodeID(), ""); err != nil { t.Fatalf("B approve: %v", err) } @@ -171,7 +171,7 @@ func TestWaitForTrustBlocksUntilApproved(t *testing.T) { case <-time.After(10 * time.Millisecond): } } - if _, err := b.Driver.ApproveHandshake(a.Daemon.NodeID()); err != nil { + if _, err := b.Driver.ApproveHandshake(a.Daemon.NodeID(), ""); err != nil { t.Fatalf("B approve: %v", err) }