From d429dab5dd0dbf53612104f4b1d1ebb5303b4e6c Mon Sep 17 00:00:00 2001 From: JackDoan Date: Thu, 14 May 2026 14:07:12 -0500 Subject: [PATCH] tweaks --- noiseutil/cipher_state_test.go | 56 ++++++++++++++++++++++++++++++++++ outside.go | 17 ++++++----- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/noiseutil/cipher_state_test.go b/noiseutil/cipher_state_test.go index a4df01e9..14070748 100644 --- a/noiseutil/cipher_state_test.go +++ b/noiseutil/cipher_state_test.go @@ -147,6 +147,62 @@ func buildCipherStatesB(b *testing.B, c noise.CipherFunc) (*noise.CipherState, * return eI, dR } +// TestDecryptDangerRelayShapeNoAlloc covers the AD-only relay path used in +// outside.go's handleOutsideRelayPacket: the body is AD, the trailing 16 bytes +// are the AEAD tag, the plaintext is empty, and the caller passes nil as the +// destination because it only needs the auth side-effect. The call must +// succeed, return an empty plaintext, and not allocate on the hot path. +func TestDecryptDangerRelayShapeNoAlloc(t *testing.T) { + cases := []struct { + name string + c noise.CipherFunc + wrap func(*noise.CipherState) CipherState + }{ + {"AESGCM", CipherAESGCM, func(cs *noise.CipherState) CipherState { return NewCipherStateAESGCM(cs) }}, + {"ChaChaPoly", noise.CipherChaChaPoly, func(cs *noise.CipherState) CipherState { return NewCipherStateChaChaPoly(cs) }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + encCS, decCS := buildCipherStates(t, tc.c) + enc, dec := tc.wrap(encCS), tc.wrap(decCS) + + ad := make([]byte, 1200) // typical relay packet body size + for i := range ad { + ad[i] = byte(i) + } + nb := make([]byte, 12) + + // Build the "signature value" the way handleOutsideRelayPacket sees it: + // empty plaintext encrypted with the body as AD yields just the 16-byte tag. + tag, err := enc.EncryptDanger(nil, ad, nil, 1, nb) + require.NoError(t, err) + require.Len(t, tag, dec.Overhead()) + + // Sanity: the relay-shaped call returns empty plaintext, no error. + out, err := dec.DecryptDanger(nil, ad, tag, 1, nb) + require.NoError(t, err) + assert.Empty(t, out) + + // Tampering with the AD must fail authentication. + ad[0] ^= 0xff + _, err = dec.DecryptDanger(nil, ad, tag, 1, nb) + require.Error(t, err) + ad[0] ^= 0xff + + // The hot path must not allocate. AllocsPerRun does a warm-up run, so any + // one-time setup is excluded. Counter has to advance so the AEAD nonce is + // unique per call, but we don't care whether the auth succeeds — we only + // care about whether the call path allocates. + var counter uint64 = 2 + allocs := testing.AllocsPerRun(100, func() { + _, _ = dec.DecryptDanger(nil, ad, tag, counter, nb) + counter++ + }) + assert.Equal(t, 0.0, allocs, "DecryptDanger(nil, ...) must not allocate") + }) + } +} + func TestCipherStateNilSafety(t *testing.T) { var aes *CipherStateAESGCM _, err := aes.EncryptDanger(nil, nil, nil, 0, make([]byte, 12)) diff --git a/outside.go b/outside.go index df4833e6..4a40210d 100644 --- a/outside.go +++ b/outside.go @@ -23,7 +23,7 @@ const ( var ErrOutOfWindow = errors.New("out of window packet") -func (f *Interface) readOutsidePackets(via ViaSender, out []byte, packet []byte, h *header.H, fwPacket *firewall.Packet, lhf *LightHouseHandler, nb []byte, q int, localCache firewall.ConntrackCache, meta udp.RxMeta) { +func (f *Interface) readOutsidePackets(via ViaSender, packet []byte, h *header.H, fwPacket *firewall.Packet, lhf *LightHouseHandler, nb []byte, q int, localCache firewall.ConntrackCache, meta udp.RxMeta) { err := h.Parse(packet) if err != nil { // Hole punch packets are 0 or 1 byte big, so lets ignore printing those errors @@ -111,10 +111,11 @@ func (f *Interface) readOutsidePackets(via ViaSender, out []byte, packet []byte, // Relay packets are special if isMessageRelay { - f.handleOutsideRelayPacket(hostinfo, via, out, packet, h, fwPacket, lhf, nb, q, localCache, meta) + f.handleOutsideRelayPacket(hostinfo, via, packet, h, fwPacket, lhf, nb, q, localCache, meta) return } + out := f.batchers[q].Reserve(len(packet))[:0] out, err = f.decrypt(hostinfo, h.MessageCounter, out, packet, h, nb) if err != nil { if f.l.Enabled(context.Background(), slog.LevelDebug) { @@ -168,7 +169,7 @@ func (f *Interface) readOutsidePackets(via ViaSender, out []byte, packet []byte, } } -func (f *Interface) handleOutsideRelayPacket(hostinfo *HostInfo, via ViaSender, out []byte, packet []byte, h *header.H, fwPacket *firewall.Packet, lhf *LightHouseHandler, nb []byte, q int, localCache firewall.ConntrackCache, meta udp.RxMeta) { +func (f *Interface) handleOutsideRelayPacket(hostinfo *HostInfo, via ViaSender, packet []byte, h *header.H, fwPacket *firewall.Packet, lhf *LightHouseHandler, nb []byte, q int, localCache firewall.ConntrackCache, meta udp.RxMeta) { // The entire body is sent as AD, not encrypted. // The packet consists of a 16-byte parsed Nebula header, Associated Data-protected payload, and a trailing 16-byte AEAD signature value. // The packet is guaranteed to be at least 16 bytes at this point, b/c it got past the h.Parse() call above. If it's @@ -176,9 +177,10 @@ func (f *Interface) handleOutsideRelayPacket(hostinfo *HostInfo, via ViaSender, // which will gracefully fail in the DecryptDanger call. signedPayload := packet[:len(packet)-hostinfo.ConnectionState.dKey.Overhead()] signatureValue := packet[len(packet)-hostinfo.ConnectionState.dKey.Overhead():] - var err error - out, err = hostinfo.ConnectionState.dKey.DecryptDanger(out, signedPayload, signatureValue, h.MessageCounter, nb) - if err != nil { + // The decrypted output is empty (relay packets carry their payload as AD) and unused. + // The recursive readOutsidePackets call below operates on signedPayload. Passing + // nil avoids reserving an arena slot. + if _, err := hostinfo.ConnectionState.dKey.DecryptDanger(nil, signedPayload, signatureValue, h.MessageCounter, nb); err != nil { return } // Successfully validated the thing. Get rid of the Relay header. @@ -211,7 +213,7 @@ func (f *Interface) handleOutsideRelayPacket(hostinfo *HostInfo, via ViaSender, relay: relay, IsRelayed: true, } - f.readOutsidePackets(via, out[:0], signedPayload, h, fwPacket, lhf, nb, q, localCache, meta) + f.readOutsidePackets(via, signedPayload, h, fwPacket, lhf, nb, q, localCache, meta) case ForwardingType: // Find the target HostInfo relay object targetHI, targetRelay, err := f.hostMap.QueryVpnAddrsRelayFor(hostinfo.vpnAddrs, relay.PeerAddr) @@ -230,6 +232,7 @@ func (f *Interface) handleOutsideRelayPacket(hostinfo *HostInfo, via ViaSender, case ForwardingType: // Forward this packet through the relay tunnel // Find the target HostInfo //todo it would potentially be nice to batch these + out := f.batchers[q].Reserve(len(packet) + header.Len + hostinfo.ConnectionState.dKey.Overhead())[:0] f.SendVia(targetHI, targetRelay, signedPayload, nb, out, false) case TerminalType: hostinfo.logger(f.l).Error("Unexpected Relay Type of Terminal")