diff --git a/gumble/gumble/client.go b/gumble/gumble/client.go index b158fd1..6b7705f 100644 --- a/gumble/gumble/client.go +++ b/gumble/gumble/client.go @@ -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) } diff --git a/gumble/gumble/conn.go b/gumble/gumble/conn.go index 6703958..88b84ca 100644 --- a/gumble/gumble/conn.go +++ b/gumble/gumble/conn.go @@ -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 } diff --git a/gumble/gumble/crypt.go b/gumble/gumble/crypt.go index 152292e..405c11b 100644 --- a/gumble/gumble/crypt.go +++ b/gumble/gumble/crypt.go @@ -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") } diff --git a/gumble/gumble/tcp_audio_protobuf_test.go b/gumble/gumble/tcp_audio_protobuf_test.go index b17ea2c..aad4985 100644 --- a/gumble/gumble/tcp_audio_protobuf_test.go +++ b/gumble/gumble/tcp_audio_protobuf_test.go @@ -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 +} diff --git a/gumble/gumble/udp15.go b/gumble/gumble/udp15.go index 4912822..cd8deba 100644 --- a/gumble/gumble/udp15.go +++ b/gumble/gumble/udp15.go @@ -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) { diff --git a/gumble/gumble/udp_resync_regression_test.go b/gumble/gumble/udp_resync_regression_test.go new file mode 100644 index 0000000..b201073 --- /dev/null +++ b/gumble/gumble/udp_resync_regression_test.go @@ -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) + } +}