Skip to content

Commit 4e0efab

Browse files
TeoSlayerteovl
andauthored
Bind inner packet Src to authenticated peerNodeID (#294)
After AEAD decrypt, drop any frame whose inner Src.Node disagrees with the authenticated frame peerNodeID, so a node holding one valid session cannot forge packets impersonating another node to the SYN trust gate or to apps. Relay frames re-enter the same path with peerNodeID from the inner secure frame. Adds TestHandleEncryptedDropsSpoofedSrc; 3 fixtures set Src=peer. Co-authored-by: Teodor Calin <teodor@vulturelabs.io>
1 parent 2cecbf5 commit 4e0efab

4 files changed

Lines changed: 82 additions & 0 deletions

File tree

pkg/daemon/tunnel.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,6 +1367,28 @@ func (tm *TunnelManager) handleEncrypted(data []byte, from *net.UDPAddr) {
13671367
return
13681368
}
13691369

1370+
// Identity binding (transport security). The AEAD authenticated the
1371+
// frame-header peerNodeID: reaching this point proves the sender holds
1372+
// the session key bound to it. The inner packet's Src, by contrast, is
1373+
// plaintext the sender chose freely. Reject any frame whose inner
1374+
// Src.Node disagrees with the authenticated peerNodeID — otherwise a
1375+
// node holding one valid session could forge packets impersonating any
1376+
// other node to the SYN trust gate and to applications (which read
1377+
// conn.RemoteAddr().Node). Relay-delivered frames re-enter this path with
1378+
// peerNodeID taken from the inner secure frame's AEAD, never the beacon's
1379+
// untrusted srcNodeID, so the check covers direct and relayed traffic
1380+
// alike. Src.Network may vary across a node's networks; identity is the
1381+
// node id, so only .Node is compared.
1382+
if pkt.Src.Node != peerNodeID {
1383+
slog.Warn("tunnel: dropping frame with spoofed source node",
1384+
"authenticated_peer", peerNodeID, "claimed_src", pkt.Src.Node)
1385+
tm.publishEvent("security.src_spoofed", map[string]interface{}{
1386+
"authenticated_peer": peerNodeID,
1387+
"claimed_src": pkt.Src.Node,
1388+
})
1389+
return
1390+
}
1391+
13701392
// v1.9.1 NAT-keepalive: drop tunnel-keepalive packets before recvCh
13711393
// delivery. They're authenticated (the AEAD verified the sender's
13721394
// nodeID AAD) so we still want all the side-effects below (recordInbound

pkg/daemon/zz_nat_keepalive_bug_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ func TestHandleEncryptedDropsKeepaliveBeforeRecvCh(t *testing.T) {
202202
Version: protocol.Version,
203203
Protocol: protocol.ProtoControl,
204204
DstPort: protocol.PortPing,
205+
Src: protocol.Addr{Node: peerNodeID}, // inner Src == authenticated peerNodeID
205206
}
206207
plaintext, err := ka.Marshal()
207208
if err != nil {

pkg/daemon/zz_nat_remap_bug_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ func TestPeerNATRemapNotLearnedOnDecrypt(t *testing.T) {
9999
// Build a real encrypted frame the way the peer would, then deliver
100100
// it via handleEncrypted with from=addrNew (the post-remap source).
101101
pkt := newPacket("post-nat-remap-payload")
102+
pkt.Src.Node = peerNodeID // well-behaved peer: inner Src == authenticated peerNodeID
102103
plaintext, err := pkt.Marshal()
103104
if err != nil {
104105
t.Fatalf("Marshal: %v", err)

pkg/daemon/zz_tunnel_handle_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,7 @@ func TestHandleEncryptedValidDeliversToRecvCh(t *testing.T) {
529529
// Build a real encrypted frame that handleEncrypted will accept:
530530
// nonce = prefix(4) || counter(8) ; AAD = BE(peerNodeID)
531531
pkt := newPacket("encrypted-payload")
532+
pkt.Src.Node = peerNodeID // well-behaved peer: inner Src == authenticated peerNodeID
532533
plaintext, err := pkt.Marshal()
533534
if err != nil {
534535
t.Fatalf("Marshal: %v", err)
@@ -560,6 +561,63 @@ func TestHandleEncryptedValidDeliversToRecvCh(t *testing.T) {
560561
}
561562
}
562563

564+
// TestHandleEncryptedDropsSpoofedSrc proves the transport identity-binding
565+
// fix: a frame that authenticates under peerNodeID but carries a DIFFERENT
566+
// inner Src.Node is dropped, not delivered. Without the fix a node holding
567+
// one valid session could impersonate any other node to applications.
568+
func TestHandleEncryptedDropsSpoofedSrc(t *testing.T) {
569+
t.Parallel()
570+
tm := NewTunnelManager()
571+
if err := tm.EnableEncryption(); err != nil {
572+
t.Fatalf("EnableEncryption: %v", err)
573+
}
574+
curve := ecdh.X25519()
575+
peerPriv, err := curve.GenerateKey(rand.Reader)
576+
if err != nil {
577+
t.Fatalf("peer keygen: %v", err)
578+
}
579+
pc, err := tm.deriveSecret(peerPriv.PublicKey().Bytes())
580+
if err != nil {
581+
t.Fatalf("deriveSecret: %v", err)
582+
}
583+
const peerNodeID = uint32(0x11223344)
584+
tm.mu.Lock()
585+
tm.envelope.Install(peerNodeID, pc)
586+
tm.mu.Unlock()
587+
588+
// Authenticated peer is 0x11223344, but the inner packet claims a
589+
// DIFFERENT source node — a forged source.
590+
pkt := newPacket("spoofed-payload")
591+
pkt.Src.Node = 0x99999999 // != peerNodeID
592+
plaintext, err := pkt.Marshal()
593+
if err != nil {
594+
t.Fatalf("Marshal: %v", err)
595+
}
596+
nonce := make([]byte, pc.AEAD.NonceSize())
597+
copy(nonce[0:4], pc.NoncePrefix[:])
598+
binary.BigEndian.PutUint64(nonce[4:12], 1)
599+
aad := make([]byte, 4)
600+
binary.BigEndian.PutUint32(aad, peerNodeID)
601+
ct := pc.AEAD.Seal(nil, nonce, plaintext, aad)
602+
603+
data := make([]byte, 4+12+len(ct))
604+
binary.BigEndian.PutUint32(data[0:4], peerNodeID)
605+
copy(data[4:16], nonce)
606+
copy(data[16:], ct)
607+
608+
tm.handleEncrypted(data, mustUDPAddr(t, "127.0.0.1:5555"))
609+
610+
if got := atomic.LoadUint64(&tm.PktsRecv); got != 0 {
611+
t.Fatalf("spoofed-Src frame must not count as received; PktsRecv = %d", got)
612+
}
613+
select {
614+
case got := <-tm.RecvCh():
615+
t.Fatalf("spoofed-Src frame must NOT be delivered, got payload %q", got.Packet.Payload)
616+
case <-time.After(100 * time.Millisecond):
617+
// correct: nothing delivered
618+
}
619+
}
620+
563621
func TestHandleEncryptedPlaintextNotAPacketReturns(t *testing.T) {
564622
t.Parallel()
565623
tm := NewTunnelManager()

0 commit comments

Comments
 (0)