Retain TCP audio until UDP is authenticated
This commit is contained in:
committed by
Brandon McGinty
parent
1fa96d4f15
commit
25467405b8
@@ -70,7 +70,7 @@ Priority 0: security and crashers
|
|||||||
Priority 1: transport, lifecycle, and correctness
|
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
|
Files: gumble/gumble/crypt.go, gumble/gumble/client.go, gumble/gumble/udp.go
|
||||||
udpActive is set immediately after CryptSetup. The TCP read routine then
|
udpActive is set immediately after CryptSetup. The TCP read routine then
|
||||||
discards UDPTunnel packets even if inbound UDP is blocked or NAT setup has
|
discards UDPTunnel packets even if inbound UDP is blocked or NAT setup has
|
||||||
|
|||||||
@@ -74,6 +74,7 @@ type Client struct {
|
|||||||
udpMu sync.RWMutex
|
udpMu sync.RWMutex
|
||||||
udpWriteMu sync.Mutex
|
udpWriteMu sync.Mutex
|
||||||
udpConn *net.UDPConn
|
udpConn *net.UDPConn
|
||||||
|
udpStarted bool
|
||||||
udpActive bool
|
udpActive bool
|
||||||
udpCryptoOut *cryptState15
|
udpCryptoOut *cryptState15
|
||||||
udpCryptoIn *cryptState15
|
udpCryptoIn *cryptState15
|
||||||
|
|||||||
@@ -317,9 +317,11 @@ func (c *Client) handleCryptSetup(buffer []byte) error {
|
|||||||
|
|
||||||
c.udpMu.Lock()
|
c.udpMu.Lock()
|
||||||
udpReady := c.udpCryptoOut != nil
|
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 {
|
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
|
noUDPConn := c.udpConn == nil
|
||||||
c.udpMu.Unlock()
|
c.udpMu.Unlock()
|
||||||
|
|||||||
@@ -662,6 +662,7 @@ func (c *Client) HandleUDPPacket15(packet []byte, pktNum uint64) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
log.Debug("UDP15 #%d: decrypt OK, plaintext_len=%d", pktNum, len(plaintext))
|
log.Debug("UDP15 #%d: decrypt OK, plaintext_len=%d", pktNum, len(plaintext))
|
||||||
|
c.markUDPActive()
|
||||||
|
|
||||||
// Check type byte (0x00 = Audio, 0x01 = Ping)
|
// Check type byte (0x00 = Audio, 0x01 = Ping)
|
||||||
if len(plaintext) < 1 {
|
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)
|
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
|
// dispatchOpus15 processes a decoded MumbleUDP.Audio frame and dispatches
|
||||||
// the decoded PCM to audio listeners.
|
// 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) {
|
func (c *Client) dispatchOpus15(pktNum uint64, session uint32, frameNum int64, opusData []byte, terminator bool, context uint32, position *[3]float32, volumeAdjustment float32) {
|
||||||
|
|||||||
@@ -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")
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user