security: bind accept/revoke to authenticated identity, gate relayed approval, timestamp-independent replay (PPA-001/002/008)

handshake.go

@@ -553,9 +553,8 @@ func (hm *Manager) processMessage(stream coreapi.Stream, msg *HandshakeMsg) {
 		return
 	}
 
-	// Replay detection: hash the message and check set
-	msgBytes, _ := json.Marshal(msg)
-	msgHash := sha256.Sum256(msgBytes)
+	replayChallenge := fmt.Sprintf("handshake:%d:%d", msg.NodeID, hm.rt.NodeID())
+	msgHash := sha256.Sum256([]byte(msg.Type + "\x00" + replayChallenge + "\x00" + msg.Signature))
 	hm.replayMu.Lock()
 	if _, seen := hm.replaySet[msgHash]; seen {
 		hm.replayMu.Unlock()
@@ -616,6 +615,18 @@ func (hm *Manager) processMessage(stream coreapi.Stream, msg *HandshakeMsg) {
 		}
 	}
 
+	if msg.Type == HandshakeAccept || msg.Type == HandshakeRevoke {
+		if stream == nil || stream.RemoteAddr().Node != msg.NodeID {
+			var authenticated uint32
+			if stream != nil {
+				authenticated = stream.RemoteAddr().Node
+			}
+			slog.Warn("handshake: accept/revoke node_id does not match authenticated sender, rejecting",
+				"claimed_node_id", msg.NodeID, "authenticated_node_id", authenticated, "type", msg.Type)
+			return
+		}
+	}
+
 	switch msg.Type {
 	case HandshakeRequest:
 		hm.handleRequest(stream, msg, registryBound)
@@ -1144,6 +1155,11 @@ func (hm *Manager) processRelayedApproval(fromNodeID uint32) {
 		delete(hm.revoked, fromNodeID)
 	}
 
+	if _, ok := hm.outgoing[fromNodeID]; !ok {
+		slog.Warn("dropping relayed approval with no matching outgoing request", "peer_node_id", fromNodeID)
+		return
+	}
+
 	delete(hm.outgoing, fromNodeID)
 	hm.markTrustedLocked(fromNodeID, &TrustRecord{
 		NodeID:     fromNodeID,

zz_handshake_accept_request_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/pilot-protocol/common/coreapi"
 	"github.com/pilot-protocol/common/crypto"
 )
 
@@ -307,7 +308,7 @@ func TestProcessMessageDispatchesHandshakeAccept(t *testing.T) {
 		NodeID:    44,
 		Timestamp: time.Now().Unix(),
 	}
-	hm.processMessage(nil, msg)
+	hm.processMessage(&addrStream{addr: coreapi.Addr{Node: 44}}, msg)
 
 	hm.mu.RLock()
 	_, ok := hm.trusted[44]

zz_handshake_connection_parse_test.go

@@ -327,7 +327,7 @@ func TestProcessMessageRevokeDispatchRemovesTrusted(t *testing.T) {
 		NodeID:    99,
 		Timestamp: time.Now().Unix(),
 	}
-	hm.processMessage(nil, msg)
+	hm.processMessage(&addrStream{addr: coreapi.Addr{Node: 99}}, msg)
 
 	hm.mu.RLock()
 	_, stillTrusted := hm.trusted[99]

zz_handshake_relayed_trust_test.go

@@ -232,25 +232,19 @@ func TestProcessRelayedApprovalEstablishesTrustAndClearsOutgoing(t *testing.T) {
 	}
 }
 
-// --- processRelayedApproval: no outgoing entry still establishes trust ---
+// --- processRelayedApproval: no outgoing entry is dropped, not trusted ---
 
-func TestProcessRelayedApprovalNoOutgoingStillEstablishes(t *testing.T) {
+func TestProcessRelayedApprovalNoOutgoingDropped(t *testing.T) {
 	t.Parallel()
-	// The outgoing map is not a precondition — if somehow the registry relays an
-	// approval for a peer we didn't track as outgoing, the code still promotes
-	// them to trusted (delete of missing key is a safe no-op in Go).
 	hm := newTestHM(t, "")
 	t.Cleanup(hm.Stop)
 
 	hm.processRelayedApproval(300)
 
 	hm.mu.RLock()
-	rec, ok := hm.trusted[300]
+	_, ok := hm.trusted[300]
 	hm.mu.RUnlock()
-	if !ok {
-		t.Fatal("trusted[300] missing — relayed approval without outgoing should still establish")
-	}
-	if !rec.Mutual {
-		t.Fatal("rec.Mutual should be true")
+	if ok {
+		t.Fatal("trusted[300] set — relayed approval without an outgoing request must be dropped")
 	}
 }

zz_ppa001_accept_revoke_identity_test.go

@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+package handshake
+
+import (
+	"testing"
+	"time"
+
+	"github.com/pilot-protocol/common/coreapi"
+)
+
+func newBoundHM(t *testing.T, selfNodeID uint32) (*Manager, *testRuntime) {
+	t.Helper()
+	rt := newTestRuntime()
+	rt.nodeID = selfNodeID
+	hm := NewManager(rt)
+	t.Cleanup(hm.Stop)
+	return hm, rt
+}
+
+func TestPPA001_ForgedUnsignedAcceptForVictimRejected(t *testing.T) {
+	t.Parallel()
+	hm, _ := newBoundHM(t, 1)
+
+	const victim = uint32(700)
+	const attacker = uint32(666)
+
+	msg := &HandshakeMsg{
+		Type:      HandshakeAccept,
+		NodeID:    victim,
+		Timestamp: time.Now().Unix(),
+	}
+	hm.processMessage(&addrStream{addr: coreapi.Addr{Node: attacker}}, msg)
+
+	hm.mu.RLock()
+	_, trusted := hm.trusted[victim]
+	hm.mu.RUnlock()
+	if trusted {
+		t.Fatal("forged unsigned accept from a non-matching authenticated node must not inject trust")
+	}
+}
+
+func TestPPA001_UnsignedAcceptWithNoStreamRejected(t *testing.T) {
+	t.Parallel()
+	hm, _ := newBoundHM(t, 1)
+
+	msg := &HandshakeMsg{
+		Type:      HandshakeAccept,
+		NodeID:    44,
+		Timestamp: time.Now().Unix(),
+	}
+	hm.processMessage(nil, msg)
+
+	hm.mu.RLock()
+	_, trusted := hm.trusted[44]
+	hm.mu.RUnlock()
+	if trusted {
+		t.Fatal("accept with no authenticated transport must not inject trust")
+	}
+}
+
+func TestPPA001_HonestAcceptOverMatchingStreamTrusts(t *testing.T) {
+	t.Parallel()
+	hm, _ := newBoundHM(t, 1)
+
+	const peer = uint32(500)
+	msg := &HandshakeMsg{
+		Type:      HandshakeAccept,
+		NodeID:    peer,
+		Timestamp: time.Now().Unix(),
+	}
+	hm.processMessage(&addrStream{addr: coreapi.Addr{Node: peer}}, msg)
+
+	hm.mu.RLock()
+	_, trusted := hm.trusted[peer]
+	hm.mu.RUnlock()
+	if !trusted {
+		t.Fatal("honest accept over a matching authenticated stream must still establish trust")
+	}
+}
+
+func TestPPA001_ForgedUnsignedRevokeForVictimRejected(t *testing.T) {
+	t.Parallel()
+	hm, rt := newBoundHM(t, 1)
+
+	const victim = uint32(700)
+	const attacker = uint32(666)
+
+	hm.mu.Lock()
+	hm.trusted[victim] = &TrustRecord{NodeID: victim, ApprovedAt: time.Now()}
+	hm.mu.Unlock()
+
+	msg := &HandshakeMsg{
+		Type:      HandshakeRevoke,
+		NodeID:    victim,
+		Timestamp: time.Now().Unix(),
+	}
+	hm.processMessage(&addrStream{addr: coreapi.Addr{Node: attacker}}, msg)
+
+	hm.mu.RLock()
+	_, stillTrusted := hm.trusted[victim]
+	hm.mu.RUnlock()
+	if !stillTrusted {
+		t.Fatal("forged unsigned revoke from a non-matching authenticated node must not revoke trust")
+	}
+
+	rt.removedPeersMu.Lock()
+	removed := append([]uint32(nil), rt.removedPeers...)
+	rt.removedPeersMu.Unlock()
+	for _, id := range removed {
+		if id == victim {
+			t.Fatal("forged revoke must not tear down the victim's tunnel")
+		}
+	}
+}
+
+func TestPPA001_HonestRevokeOverMatchingStreamRevokes(t *testing.T) {
+	t.Parallel()
+	hm, rt := newBoundHM(t, 1)
+
+	const peer = uint32(800)
+	hm.mu.Lock()
+	hm.trusted[peer] = &TrustRecord{NodeID: peer, ApprovedAt: time.Now()}
+	hm.mu.Unlock()
+
+	msg := &HandshakeMsg{
+		Type:      HandshakeRevoke,
+		NodeID:    peer,
+		Timestamp: time.Now().Unix(),
+	}
+	hm.processMessage(&addrStream{addr: coreapi.Addr{Node: peer}}, msg)
+
+	hm.mu.RLock()
+	_, stillTrusted := hm.trusted[peer]
+	hm.mu.RUnlock()
+	if stillTrusted {
+		t.Fatal("honest revoke over a matching authenticated stream must revoke trust")
+	}
+
+	rt.removedPeersMu.Lock()
+	removed := append([]uint32(nil), rt.removedPeers...)
+	rt.removedPeersMu.Unlock()
+	var tornDown bool
+	for _, id := range removed {
+		if id == peer {
+			tornDown = true
+		}
+	}
+	if !tornDown {
+		t.Fatal("honest revoke must tear down the peer's tunnel")
+	}
+}

zz_ppa002_relayed_approval_precondition_test.go

@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+package handshake
+
+import (
+	"testing"
+	"time"
+)
+
+func TestPPA002_RelayedApprovalWithoutOutgoingDropped(t *testing.T) {
+	t.Parallel()
+	hm := newTestHM(t, "")
+	t.Cleanup(hm.Stop)
+
+	hm.ProcessRelayedApproval(4242)
+
+	hm.mu.RLock()
+	_, trusted := hm.trusted[4242]
+	hm.mu.RUnlock()
+	if trusted {
+		t.Fatal("relayed approval for a peer we never sent a request to must be dropped")
+	}
+}
+
+func TestPPA002_RelayedApprovalWithOutgoingTrusts(t *testing.T) {
+	t.Parallel()
+	hm := newTestHM(t, "")
+	t.Cleanup(hm.Stop)
+
+	hm.mu.Lock()
+	hm.outgoing[4242] = time.Now()
+	hm.mu.Unlock()
+
+	hm.ProcessRelayedApproval(4242)
+
+	hm.mu.RLock()
+	rec, trusted := hm.trusted[4242]
+	_, stillOutgoing := hm.outgoing[4242]
+	hm.mu.RUnlock()
+	if !trusted {
+		t.Fatal("relayed approval matching an outgoing request must establish trust")
+	}
+	if !rec.Mutual {
+		t.Fatal("relayed approval must record mutual trust")
+	}
+	if stillOutgoing {
+		t.Fatal("outgoing entry must be cleared once trust is established")
+	}
+}

zz_ppa008_replay_key_timestamp_test.go

@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+package handshake
+
+import (
+	"encoding/base64"
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/pilot-protocol/common/crypto"
+)
+
+func TestPPA008_SignedMessageReStampedIsDeduped(t *testing.T) {
+	t.Parallel()
+	rt := newTestRuntime()
+	rt.nodeID = 1
+	hm := NewManager(rt)
+	t.Cleanup(hm.Stop)
+
+	id, err := crypto.GenerateIdentity()
+	if err != nil {
+		t.Fatalf("generate identity: %v", err)
+	}
+
+	const peer = uint32(909)
+	challenge := fmt.Sprintf("handshake:%d:%d", peer, rt.nodeID)
+	sig := base64.StdEncoding.EncodeToString(id.Sign([]byte(challenge)))
+	pub := base64.StdEncoding.EncodeToString(id.PublicKey)
+
+	first := &HandshakeMsg{
+		Type:      HandshakeReject,
+		NodeID:    peer,
+		PublicKey: pub,
+		Signature: sig,
+		Timestamp: time.Now().Unix(),
+	}
+
+	hm.mu.Lock()
+	hm.outgoing[peer] = time.Now()
+	hm.mu.Unlock()
+	hm.processMessage(nil, first)
+
+	hm.mu.RLock()
+	_, clearedByFirst := hm.outgoing[peer]
+	hm.mu.RUnlock()
+	if clearedByFirst {
+		t.Fatal("first signed reject should have dispatched and cleared outgoing")
+	}
+
+	hm.mu.Lock()
+	hm.outgoing[peer] = time.Now()
+	hm.mu.Unlock()
+
+	reStamped := &HandshakeMsg{
+		Type:      HandshakeReject,
+		NodeID:    peer,
+		PublicKey: pub,
+		Signature: sig,
+		Timestamp: first.Timestamp + 7,
+	}
+	hm.processMessage(nil, reStamped)
+
+	hm.mu.RLock()
+	_, stillOutgoing := hm.outgoing[peer]
+	hm.mu.RUnlock()
+	if !stillOutgoing {
+		t.Fatal("re-stamped copy of a captured signed message must hit the replay set (timestamp must not be part of the key)")
+	}
+}
+
+func TestPPA008_DistinctSendersGetDistinctReplayKeys(t *testing.T) {
+	t.Parallel()
+	hm := newTestHM(t, "")
+	t.Cleanup(hm.Stop)
+
+	hm.mu.Lock()
+	hm.outgoing[11] = time.Now()
+	hm.outgoing[22] = time.Now()
+	hm.mu.Unlock()
+
+	ts := time.Now().Unix()
+	hm.processMessage(nil, &HandshakeMsg{Type: HandshakeReject, NodeID: 11, Timestamp: ts})
+	hm.processMessage(nil, &HandshakeMsg{Type: HandshakeReject, NodeID: 22, Timestamp: ts})
+
+	hm.mu.RLock()
+	_, out11 := hm.outgoing[11]
+	_, out22 := hm.outgoing[22]
+	hm.mu.RUnlock()
+	if out11 || out22 {
+		t.Fatal("distinct senders must get distinct replay keys and both dispatch — no false dedup")
+	}
+}