The official Mumble client has two protections Barnard was missing. After UDP decryption fails for over five seconds, it requests a cryptographic resync, retrying no more than every five seconds. It always accepts TCP-tunneled voice, even while UDP is active, because transports can briefly overlap during fallback. These are now implemented.

2026-09-08 12:20:30 -04:00
parent ca9c25f2d8
commit c1722c2491
6 changed files with 282 additions and 9 deletions
@@ -83,6 +83,8 @@ type Client struct {
 	udpProtobuf       bool
 	udpFallbackLogged atomic.Bool
 	udpFirstRecv      atomic.Bool
+	udpLastGood       time.Time
+	udpLastRequest    time.Time
 	cryptOut          cryptState // client→server encryption
 	cryptIn           cryptState // server→client encryption

@@ -328,14 +330,10 @@ func (c *Client) readRoutine() {
 		if err != nil {
 			break
 		}
-		// When UDP audio is active, ignore TCP-tunneled audio
-		// (packet type 1) to avoid double-processing packets.
-		c.udpMu.RLock()
-		udpActive := c.udpActive
-		c.udpMu.RUnlock()
-		if pType == 1 && udpActive {
-			continue
-		}
+		// Always accept TCP-tunneled audio, even after UDP has worked. The
+		// server can move a client back to the tunnel when the UDP return path
+		// fails, and UDP and TCP packets may briefly overlap during that
+		// transition. The per-user sequence handling rejects duplicates.
 		if int(pType) < len(handlers) {
 			handlers[pType](c, data)
 		}
@@ -131,6 +131,9 @@ func (c *Conn) WritePacket(ptype uint16, data []byte) error {
 	if err := c.writeHeader(uint16(ptype), uint32(len(data))); err != nil {
 		return err
 	}
+	if len(data) == 0 {
+		return nil
+	}
 	if _, err := c.Conn.Write(data); err != nil {
 		return err
 	}
@@ -7,6 +7,7 @@ import (
 	"encoding/binary"
 	"errors"
 	"sync"
+	"time"

 	"git.stormux.org/storm/barnard/gumble/gumble/MumbleProto"
 	"git.stormux.org/storm/barnard/log"
@@ -321,7 +322,39 @@ func (c *Client) handleCryptSetup(buffer []byte) error {
 			log.Info("received CryptSetup: key_len=%d client_nonce_len=%d server_nonce_len=%d",
 				len(packet.Key), len(packet.ClientNonce), len(packet.ServerNonce))
 		}
-	} else if !c.cryptOut.isInitialized() {
+	} else if packet.ServerNonce != nil {
+		c.udpMu.RLock()
+		inbound := c.udpCryptoIn
+		c.udpMu.RUnlock()
+		if inbound == nil {
+			return errors.New("gumble: received UDP resync before crypto initialization")
+		}
+		if err := inbound.setDecryptIV(packet.ServerNonce); err != nil {
+			return err
+		}
+		c.udpMu.Lock()
+		c.udpLastGood = time.Now()
+		c.udpLastRequest = time.Time{}
+		c.udpMu.Unlock()
+		log.Info("UDP crypto resynchronized from server nonce")
+	} else if c.cryptOut.isInitialized() {
+		// An empty CryptSetup is the server asking us for the current client
+		// nonce so it can resynchronize decryption in the other direction.
+		c.udpMu.RLock()
+		outbound := c.udpCryptoOut
+		c.udpMu.RUnlock()
+		if outbound == nil {
+			return errors.New("gumble: received UDP resync request without outbound crypto")
+		}
+		clientNonce, err := outbound.getEncryptIV()
+		if err != nil {
+			return err
+		}
+		if err := c.Conn.WriteProto(&MumbleProto.CryptSetup{ClientNonce: clientNonce}); err != nil {
+			return err
+		}
+		log.Info("sent client nonce for UDP crypto resync")
+	} else {
 		// Only log incomplete once before crypto is set up
 		log.Debug("received CryptSetup with incomplete fields, waiting for full key exchange")
 	}
@@ -3,6 +3,7 @@ package gumble
 import (
 	"net"
 	"testing"
+	"time"
 )

 // Regression: Mumble 1.5 decodes UDPTunnel packets as native protobuf UDP
@@ -38,3 +39,46 @@ func TestWriteAudioUsesProtobufEnvelopeForTCPFallback(t *testing.T) {
 		t.Fatalf("payload=%x want=%x", got.data, want)
 	}
 }
+
+// Regression: once an authenticated UDP response arrived, readRoutine used to
+// discard every later TCP UDPTunnel packet. If the server moved voice back to
+// TCP after the UDP return path failed, incoming audio stayed silent until the
+// client restarted and cleared udpActive.
+func TestReadRoutineAcceptsTCPAudioWhileUDPIsActive(t *testing.T) {
+	local, remote := net.Pipe()
+	config := NewConfig()
+	listener := &terminatorListener{packets: make(chan *AudioPacket, 1)}
+	config.AttachAudio(listener)
+	user := &User{Session: 1, Name: "speaker", decoder: &terminatorDecoder{}}
+	client := &Client{
+		Config:      config,
+		Conn:        NewConn(local),
+		Users:       Users{user.Session: user},
+		end:         make(chan struct{}),
+		state:       uint32(StateSynced),
+		udpActive:   true,
+		udpProtobuf: true,
+	}
+	go client.readRoutine()
+
+	// Native UDP type byte followed by MumbleUDP.Audio fields:
+	// sender_session=1, frame_number=1, opus_data={0x01}.
+	payload := mustDecodeHex("00180120012a0101")
+	if err := NewConn(remote).WritePacket(1, payload); err != nil {
+		t.Fatal(err)
+	}
+
+	select {
+	case packet := <-listener.packets:
+		if packet.Sender != user {
+			t.Fatalf("TCP-tunneled audio sender = %v; want %v", packet.Sender, user)
+		}
+	case <-time.After(time.Second):
+		t.Fatal("TCP-tunneled audio was discarded while UDP was active")
+	}
+
+	if err := remote.Close(); err != nil {
+		t.Fatal(err)
+	}
+	<-client.end
+}
@@ -8,7 +8,9 @@ import (
 	"errors"
 	"math"
 	"sync"
+	"time"

+	"git.stormux.org/storm/barnard/gumble/gumble/MumbleProto"
 	"git.stormux.org/storm/barnard/gumble/gumble/varint"
 	"git.stormux.org/storm/barnard/log"
 )
@@ -193,6 +195,7 @@ func decodeUDPAudio(data []byte) (session uint32, frameNumber uint64, opusData [
 const (
 	udp15BlockSize  = 16
 	udp15HeaderSize = 4 // 1 byte IV + 3 bytes tag
+	udpResyncDelay  = 5 * time.Second
 )

 // cryptState15 implements Mumble 1.5 native UDP encryption.
@@ -333,6 +336,31 @@ func (cs *cryptState15) decrypt15(packet []byte) ([]byte, error) {
 	return plaintext, nil
 }

+// setDecryptIV applies the server nonce returned after a CryptSetup resync
+// request while preserving the existing key and outbound crypto state.
+func (cs *cryptState15) setDecryptIV(serverNonce []byte) error {
+	cs.mu.Lock()
+	defer cs.mu.Unlock()
+	if !cs.initialized {
+		return errors.New("gumble: crypto not initialized")
+	}
+	if len(serverNonce) != len(cs.decryptIV) {
+		return errors.New("gumble: invalid server nonce")
+	}
+	copy(cs.decryptIV[:], serverNonce)
+	cs.history = [256]byte{}
+	return nil
+}
+
+func (cs *cryptState15) getEncryptIV() ([]byte, error) {
+	cs.mu.Lock()
+	defer cs.mu.Unlock()
+	if !cs.initialized {
+		return nil, errors.New("gumble: crypto not initialized")
+	}
+	return append([]byte(nil), cs.encryptIV[:]...), nil
+}
+
 // advanceIV increments a 16-byte IV as a little-endian integer.
 func advanceIV(iv []byte) {
 	for i := 0; i < len(iv); i++ {
@@ -579,6 +607,8 @@ func (c *Client) setUDP15Crypto(key, clientNonce, serverNonce []byte) error {
 	c.udpCryptoOut = outbound
 	c.udpCryptoIn = inbound
 	c.udpFrameNumber = 0
+	c.udpLastGood = time.Now()
+	c.udpLastRequest = time.Time{}
 	c.udpMu.Unlock()
 	c.udpWriteMu.Unlock()
 	log.Info("Mumble 1.5 native UDP crypto initialized")
@@ -674,6 +704,7 @@ func (c *Client) HandleUDPPacket15(packet []byte, pktNum uint64) {
 	plaintext, err := cs.decrypt15(packet)
 	if err != nil {
 		log.Warn("UDP15 #%d: decrypt failed: %v", pktNum, err)
+		c.requestUDPResync(time.Now())
 		return
 	}

@@ -716,9 +747,36 @@ func (c *Client) HandleUDPPacket15(packet []byte, pktNum uint64) {
 func (c *Client) markUDPActive() {
 	c.udpMu.Lock()
 	c.udpActive = true
+	c.udpLastGood = time.Now()
 	c.udpMu.Unlock()
 }

+// requestUDPResync mirrors Mumble's recovery for a UDP crypto stream that has
+// stopped decrypting. A request is sent only after five seconds without a good
+// packet and is rate-limited to one every five seconds until recovery.
+func (c *Client) requestUDPResync(now time.Time) {
+	c.udpMu.Lock()
+	lastGood := c.udpLastGood
+	lastRequest := c.udpLastRequest
+	if lastGood.IsZero() {
+		c.udpLastGood = now
+		c.udpMu.Unlock()
+		return
+	}
+	if now.Sub(lastGood) <= udpResyncDelay || (!lastRequest.IsZero() && now.Sub(lastRequest) <= udpResyncDelay) {
+		c.udpMu.Unlock()
+		return
+	}
+	c.udpLastRequest = now
+	c.udpMu.Unlock()
+
+	if err := c.Conn.WriteProto(&MumbleProto.CryptSetup{}); err != nil {
+		log.Warn("requesting UDP crypto resync failed: %v", err)
+		return
+	}
+	log.Info("requested UDP crypto resync after sustained decrypt failures")
+}
+
 // 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) {
@@ -0,0 +1,137 @@
+package gumble
+
+import (
+	"net"
+	"testing"
+	"time"
+
+	"git.stormux.org/storm/barnard/gumble/gumble/MumbleProto"
+	"google.golang.org/protobuf/proto"
+)
+
+func TestUDPResyncWaitsForSustainedDecryptFailure(t *testing.T) {
+	now := time.Now()
+	client := &Client{udpLastGood: now.Add(-4 * time.Second)}
+
+	client.requestUDPResync(now)
+
+	if !client.udpLastRequest.IsZero() {
+		t.Fatal("UDP resync was requested before five seconds without a good packet")
+	}
+}
+
+func TestUDPResyncRequestUsesEmptyCryptSetup(t *testing.T) {
+	local, remote := net.Pipe()
+	defer local.Close()
+	defer remote.Close()
+	now := time.Now()
+	client := &Client{
+		Conn:        NewConn(local),
+		udpLastGood: now.Add(-6 * time.Second),
+	}
+	received := make(chan struct {
+		packetType uint16
+		data       []byte
+		err        error
+	}, 1)
+	go func() {
+		packetType, data, err := NewConn(remote).ReadPacket()
+		received <- struct {
+			packetType uint16
+			data       []byte
+			err        error
+		}{packetType, data, err}
+	}()
+
+	client.requestUDPResync(now)
+	packet := <-received
+	if packet.err != nil {
+		t.Fatal(packet.err)
+	}
+	if packet.packetType != 15 || len(packet.data) != 0 {
+		t.Fatalf("resync packet: type=%d data=%x; want empty CryptSetup", packet.packetType, packet.data)
+	}
+	if !client.udpLastRequest.Equal(now) {
+		t.Fatalf("last request = %v; want %v", client.udpLastRequest, now)
+	}
+
+	client.requestUDPResync(now.Add(time.Second))
+	if !client.udpLastRequest.Equal(now) {
+		t.Fatal("rate-limited resync changed the last-request time")
+	}
+}
+
+func TestCryptSetupServerNonceResynchronizesInboundUDP(t *testing.T) {
+	key := make([]byte, 16)
+	clientNonce := make([]byte, 16)
+	serverNonce := make([]byte, 16)
+	serverNonce[0] = 7
+	client := &Client{}
+	if err := client.setUDP15Crypto(key, clientNonce, make([]byte, 16)); err != nil {
+		t.Fatal(err)
+	}
+	client.udpLastRequest = time.Now()
+	data, err := proto.Marshal(&MumbleProto.CryptSetup{ServerNonce: serverNonce})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if err := client.handleCryptSetup(data); err != nil {
+		t.Fatal(err)
+	}
+
+	client.udpCryptoIn.mu.Lock()
+	gotNonce := client.udpCryptoIn.decryptIV
+	client.udpCryptoIn.mu.Unlock()
+	if gotNonce != [16]byte{7} {
+		t.Fatalf("inbound nonce = %x; want %x", gotNonce, serverNonce)
+	}
+	if !client.udpLastRequest.IsZero() {
+		t.Fatal("successful resync did not clear the request timer")
+	}
+}
+
+func TestEmptyCryptSetupReturnsCurrentClientNonce(t *testing.T) {
+	local, remote := net.Pipe()
+	defer local.Close()
+	defer remote.Close()
+	key := make([]byte, 16)
+	clientNonce := make([]byte, 16)
+	clientNonce[0] = 9
+	client := &Client{Conn: NewConn(local)}
+	if err := client.cryptOut.setup(key, clientNonce); err != nil {
+		t.Fatal(err)
+	}
+	if err := client.setUDP15Crypto(key, clientNonce, make([]byte, 16)); err != nil {
+		t.Fatal(err)
+	}
+	received := make(chan struct {
+		packetType uint16
+		data       []byte
+		err        error
+	}, 1)
+	go func() {
+		packetType, data, err := NewConn(remote).ReadPacket()
+		received <- struct {
+			packetType uint16
+			data       []byte
+			err        error
+		}{packetType, data, err}
+	}()
+
+	if err := client.handleCryptSetup(nil); err != nil {
+		t.Fatal(err)
+	}
+	packet := <-received
+	if packet.err != nil {
+		t.Fatal(packet.err)
+	}
+	var response MumbleProto.CryptSetup
+	if err := proto.Unmarshal(packet.data, &response); err != nil {
+		t.Fatal(err)
+	}
+	if packet.packetType != 15 || string(response.ClientNonce) != string(clientNonce) {
+		t.Fatalf("resync response: type=%d nonce=%x; want type=15 nonce=%x",
+			packet.packetType, response.ClientNonce, clientNonce)
+	}
+}