diff --git a/fix.txt b/fix.txt index 66a9f17..501511a 100644 --- a/fix.txt +++ b/fix.txt @@ -70,7 +70,7 @@ Priority 0: security and crashers Priority 1: transport, lifecycle, and correctness --------------------------------------------------- -8. TCP audio is discarded before UDP is proven usable +[x] 8. TCP audio is discarded before UDP is proven usable Files: gumble/gumble/crypt.go, gumble/gumble/client.go, gumble/gumble/udp.go udpActive is set immediately after CryptSetup. The TCP read routine then discards UDPTunnel packets even if inbound UDP is blocked or NAT setup has diff --git a/gumble/gumble/client.go b/gumble/gumble/client.go index 3c946f6..43054b6 100644 --- a/gumble/gumble/client.go +++ b/gumble/gumble/client.go @@ -74,6 +74,7 @@ type Client struct { udpMu sync.RWMutex udpWriteMu sync.Mutex udpConn *net.UDPConn + udpStarted bool udpActive bool udpCryptoOut *cryptState15 udpCryptoIn *cryptState15 diff --git a/gumble/gumble/crypt.go b/gumble/gumble/crypt.go index ce7d8e8..4907c34 100644 --- a/gumble/gumble/crypt.go +++ b/gumble/gumble/crypt.go @@ -317,9 +317,11 @@ func (c *Client) handleCryptSetup(buffer []byte) error { c.udpMu.Lock() udpReady := c.udpCryptoOut != nil - startUDP := c.cryptOut.initialized && c.udpConn != nil && !c.udpActive && udpReady + startUDP := c.cryptOut.initialized && c.udpConn != nil && !c.udpStarted && udpReady if startUDP { - c.udpActive = true + // Keep TCP tunnelling enabled until an authenticated UDP packet proves + // that the inbound path works. + c.udpStarted = true } noUDPConn := c.udpConn == nil c.udpMu.Unlock() diff --git a/gumble/gumble/udp15.go b/gumble/gumble/udp15.go index 61df239..84da381 100644 --- a/gumble/gumble/udp15.go +++ b/gumble/gumble/udp15.go @@ -662,6 +662,7 @@ func (c *Client) HandleUDPPacket15(packet []byte, pktNum uint64) { } log.Debug("UDP15 #%d: decrypt OK, plaintext_len=%d", pktNum, len(plaintext)) + c.markUDPActive() // Check type byte (0x00 = Audio, 0x01 = Ping) if len(plaintext) < 1 { @@ -692,6 +693,14 @@ func (c *Client) HandleUDPPacket15(packet []byte, pktNum uint64) { log.Warn("UDP15 #%d: unknown message type 0x%02x", pktNum, msgType) } +// markUDPActive switches outgoing audio to UDP only after authentication has +// proved that packets can return through the network path. +func (c *Client) markUDPActive() { + c.udpMu.Lock() + c.udpActive = true + c.udpMu.Unlock() +} + // dispatchOpus15 processes a decoded MumbleUDP.Audio frame and dispatches // the decoded PCM to audio listeners. func (c *Client) dispatchOpus15(pktNum uint64, session uint32, frameNum int64, opusData []byte, terminator bool, context uint32, position *[3]float32, volumeAdjustment float32) { diff --git a/gumble/gumble/udp_fallback_regression_test.go b/gumble/gumble/udp_fallback_regression_test.go new file mode 100644 index 0000000..66e70c2 --- /dev/null +++ b/gumble/gumble/udp_fallback_regression_test.go @@ -0,0 +1,19 @@ +package gumble + +import "testing" + +// Regression: CryptSetup selected UDP before any authenticated packet had +// returned, causing TCP audio to be discarded on blocked inbound UDP paths. +func TestUDPOnlyBecomesActiveAfterAuthenticatedResponse(t *testing.T) { + c := &Client{} + if c.udpActive { + t.Fatal("new transport is unexpectedly active") + } + c.markUDPActive() + c.udpMu.RLock() + active := c.udpActive + c.udpMu.RUnlock() + if !active { + t.Fatal("authenticated UDP response did not activate UDP") + } +}