From c398b2631f2cf83829334dd273792b23a63b0965 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Sat, 20 Jun 2026 01:29:23 +0200 Subject: [PATCH 1/3] feat(webrtc): support webrtc-direct v2 Chrome is removing the SDP munging that v1 browser-to-server dials rely on (the WebRTC-NoSdpMangleUfrag field trial), so the listener now also speaks the v2 flow, which carries the client's ICE password in the server ufrag instead of munging. Implements libp2p/specs#715. - listener: parse both halves of the STUN USERNAME, dispatch on the ufrag prefix (v1, v2, or reject an unknown version), recover the v2 client password, and set pion's ICE credentials to match. - udpmux: surface the server and client ufrag separately and key the mux on the server (local) ufrag, which is what pion looks up. - validation: reject STUN USERNAME fragments outside the RFC 8839 ice-char set or length bounds before they reach SDP. - dialer: add opt-in WithDialerVersion(2) that emits the v2 wire format for go-to-go and testing; v1 stays the default. - tests: parsing/validation units plus end-to-end v1 and v2 dials against a single listener. --- p2p/transport/webrtc/listener.go | 103 ++++++++++++++++++++- p2p/transport/webrtc/sdp.go | 9 +- p2p/transport/webrtc/sdp_test.go | 47 +++++++++- p2p/transport/webrtc/transport.go | 93 ++++++++++++++----- p2p/transport/webrtc/transport_test.go | 113 ++++++++++++++++++++++++ p2p/transport/webrtc/udpmux/mux.go | 47 ++++++---- p2p/transport/webrtc/udpmux/mux_test.go | 38 ++++++++ 7 files changed, 402 insertions(+), 48 deletions(-) diff --git a/p2p/transport/webrtc/listener.go b/p2p/transport/webrtc/listener.go index 4616f0cfb6..ee01e6d140 100644 --- a/p2p/transport/webrtc/listener.go +++ b/p2p/transport/webrtc/listener.go @@ -32,6 +32,67 @@ var _ network.ConnMultiaddrs = &connMultiaddrs{} func (c *connMultiaddrs) LocalMultiaddr() ma.Multiaddr { return c.local } func (c *connMultiaddrs) RemoteMultiaddr() ma.Multiaddr { return c.remote } +// The "libp2p+webrtc+" namespace selects the WebRTC Direct handshake version via +// the ICE username fragment. v1 ("libp2p+webrtc+v1/") munges the SDP; v2 +// ("libp2p+webrtc+v2/") does not, because Chromium's WebRTC-NoSdpMangleUfrag +// field trial rejects an offer whose ICE ufrag was munged. A v2 client instead +// leaves its local credentials untouched and puts its own ICE password in the +// server ufrag as "libp2p+webrtc+v2/", which the server reads back +// from the STUN USERNAME with no SDP exchange. A server MUST reject a version it +// does not recognize rather than guess one. v2 was introduced in +// https://github.com/libp2p/specs/pull/715; the spec lives at +// https://github.com/libp2p/specs/blob/master/webrtc/webrtc-direct.md and the +// js-libp2p implementation at https://github.com/libp2p/js-libp2p/pull/3480. +const ( + ufragPrefixV1 = "libp2p+webrtc+v1/" + ufragPrefixV2 = "libp2p+webrtc+v2/" +) + +const ( + // ICE credential bounds, per RFC 8839 section 5.4 + // (ufrag = 4*256ice-char, password = 22*256ice-char). + iceUfragMinLen = 4 + icePwdMinLen = 22 + iceCredentialMax = 256 +) + +// ice-char is ASCII-only, so any string that passes isICECharString has exactly +// one byte per character. The len() byte count used by isICEUfrag/isICEPwd then +// equals both the ice-char count and the UTF-16 length that js-libp2p measures, +// so the two implementations accept the same credentials; non-ice-char input +// (including any multi-byte UTF-8) is rejected by both regardless of how each +// counts length. + +// isICEChar reports whether b is in the RFC 8839 ice-char set +// (ALPHA / DIGIT / "+" / "/"). +func isICEChar(b byte) bool { + return (b >= 'a' && b <= 'z') || + (b >= 'A' && b <= 'Z') || + (b >= '0' && b <= '9') || + b == '+' || b == '/' +} + +func isICECharString(s string) bool { + for i := 0; i < len(s); i++ { + if !isICEChar(s[i]) { + return false + } + } + return true +} + +// isICEUfrag reports whether s is a syntactically valid ICE username fragment +// (RFC 8839 section 5.4: ufrag = 4*256ice-char). +func isICEUfrag(s string) bool { + return len(s) >= iceUfragMinLen && len(s) <= iceCredentialMax && isICECharString(s) +} + +// isICEPwd reports whether s is a syntactically valid ICE password +// (RFC 8839 section 5.4: password = 22*256ice-char). +func isICEPwd(s string) bool { + return len(s) >= icePwdMinLen && len(s) <= iceCredentialMax && isICECharString(s) +} + const ( candidateSetupTimeout = 10 * time.Second // This is higher than other transports(64) as there's no way to detect a peer that has gone away after @@ -196,9 +257,41 @@ func (l *listener) setupConnection( } }() + // candidate.Ufrag is the server (local) ufrag and candidate.RemoteUfrag is the + // client ufrag, parsed from the STUN USERNAME ("server_ufrag:client_ufrag", + // RFC 8445 section 7.2.2). Both come from an attacker-controlled STUN packet + // and are templated into the inferred SDP offer and pion's ICE credentials, so + // reject anything that is not a valid ICE username fragment (ice-char, length + // 4..256) per RFC 8839 section 5.4 before using it. + serverUfrag := candidate.Ufrag + clientUfrag := candidate.RemoteUfrag + if !isICEUfrag(serverUfrag) || !isICEUfrag(clientUfrag) { + return nil, fmt.Errorf("invalid ICE ufrag in STUN username") + } + // Select the version explicitly from the server ufrag prefix. The client + // password lets us render a valid remote (client) SDP offer: in v1 the client + // uses one shared value (client password == client ufrag); in v2 the client + // encodes its password into the server ufrag as "libp2p+webrtc+v2/". + // An unrecognized version is rejected, never treated as v1. See https://github.com/libp2p/specs/blob/master/webrtc/webrtc-direct.md. + var clientPwd string + switch { + case strings.HasPrefix(serverUfrag, ufragPrefixV2): + pwd := strings.TrimPrefix(serverUfrag, ufragPrefixV2) + // The recovered value becomes the inferred offer's ice-pwd, so it must be a + // valid ICE password (ice-char, length 22..256) per RFC 8839 section 5.4. + if !isICEPwd(pwd) { + return nil, fmt.Errorf("invalid v2 ufrag %q: recovered client password is not a valid ICE password", serverUfrag) + } + clientPwd = pwd + case strings.HasPrefix(serverUfrag, ufragPrefixV1): + clientPwd = clientUfrag + default: + return nil, fmt.Errorf("unsupported WebRTC Direct version in ufrag %q", serverUfrag) + } + settingEngine := webrtc.SettingEngine{LoggerFactory: pionLoggerFactory} settingEngine.SetAnsweringDTLSRole(webrtc.DTLSRoleServer) - settingEngine.SetICECredentials(candidate.Ufrag, candidate.Ufrag) + settingEngine.SetICECredentials(serverUfrag, serverUfrag) settingEngine.SetLite(true) settingEngine.SetICEUDPMux(l.mux) settingEngine.SetIncludeLoopbackCandidate(true) @@ -224,9 +317,11 @@ func (l *listener) setupConnection( } errC := addOnConnectionStateChangeCallback(w.PeerConnection) - // Infer the client SDP from the incoming STUN message by setting the ice-ufrag. + // Infer the client SDP offer from the incoming STUN message using the client + // ufrag and password. pion validates the full "server_ufrag:client_ufrag" + // USERNAME on inbound checks, so the remote ice-ufrag must be the client ufrag. if err := w.PeerConnection.SetRemoteDescription(webrtc.SessionDescription{ - SDP: createClientSDP(candidate.Addr, candidate.Ufrag), + SDP: createClientSDP(candidate.Addr, clientUfrag, clientPwd), Type: webrtc.SDPTypeOffer, }); err != nil { return nil, err @@ -244,7 +339,7 @@ func (l *listener) setupConnection( return nil, ctx.Err() case err := <-errC: if err != nil { - return nil, fmt.Errorf("peer connection failed for ufrag: %s", candidate.Ufrag) + return nil, fmt.Errorf("peer connection failed for ufrag: %s", serverUfrag) } } diff --git a/p2p/transport/webrtc/sdp.go b/p2p/transport/webrtc/sdp.go index 878b668a18..76d76f2008 100644 --- a/p2p/transport/webrtc/sdp.go +++ b/p2p/transport/webrtc/sdp.go @@ -14,6 +14,10 @@ import ( // The fingerprint used to render a client SDP is arbitrary since // it fingerprint verification is disabled in favour of a noise // handshake. The max message size is fixed to 16384 bytes. +// +// The ice-ufrag and ice-pwd are rendered separately. In WebRTC Direct v1 they +// are the same shared value; in v2 the client ufrag and the recovered client +// password differ. See https://github.com/libp2p/specs/blob/master/webrtc/webrtc-direct.md. const clientSDP = `v=0 o=- 0 0 IN %[1]s %[2]s s=- @@ -24,14 +28,14 @@ m=application %[3]d UDP/DTLS/SCTP webrtc-datachannel a=mid:0 a=ice-options:ice2 a=ice-ufrag:%[4]s -a=ice-pwd:%[4]s +a=ice-pwd:%[5]s a=fingerprint:sha-256 ba:78:16:bf:8f:01:cf:ea:41:41:40:de:5d:ae:22:23:b0:03:61:a3:96:17:7a:9c:b4:10:ff:61:f2:00:15:ad a=setup:actpass a=sctp-port:5000 a=max-message-size:16384 ` -func createClientSDP(addr *net.UDPAddr, ufrag string) string { +func createClientSDP(addr *net.UDPAddr, ufrag, pwd string) string { ipVersion := "IP4" if addr.IP.To4() == nil { ipVersion = "IP6" @@ -42,6 +46,7 @@ func createClientSDP(addr *net.UDPAddr, ufrag string) string { addr.IP, addr.Port, ufrag, + pwd, ) } diff --git a/p2p/transport/webrtc/sdp_test.go b/p2p/transport/webrtc/sdp_test.go index 2e7ac5b3e8..030a6640e9 100644 --- a/p2p/transport/webrtc/sdp_test.go +++ b/p2p/transport/webrtc/sdp_test.go @@ -3,12 +3,43 @@ package libp2pwebrtc import ( "encoding/hex" "net" + "strings" "testing" "github.com/multiformats/go-multihash" "github.com/stretchr/testify/require" ) +// ICE username fragments and passwords are parsed from an attacker-controlled +// STUN USERNAME and templated into the inferred client SDP offer, so they must +// be validated against the ice-char set and length bounds in RFC 8839 section +// 5.4 before use. +func TestICECredentialValidation(t *testing.T) { + v2Ufrag := ufragPrefixV2 + strings.Repeat("a", 24) + require.True(t, isICEUfrag("abcd")) // 4 chars, minimum ufrag + require.True(t, isICEUfrag("libp2p+webrtc+v1/abcdEFGH")) // '+' and '/' are ice-char + require.True(t, isICEUfrag(v2Ufrag)) + require.True(t, isICEPwd(strings.Repeat("a", 22))) // 22 chars, minimum password + + // charset violations (e.g. CRLF/SDP injection attempts) + require.False(t, isICEUfrag("abc\r\na=candidate:x")) + require.False(t, isICEUfrag("ab:cd")) // ':' is not an ice-char + require.False(t, isICEPwd(strings.Repeat("a", 21)+"\n")) + + // length violations + require.False(t, isICEUfrag("abc")) // 3 < 4 + require.False(t, isICEUfrag("")) // empty + require.False(t, isICEPwd(strings.Repeat("a", 21))) // 21 < 22 + require.False(t, isICEUfrag(strings.Repeat("a", 257))) // > 256 + + // multi-byte UTF-8 input: the len() byte count differs from the character + // count (and from the UTF-16 length js-libp2p measures), but both reject it + // on the charset check, so the length-representation difference between the + // implementations never changes the decision. + require.False(t, isICEUfrag("abécd")) // 'é' is 2 UTF-8 bytes and not ice-char + require.False(t, isICEUfrag("ab😀cd")) // emoji: 4 UTF-8 bytes / 2 UTF-16 units +} + const expectedServerSDP = `v=0 o=- 0 0 IN IP4 0.0.0.0 s=- @@ -68,16 +99,28 @@ a=max-message-size:16384 func TestRenderClientSDP(t *testing.T) { addr := &net.UDPAddr{IP: net.IPv4(0, 0, 0, 0), Port: 37826} ufrag := "d2c0fc07-8bb3-42ae-bae2-a6fce8a0b581" - sdp := createClientSDP(addr, ufrag) + sdp := createClientSDP(addr, ufrag, ufrag) require.Equal(t, expectedClientSDP, sdp) } +// In WebRTC Direct v2 the inferred client offer carries distinct ice-ufrag and +// ice-pwd values: the client ufrag from the STUN USERNAME and the client +// password recovered from the "libp2p+webrtc+v2/" server ufrag. +func TestRenderClientSDPV2DistinctCredentials(t *testing.T) { + addr := &net.UDPAddr{IP: net.IPv4(0, 0, 0, 0), Port: 37826} + clientUfrag := "browserClientUfrag" + clientPwd := "browserClientPassword1234" + sdp := createClientSDP(addr, clientUfrag, clientPwd) + require.Contains(t, sdp, "a=ice-ufrag:"+clientUfrag+"\n") + require.Contains(t, sdp, "a=ice-pwd:"+clientPwd+"\n") +} + func BenchmarkRenderClientSDP(b *testing.B) { addr := &net.UDPAddr{IP: net.IPv4(0, 0, 0, 0), Port: 37826} ufrag := "d2c0fc07-8bb3-42ae-bae2-a6fce8a0b581" for i := 0; i < b.N; i++ { - createClientSDP(addr, ufrag) + createClientSDP(addr, ufrag, ufrag) } } diff --git a/p2p/transport/webrtc/transport.go b/p2p/transport/webrtc/transport.go index b498889493..acb34029fd 100644 --- a/p2p/transport/webrtc/transport.go +++ b/p2p/transport/webrtc/transport.go @@ -89,6 +89,12 @@ type WebRTCTransport struct { listenUDP func(network string, laddr *net.UDPAddr) (net.PacketConn, error) + // dialerVersion picks which WebRTC Direct handshake to use when dialing: 0 + // (unset) and 1 use v1 (SDP munging), 2 uses v2 (no munging; the client + // password rides in the server ufrag). The listener accepts both either way; + // it detects the version from the ICE username fragment prefix. See https://github.com/libp2p/specs/blob/master/webrtc/webrtc-direct.md. + dialerVersion int + // timeouts peerConnectionTimeouts iceTimeouts @@ -100,6 +106,24 @@ var _ tpt.Transport = &WebRTCTransport{} type Option func(*WebRTCTransport) error +// WithDialerVersion picks which WebRTC Direct handshake to use when dialing. It +// must be 1 (v1, SDP munging) or 2 (v2, no munging); any other value, including +// 0, is rejected. With the option unset, dialing defaults to v1. The listener +// accepts both either way. Browsers will need v2 once Chromium's +// WebRTC-NoSdpMangleUfrag field trial ships; this option lets a go dialer use the +// same v2 wire format. See https://github.com/libp2p/specs/blob/master/webrtc/webrtc-direct.md. +func WithDialerVersion(version int) Option { + return func(t *WebRTCTransport) error { + switch version { + case 1, 2: + t.dialerVersion = version + return nil + default: + return fmt.Errorf("unknown WebRTC Direct dialer version %d", version) + } + } +} + type iceTimeouts struct { Disconnect time.Duration Failed time.Duration @@ -301,17 +325,38 @@ func (t *WebRTCTransport) dial(ctx context.Context, scope network.ConnManagement return nil, fmt.Errorf("resolve udp address: %w", err) } - // Instead of encoding the local fingerprint we - // generate a random UUID as the connection ufrag. - // The only requirement here is that the ufrag and password - // must be equal, which will allow the server to determine - // the password using the STUN message. - ufrag := genUfrag() + // The server has no signaling channel, so it must infer our ICE credentials + // from the STUN connectivity checks. + // + // In v1 we generate a single random value, prefixed with "libp2p+webrtc+v1/", + // and use it as both our local ufrag and password and as the server's. The + // server reads it from the STUN USERNAME and derives the password from it. + // + // In v2 our local ufrag and password are independent random values (as a + // browser's would be) and we encode our password into the server ufrag as + // "libp2p+webrtc+v2/", so the server can recover it from the STUN + // USERNAME without us munging our local SDP. See https://github.com/libp2p/specs/blob/master/webrtc/webrtc-direct.md. + var localUfrag, localPwd, serverUfrag string + switch t.dialerVersion { + // TODO: flip the default to v2 roughly 12 months after Chromium ships its + // no-munging behavior (the WebRTC-NoSdpMangleUfrag field trial, + // https://webrtc-review.googlesource.com/c/src/+/385721) in stable, giving + // servers and other implementations a grace period to adopt v2 first. + case 0, 1: + localUfrag = genUfrag() + localPwd = localUfrag + serverUfrag = localUfrag + case 2: + localUfrag, localPwd = genV2ClientCredentials() + serverUfrag = ufragPrefixV2 + localPwd + default: + return nil, fmt.Errorf("unsupported WebRTC Direct dialer version %d", t.dialerVersion) + } settingEngine := webrtc.SettingEngine{ LoggerFactory: pionLoggerFactory, } - settingEngine.SetICECredentials(ufrag, ufrag) + settingEngine.SetICECredentials(localUfrag, localPwd) settingEngine.DetachDataChannels() // use the first best address candidate settingEngine.SetPrflxAcceptanceMinWait(0) @@ -349,7 +394,7 @@ func (t *WebRTCTransport) dial(ctx context.Context, scope network.ConnManagement return nil, fmt.Errorf("set local description: %w", err) } - answerSDPString, err := createServerSDP(raddr, ufrag, *remoteMultihash) + answerSDPString, err := createServerSDP(raddr, serverUfrag, *remoteMultihash) if err != nil { return nil, fmt.Errorf("render server SDP: %w", err) } @@ -421,22 +466,28 @@ func (t *WebRTCTransport) dial(ctx context.Context, scope network.ConnManagement } func genUfrag() string { - const ( - uFragAlphabet = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890" - uFragPrefix = "libp2p+webrtc+v1/" - uFragIdLength = 32 - uFragLength = len(uFragPrefix) + uFragIdLength - ) + return ufragPrefixV1 + randIceString(32) +} +// genV2ClientCredentials returns a random ICE ufrag and password for the WebRTC +// Direct v2 dial flow. Unlike v1, the two values are independent and carry no +// prefix; the password is instead encoded into the server ufrag as +// "libp2p+webrtc+v2/". Both are 32 ice-chars, comfortably above the RFC +// 8839 section 5.4 minimums (4 for the ufrag, 22 for the password). +func genV2ClientCredentials() (ufrag, pwd string) { + return randIceString(32), randIceString(32) +} + +// randIceString returns a string of n characters drawn from the ICE ufrag/pwd +// alphabet (the alphanumeric subset of RFC 8839 ice-char, also used by browsers). +func randIceString(n int) string { + const alphabet = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890" seed := [32]byte{} rand.Read(seed[:]) - r := mrand.New(mrand.New(mrand.NewChaCha8(seed))) - b := make([]byte, uFragLength) - for i := range len(uFragPrefix) { - b[i] = uFragPrefix[i] - } - for i := len(uFragPrefix); i < uFragLength; i++ { - b[i] = uFragAlphabet[r.IntN(len(uFragAlphabet))] + r := mrand.New(mrand.NewChaCha8(seed)) + b := make([]byte, n) + for i := range b { + b[i] = alphabet[r.IntN(len(alphabet))] } return string(b) } diff --git a/p2p/transport/webrtc/transport_test.go b/p2p/transport/webrtc/transport_test.go index a5e969fd1b..3e1d5f1f8a 100644 --- a/p2p/transport/webrtc/transport_test.go +++ b/p2p/transport/webrtc/transport_test.go @@ -221,6 +221,119 @@ func TestTransportWebRTC_CanListenSingle(t *testing.T) { } } +// TestTransportWebRTC_v2Dial runs the v2 (no SDP munging) dial flow end to end +// against the standard listener: a full ICE + DTLS + Noise handshake plus a +// two-way stream. The listener detects v2 from the ICE username fragment prefix +// with no extra configuration. +func TestTransportWebRTC_v2Dial(t *testing.T) { + tr, listeningPeer := getTransport(t) + tr1, connectingPeer := getTransport(t, WithDialerVersion(2)) + listenMultiaddr := ma.StringCast("/ip4/127.0.0.1/udp/0/webrtc-direct") + listener, err := tr.Listen(listenMultiaddr) + require.NoError(t, err) + defer listener.Close() + + streamChan := make(chan network.MuxedStream) + go func() { + conn, err := tr1.Dial(context.Background(), listener.Multiaddr(), listeningPeer) + assert.NoError(t, err) + t.Cleanup(func() { conn.Close() }) + stream, err := conn.AcceptStream() + assert.NoError(t, err) + streamChan <- stream + }() + + conn, err := listener.Accept() + require.NoError(t, err) + defer conn.Close() + require.Equal(t, connectingPeer, conn.RemotePeer()) + + stream, err := conn.OpenStream(context.Background()) + require.NoError(t, err) + _, err = stream.Write([]byte("test")) + require.NoError(t, err) + + var str network.MuxedStream + select { + case str = <-streamChan: + case <-time.After(5 * time.Second): + t.Fatal("stream opening timed out") + } + buf := make([]byte, 100) + str.SetReadDeadline(time.Now().Add(5 * time.Second)) + n, err := str.Read(buf) + require.NoError(t, err) + require.Equal(t, "test", string(buf[:n])) +} + +// TestTransportWebRTC_ListenerAcceptsBothVersions confirms a single listener +// accepts both v1 and v2 dialers concurrently, dispatching on the ICE username +// fragment prefix. +func TestTransportWebRTC_ListenerAcceptsBothVersions(t *testing.T) { + tr, listeningPeer := getTransport(t) + listenMultiaddr := ma.StringCast("/ip4/127.0.0.1/udp/0/webrtc-direct") + listener, err := tr.Listen(listenMultiaddr) + require.NoError(t, err) + defer listener.Close() + + for _, version := range []int{1, 2} { + t.Run(fmt.Sprintf("v%d", version), func(t *testing.T) { + client, connectingPeer := getTransport(t, WithDialerVersion(version)) + done := make(chan struct{}) + go func() { + conn, err := client.Dial(context.Background(), listener.Multiaddr(), listeningPeer) + assert.NoError(t, err) + if conn != nil { + t.Cleanup(func() { conn.Close() }) + } + close(done) + }() + + conn, err := listener.Accept() + require.NoError(t, err) + defer conn.Close() + require.Equal(t, connectingPeer, conn.RemotePeer()) + select { + case <-done: + case <-time.After(10 * time.Second): + t.Fatal("dial timed out") + } + }) + } +} + +// WithDialerVersion requires an explicit, known version: it accepts 1/2 and +// rejects 0 or unknown values rather than silently defaulting. +func TestWithDialerVersion(t *testing.T) { + privKey, _, err := crypto.GenerateKeyPair(crypto.Ed25519, -1) + require.NoError(t, err) + + for _, version := range []int{1, 2} { + _, err := New(privKey, nil, nil, &network.NullResourceManager{}, netListenUDP, WithDialerVersion(version)) + require.NoError(t, err, "version %d should be accepted", version) + } + + for _, version := range []int{0, 3} { + _, err := New(privKey, nil, nil, &network.NullResourceManager{}, netListenUDP, WithDialerVersion(version)) + require.Error(t, err, "version %d should be rejected", version) + } +} + +// An unknown dialer version is rejected at dial time, not treated as v1. +func TestTransportWebRTC_DialerRejectsUnknownVersion(t *testing.T) { + tr, listeningPeer := getTransport(t) + listener, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/webrtc-direct")) + require.NoError(t, err) + defer listener.Close() + + client, _ := getTransport(t) + // set directly to simulate a future/unknown version (WithDialerVersion would + // reject it earlier); the dial must error rather than assume v1 + client.dialerVersion = 3 + _, err = client.Dial(context.Background(), listener.Multiaddr(), listeningPeer) + require.ErrorContains(t, err, "unsupported WebRTC Direct dialer version") +} + // WithListenerMaxInFlightConnections sets the maximum number of connections that are in-flight, i.e // they are being negotiated, or are waiting to be accepted. func WithListenerMaxInFlightConnections(m uint32) Option { diff --git a/p2p/transport/webrtc/udpmux/mux.go b/p2p/transport/webrtc/udpmux/mux.go index 281cb881bc..0813634a31 100644 --- a/p2p/transport/webrtc/udpmux/mux.go +++ b/p2p/transport/webrtc/udpmux/mux.go @@ -32,8 +32,15 @@ const ReceiveBufSize = 1500 const maxAddrsPerUfrag = 32 type Candidate struct { + // Ufrag is the local (server) ICE ufrag, parsed from the part of the STUN + // USERNAME attribute before the ':'. pion retrieves a muxed connection by + // its local ufrag, so the mux is keyed on this value. Ufrag string - Addr *net.UDPAddr + // RemoteUfrag is the dialing peer's (client) ICE ufrag, parsed from the part + // of the STUN USERNAME attribute after the ':'. In WebRTC Direct v1 it is + // identical to Ufrag; in v2 the two differ. + RemoteUfrag string + Addr *net.UDPAddr } // UDPMux multiplexes multiple ICE connections over a single net.PacketConn, @@ -203,18 +210,18 @@ func (mux *UDPMux) processPacket(buf []byte, addr net.Addr) (processed bool) { return false } - ufrag, err := ufragFromSTUNMessage(msg) + serverUfrag, clientUfrag, err := ufragsFromSTUNMessage(msg) if err != nil { - log.Debug("could not find STUN username", "error", err) + log.Debug("could not parse STUN username", "error", err) return false } - connCreated, conn := mux.getOrCreateConn(ufrag, isIPv6, mux, udpAddr) + connCreated, conn := mux.getOrCreateConn(serverUfrag, isIPv6, mux, udpAddr) if connCreated { select { - case mux.queue <- Candidate{Addr: udpAddr, Ufrag: ufrag}: + case mux.queue <- Candidate{Addr: udpAddr, Ufrag: serverUfrag, RemoteUfrag: clientUfrag}: default: - log.Debug("queue full, dropping incoming candidate", "ufrag", ufrag, "addr", udpAddr) + log.Debug("queue full, dropping incoming candidate", "ufrag", serverUfrag, "addr", udpAddr) conn.Close() return false } @@ -243,25 +250,27 @@ type ufragConnKey struct { isIPv6 bool } -// ufragFromSTUNMessage returns the local or ufrag -// from the STUN username attribute. Local ufrag is the ufrag of the -// peer which initiated the connectivity check, e.g in a connectivity -// check from A to B, the username attribute will be B_ufrag:A_ufrag -// with the local ufrag value being A_ufrag. In case of ice-lite, the -// localUfrag value will always be the remote peer's ufrag since ICE-lite -// implementations do not generate connectivity checks. In our specific -// case, since the local and remote ufrag is equal, we can return -// either value. -func ufragFromSTUNMessage(msg *stun.Message) (string, error) { +// ufragsFromSTUNMessage returns the local (server) and remote (client) ufrag +// from the STUN USERNAME attribute. For a connectivity check from client A to +// server B the USERNAME is "B_ufrag:A_ufrag" (RFC 8445 §7.2.2, +// ":" from the sender's perspective). B's pion ICE +// agent retrieves a muxed connection by its local (server) ufrag, so we key the +// mux on B_ufrag (the part before the ':'). +// +// In WebRTC Direct v1 the two ufrags are identical. In v2 they differ: +// B_ufrag is "libp2p+webrtc+v2/" and A_ufrag is the client ufrag. +// An ufrag never contains a ':' (RFC 8445 ice-char), so splitting on the first +// ':' is unambiguous. +func ufragsFromSTUNMessage(msg *stun.Message) (serverUfrag, clientUfrag string, err error) { attr, err := msg.Get(stun.AttrUsername) if err != nil { - return "", err + return "", "", err } index := bytes.Index(attr, []byte{':'}) if index == -1 { - return "", fmt.Errorf("invalid STUN username attribute") + return "", "", fmt.Errorf("invalid STUN username attribute") } - return string(attr[index+1:]), nil + return string(attr[:index]), string(attr[index+1:]), nil } // RemoveConnByUfrag removes the connection associated with the ufrag and all the diff --git a/p2p/transport/webrtc/udpmux/mux_test.go b/p2p/transport/webrtc/udpmux/mux_test.go index 1fce6501e3..f132113062 100644 --- a/p2p/transport/webrtc/udpmux/mux_test.go +++ b/p2p/transport/webrtc/udpmux/mux_test.go @@ -31,6 +31,44 @@ func setupMapping(t *testing.T, ufrag string, from net.PacketConn, m *UDPMux) { require.NoError(t, err) } +func getSTUNBindingRequestWithUsername(username string) *stun.Message { + msg := stun.New() + msg.SetType(stun.BindingRequest) + uattr := stun.RawAttribute{ + Type: stun.AttrUsername, + Value: []byte(username), + } + uattr.AddTo(msg) + msg.Encode() + return msg +} + +// In WebRTC Direct v2 the STUN USERNAME carries distinct server and client +// ufrags ("server_ufrag:client_ufrag"). The mux must key on the server (local) +// ufrag, since that is the ufrag pion uses to retrieve the muxed connection, +// while still surfacing the client ufrag on the Candidate. +func TestAcceptV2DistinctUfrags(t *testing.T) { + c := newPacketConn(t) + defer c.Close() + m := NewUDPMux(c) + m.Start() + defer m.Close() + + serverUfrag := "libp2p+webrtc+v2/browserClientPassword1234" + clientUfrag := "browserClientUfrag" + + from := newPacketConn(t) + msg := getSTUNBindingRequestWithUsername(serverUfrag + ":" + clientUfrag) + _, err := from.WriteTo(msg.Raw, m.GetListenAddresses()[0]) + require.NoError(t, err) + + cand, err := m.Accept(context.Background()) + require.NoError(t, err) + require.Equal(t, serverUfrag, cand.Ufrag) + require.Equal(t, clientUfrag, cand.RemoteUfrag) + require.Equal(t, from.LocalAddr(), cand.Addr) +} + func newPacketConn(t *testing.T) net.PacketConn { t.Helper() udpPort0 := &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0} From 53e9c5aa20edd8a56ccf6ff0b1a09715209922ea Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 7 Jul 2026 02:22:22 +0200 Subject: [PATCH 2/3] test(webrtc): cover v1 and v2 dial flows The transport integration tests only exercised the default (v1) dial flow. Add an explicit v2 case so the no-munging path has coverage against the shared listener, which accepts both versions and selects one from the ufrag prefix. Both variants run the full battery (ping, streams, resets, deadlines, rcmgr); the WebRTC name gates move from exact match to substring so v1 and v2 are gated identically. --- p2p/test/transport/rcmgr_test.go | 2 +- p2p/test/transport/transport_test.go | 31 ++++++++++++++++++++++------ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/p2p/test/transport/rcmgr_test.go b/p2p/test/transport/rcmgr_test.go index cd24a8e876..cd0bb9b692 100644 --- a/p2p/test/transport/rcmgr_test.go +++ b/p2p/test/transport/rcmgr_test.go @@ -84,7 +84,7 @@ func TestResourceManagerIsUsed(t *testing.T) { } return nil }) - if tc.Name == "WebRTC" { + if strings.Contains(tc.Name, "WebRTC") { // webrtc receive buffer is a fix sized buffer allocated up front connScope.EXPECT().ReserveMemory(gomock.Any(), gomock.Any()) } diff --git a/p2p/test/transport/transport_test.go b/p2p/test/transport/transport_test.go index 4c26d987c4..eddcb01a61 100644 --- a/p2p/test/transport/transport_test.go +++ b/p2p/test/transport/transport_test.go @@ -339,10 +339,29 @@ var transportsToTest = []TransportTestCase{ }, }, { - Name: "WebRTC", + // WebRTC-v1 dials with the v1 flow (SDP munging). + Name: "WebRTC-v1", HostGenerator: func(t *testing.T, opts TransportTestCaseOpts) host.Host { libp2pOpts := transformOpts(opts) - libp2pOpts = append(libp2pOpts, libp2p.Transport(libp2pwebrtc.New)) + libp2pOpts = append(libp2pOpts, libp2p.Transport(libp2pwebrtc.New, libp2pwebrtc.WithDialerVersion(1))) + if opts.NoListen { + libp2pOpts = append(libp2pOpts, libp2p.NoListenAddrs) + } else { + libp2pOpts = append(libp2pOpts, libp2p.ListenAddrStrings("/ip4/127.0.0.1/udp/0/webrtc-direct")) + } + h, err := libp2p.New(libp2pOpts...) + require.NoError(t, err) + return h + }, + }, + { + // WebRTC-v2 dials with the v2 flow (no SDP munging). The listener accepts + // both versions and picks one from the ufrag prefix, so only the dialer + // needs the option; the same generator is fine for both hosts. + Name: "WebRTC-v2", + HostGenerator: func(t *testing.T, opts TransportTestCaseOpts) host.Host { + libp2pOpts := transformOpts(opts) + libp2pOpts = append(libp2pOpts, libp2p.Transport(libp2pwebrtc.New, libp2pwebrtc.WithDialerVersion(2))) if opts.NoListen { libp2pOpts = append(libp2pOpts, libp2p.NoListenAddrs) } else { @@ -893,7 +912,7 @@ func TestDiscoverPeerIDFromSecurityNegotiation(t *testing.T) { func TestCloseConnWhenBlocked(t *testing.T) { for _, tc := range transportsToTest { // WebRTC doesn't have a connection when rcmgr blocks it, so there's nothing to close. - if tc.Name == "WebRTC" { + if strings.Contains(tc.Name, "WebRTC") { continue } t.Run(tc.Name, func(t *testing.T) { @@ -933,7 +952,7 @@ func TestCloseConnWhenBlocked(t *testing.T) { // connection attempt func TestConnDroppedWhenBlocked(t *testing.T) { for _, tc := range transportsToTest { - if tc.Name != "WebRTC" { + if !strings.Contains(tc.Name, "WebRTC") { continue } t.Run(tc.Name, func(t *testing.T) { @@ -1096,7 +1115,7 @@ func TestErrorCodes(t *testing.T) { }) t.Run("StreamResetByConnCloseWithError", func(t *testing.T) { - if tc.Name == "WebRTC" { + if strings.Contains(tc.Name, "WebRTC") { t.Skipf("skipping: %s, not implemented", tc.Name) return } @@ -1124,7 +1143,7 @@ func TestErrorCodes(t *testing.T) { }) t.Run("NewStreamErrorByConnCloseWithError", func(t *testing.T) { - if tc.Name == "WebRTC" { + if strings.Contains(tc.Name, "WebRTC") { t.Skipf("skipping: %s, not implemented", tc.Name) return } From 48464706de785728a9f9281e993961326d13b897 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 7 Jul 2026 20:28:15 +0200 Subject: [PATCH 3/3] refactor(webrtc): validate ufrags in udpmux The udpmux now owns parsing and validation of the STUN USERNAME, so a malformed or unsupported request is dropped before the mux allocates a connection or queues a candidate for the listener. - udpmux: credentialsFromSTUNMessage validates both ICE ufrags, selects the WebRTC Direct version, and recovers the client password before getOrCreateConn. The ice-char validators and the v1/v2 ufrag prefixes live in ufrag.go, so the listener no longer re-validates. - udpmux: rename Candidate.Ufrag to LocalUfrag, add RemotePwd, and name the local ufrag consistently across the maps and helpers. - udpmux: drop the empty-ufrag guard in RemoveConnByUfrag, now unreachable since empty ufrags fail validation upstream. - transport: New defaults dialerVersion to 1 so dial never handles the confusing 0 value; the dialer reads udpmux.UfragPrefixV1/V2. --- p2p/transport/webrtc/listener.go | 103 ++------------- p2p/transport/webrtc/sdp_test.go | 31 ----- p2p/transport/webrtc/transport.go | 20 ++- p2p/transport/webrtc/udpmux/mux.go | 121 +++++++++++------- p2p/transport/webrtc/udpmux/mux_test.go | 64 +++++++-- .../webrtc/udpmux/muxed_connection.go | 28 ++-- p2p/transport/webrtc/udpmux/ufrag.go | 62 +++++++++ p2p/transport/webrtc/udpmux/ufrag_test.go | 94 ++++++++++++++ 8 files changed, 320 insertions(+), 203 deletions(-) create mode 100644 p2p/transport/webrtc/udpmux/ufrag.go create mode 100644 p2p/transport/webrtc/udpmux/ufrag_test.go diff --git a/p2p/transport/webrtc/listener.go b/p2p/transport/webrtc/listener.go index ee01e6d140..0d3701bb83 100644 --- a/p2p/transport/webrtc/listener.go +++ b/p2p/transport/webrtc/listener.go @@ -32,67 +32,6 @@ var _ network.ConnMultiaddrs = &connMultiaddrs{} func (c *connMultiaddrs) LocalMultiaddr() ma.Multiaddr { return c.local } func (c *connMultiaddrs) RemoteMultiaddr() ma.Multiaddr { return c.remote } -// The "libp2p+webrtc+" namespace selects the WebRTC Direct handshake version via -// the ICE username fragment. v1 ("libp2p+webrtc+v1/") munges the SDP; v2 -// ("libp2p+webrtc+v2/") does not, because Chromium's WebRTC-NoSdpMangleUfrag -// field trial rejects an offer whose ICE ufrag was munged. A v2 client instead -// leaves its local credentials untouched and puts its own ICE password in the -// server ufrag as "libp2p+webrtc+v2/", which the server reads back -// from the STUN USERNAME with no SDP exchange. A server MUST reject a version it -// does not recognize rather than guess one. v2 was introduced in -// https://github.com/libp2p/specs/pull/715; the spec lives at -// https://github.com/libp2p/specs/blob/master/webrtc/webrtc-direct.md and the -// js-libp2p implementation at https://github.com/libp2p/js-libp2p/pull/3480. -const ( - ufragPrefixV1 = "libp2p+webrtc+v1/" - ufragPrefixV2 = "libp2p+webrtc+v2/" -) - -const ( - // ICE credential bounds, per RFC 8839 section 5.4 - // (ufrag = 4*256ice-char, password = 22*256ice-char). - iceUfragMinLen = 4 - icePwdMinLen = 22 - iceCredentialMax = 256 -) - -// ice-char is ASCII-only, so any string that passes isICECharString has exactly -// one byte per character. The len() byte count used by isICEUfrag/isICEPwd then -// equals both the ice-char count and the UTF-16 length that js-libp2p measures, -// so the two implementations accept the same credentials; non-ice-char input -// (including any multi-byte UTF-8) is rejected by both regardless of how each -// counts length. - -// isICEChar reports whether b is in the RFC 8839 ice-char set -// (ALPHA / DIGIT / "+" / "/"). -func isICEChar(b byte) bool { - return (b >= 'a' && b <= 'z') || - (b >= 'A' && b <= 'Z') || - (b >= '0' && b <= '9') || - b == '+' || b == '/' -} - -func isICECharString(s string) bool { - for i := 0; i < len(s); i++ { - if !isICEChar(s[i]) { - return false - } - } - return true -} - -// isICEUfrag reports whether s is a syntactically valid ICE username fragment -// (RFC 8839 section 5.4: ufrag = 4*256ice-char). -func isICEUfrag(s string) bool { - return len(s) >= iceUfragMinLen && len(s) <= iceCredentialMax && isICECharString(s) -} - -// isICEPwd reports whether s is a syntactically valid ICE password -// (RFC 8839 section 5.4: password = 22*256ice-char). -func isICEPwd(s string) bool { - return len(s) >= icePwdMinLen && len(s) <= iceCredentialMax && isICECharString(s) -} - const ( candidateSetupTimeout = 10 * time.Second // This is higher than other transports(64) as there's no way to detect a peer that has gone away after @@ -196,8 +135,8 @@ func (l *listener) listen() { conn, err := l.handleCandidate(ctx, candidate) if err != nil { - l.mux.RemoveConnByUfrag(candidate.Ufrag) - log.Debug("could not accept connection", "ufrag", candidate.Ufrag, "error", err) + l.mux.RemoveConnByUfrag(candidate.LocalUfrag) + log.Debug("could not accept connection", "ufrag", candidate.LocalUfrag, "error", err) return } @@ -257,37 +196,11 @@ func (l *listener) setupConnection( } }() - // candidate.Ufrag is the server (local) ufrag and candidate.RemoteUfrag is the - // client ufrag, parsed from the STUN USERNAME ("server_ufrag:client_ufrag", - // RFC 8445 section 7.2.2). Both come from an attacker-controlled STUN packet - // and are templated into the inferred SDP offer and pion's ICE credentials, so - // reject anything that is not a valid ICE username fragment (ice-char, length - // 4..256) per RFC 8839 section 5.4 before using it. - serverUfrag := candidate.Ufrag - clientUfrag := candidate.RemoteUfrag - if !isICEUfrag(serverUfrag) || !isICEUfrag(clientUfrag) { - return nil, fmt.Errorf("invalid ICE ufrag in STUN username") - } - // Select the version explicitly from the server ufrag prefix. The client - // password lets us render a valid remote (client) SDP offer: in v1 the client - // uses one shared value (client password == client ufrag); in v2 the client - // encodes its password into the server ufrag as "libp2p+webrtc+v2/". - // An unrecognized version is rejected, never treated as v1. See https://github.com/libp2p/specs/blob/master/webrtc/webrtc-direct.md. - var clientPwd string - switch { - case strings.HasPrefix(serverUfrag, ufragPrefixV2): - pwd := strings.TrimPrefix(serverUfrag, ufragPrefixV2) - // The recovered value becomes the inferred offer's ice-pwd, so it must be a - // valid ICE password (ice-char, length 22..256) per RFC 8839 section 5.4. - if !isICEPwd(pwd) { - return nil, fmt.Errorf("invalid v2 ufrag %q: recovered client password is not a valid ICE password", serverUfrag) - } - clientPwd = pwd - case strings.HasPrefix(serverUfrag, ufragPrefixV1): - clientPwd = clientUfrag - default: - return nil, fmt.Errorf("unsupported WebRTC Direct version in ufrag %q", serverUfrag) - } + // The udpmux has already parsed and validated the STUN USERNAME: LocalUfrag is + // the server (local) ufrag, RemoteUfrag the client ufrag, and RemotePwd the + // client ICE password (recovered per WebRTC Direct version). See + // udpmux.credentialsFromSTUNMessage. + serverUfrag := candidate.LocalUfrag settingEngine := webrtc.SettingEngine{LoggerFactory: pionLoggerFactory} settingEngine.SetAnsweringDTLSRole(webrtc.DTLSRoleServer) @@ -321,7 +234,7 @@ func (l *listener) setupConnection( // ufrag and password. pion validates the full "server_ufrag:client_ufrag" // USERNAME on inbound checks, so the remote ice-ufrag must be the client ufrag. if err := w.PeerConnection.SetRemoteDescription(webrtc.SessionDescription{ - SDP: createClientSDP(candidate.Addr, clientUfrag, clientPwd), + SDP: createClientSDP(candidate.Addr, candidate.RemoteUfrag, candidate.RemotePwd), Type: webrtc.SDPTypeOffer, }); err != nil { return nil, err diff --git a/p2p/transport/webrtc/sdp_test.go b/p2p/transport/webrtc/sdp_test.go index 030a6640e9..e474c0094b 100644 --- a/p2p/transport/webrtc/sdp_test.go +++ b/p2p/transport/webrtc/sdp_test.go @@ -3,43 +3,12 @@ package libp2pwebrtc import ( "encoding/hex" "net" - "strings" "testing" "github.com/multiformats/go-multihash" "github.com/stretchr/testify/require" ) -// ICE username fragments and passwords are parsed from an attacker-controlled -// STUN USERNAME and templated into the inferred client SDP offer, so they must -// be validated against the ice-char set and length bounds in RFC 8839 section -// 5.4 before use. -func TestICECredentialValidation(t *testing.T) { - v2Ufrag := ufragPrefixV2 + strings.Repeat("a", 24) - require.True(t, isICEUfrag("abcd")) // 4 chars, minimum ufrag - require.True(t, isICEUfrag("libp2p+webrtc+v1/abcdEFGH")) // '+' and '/' are ice-char - require.True(t, isICEUfrag(v2Ufrag)) - require.True(t, isICEPwd(strings.Repeat("a", 22))) // 22 chars, minimum password - - // charset violations (e.g. CRLF/SDP injection attempts) - require.False(t, isICEUfrag("abc\r\na=candidate:x")) - require.False(t, isICEUfrag("ab:cd")) // ':' is not an ice-char - require.False(t, isICEPwd(strings.Repeat("a", 21)+"\n")) - - // length violations - require.False(t, isICEUfrag("abc")) // 3 < 4 - require.False(t, isICEUfrag("")) // empty - require.False(t, isICEPwd(strings.Repeat("a", 21))) // 21 < 22 - require.False(t, isICEUfrag(strings.Repeat("a", 257))) // > 256 - - // multi-byte UTF-8 input: the len() byte count differs from the character - // count (and from the UTF-16 length js-libp2p measures), but both reject it - // on the charset check, so the length-representation difference between the - // implementations never changes the decision. - require.False(t, isICEUfrag("abécd")) // 'é' is 2 UTF-8 bytes and not ice-char - require.False(t, isICEUfrag("ab😀cd")) // emoji: 4 UTF-8 bytes / 2 UTF-16 units -} - const expectedServerSDP = `v=0 o=- 0 0 IN IP4 0.0.0.0 s=- diff --git a/p2p/transport/webrtc/transport.go b/p2p/transport/webrtc/transport.go index acb34029fd..8d378064fc 100644 --- a/p2p/transport/webrtc/transport.go +++ b/p2p/transport/webrtc/transport.go @@ -29,6 +29,7 @@ import ( "github.com/libp2p/go-libp2p/p2p/security/noise" libp2pquic "github.com/libp2p/go-libp2p/p2p/transport/quic" "github.com/libp2p/go-libp2p/p2p/transport/webrtc/pb" + "github.com/libp2p/go-libp2p/p2p/transport/webrtc/udpmux" "github.com/libp2p/go-msgio" ma "github.com/multiformats/go-multiaddr" @@ -89,10 +90,12 @@ type WebRTCTransport struct { listenUDP func(network string, laddr *net.UDPAddr) (net.PacketConn, error) - // dialerVersion picks which WebRTC Direct handshake to use when dialing: 0 - // (unset) and 1 use v1 (SDP munging), 2 uses v2 (no munging; the client - // password rides in the server ufrag). The listener accepts both either way; - // it detects the version from the ICE username fragment prefix. See https://github.com/libp2p/specs/blob/master/webrtc/webrtc-direct.md. + // dialerVersion picks which WebRTC Direct handshake to use when dialing: 1 + // uses v1 (SDP munging), 2 uses v2 (no munging; the client password rides in + // the server ufrag). New defaults it to 1, and WithDialerVersion only accepts + // 1 or 2, so dial never sees the confusing 0 value. The listener accepts both + // either way; it detects the version from the ICE username fragment prefix. + // See https://github.com/libp2p/specs/blob/master/webrtc/webrtc-direct.md. dialerVersion int // timeouts @@ -186,6 +189,9 @@ func New(privKey ic.PrivKey, psk pnet.PSK, gater connmgr.ConnectionGater, rcmgr }, maxInFlightConnections: DefaultMaxInFlightConnections, + + // Default to v1; WithDialerVersion can override it before dial time. + dialerVersion: 1, } for _, opt := range opts { if err := opt(transport); err != nil { @@ -342,13 +348,13 @@ func (t *WebRTCTransport) dial(ctx context.Context, scope network.ConnManagement // no-munging behavior (the WebRTC-NoSdpMangleUfrag field trial, // https://webrtc-review.googlesource.com/c/src/+/385721) in stable, giving // servers and other implementations a grace period to adopt v2 first. - case 0, 1: + case 1: localUfrag = genUfrag() localPwd = localUfrag serverUfrag = localUfrag case 2: localUfrag, localPwd = genV2ClientCredentials() - serverUfrag = ufragPrefixV2 + localPwd + serverUfrag = udpmux.UfragPrefixV2 + localPwd default: return nil, fmt.Errorf("unsupported WebRTC Direct dialer version %d", t.dialerVersion) } @@ -466,7 +472,7 @@ func (t *WebRTCTransport) dial(ctx context.Context, scope network.ConnManagement } func genUfrag() string { - return ufragPrefixV1 + randIceString(32) + return udpmux.UfragPrefixV1 + randIceString(32) } // genV2ClientCredentials returns a random ICE ufrag and password for the WebRTC diff --git a/p2p/transport/webrtc/udpmux/mux.go b/p2p/transport/webrtc/udpmux/mux.go index 0813634a31..b167fafce8 100644 --- a/p2p/transport/webrtc/udpmux/mux.go +++ b/p2p/transport/webrtc/udpmux/mux.go @@ -3,7 +3,6 @@ package udpmux import ( - "bytes" "context" "errors" "fmt" @@ -32,15 +31,19 @@ const ReceiveBufSize = 1500 const maxAddrsPerUfrag = 32 type Candidate struct { - // Ufrag is the local (server) ICE ufrag, parsed from the part of the STUN - // USERNAME attribute before the ':'. pion retrieves a muxed connection by - // its local ufrag, so the mux is keyed on this value. - Ufrag string + // LocalUfrag is the local (server) ICE ufrag, parsed from the part of the + // STUN USERNAME attribute before the ':'. pion retrieves a muxed connection + // by its local ufrag, so the mux is keyed on this value. + LocalUfrag string // RemoteUfrag is the dialing peer's (client) ICE ufrag, parsed from the part // of the STUN USERNAME attribute after the ':'. In WebRTC Direct v1 it is - // identical to Ufrag; in v2 the two differ. + // identical to LocalUfrag; in v2 the two differ. RemoteUfrag string - Addr *net.UDPAddr + // RemotePwd is the dialing peer's (client) ICE password. The listener needs + // it to infer the client's SDP offer. In v1 it equals RemoteUfrag; in v2 it + // is recovered from the server ufrag ("libp2p+webrtc+v2/"). + RemotePwd string + Addr *net.UDPAddr } // UDPMux multiplexes multiple ICE connections over a single net.PacketConn, @@ -63,13 +66,14 @@ type UDPMux struct { queue chan Candidate mx sync.Mutex - // ufragMap allows us to multiplex incoming STUN packets based on ufrag + // ufragMap allows us to multiplex incoming STUN packets based on the local + // (server) ufrag, which is the ufrag pion uses to retrieve a muxed connection. ufragMap map[ufragConnKey]*muxedConnection // addrMap allows us to correctly direct incoming packets after the connection // is established and ufrag isn't available on all packets addrMap map[string]*muxedConnection // ufragAddrMap allows cleaning up all addresses from the addrMap once the connection is closed - // During the ICE connectivity checks, the same ufrag might be used on multiple addresses. + // During the ICE connectivity checks, the same local ufrag might be used on multiple addresses. ufragAddrMap map[ufragConnKey][]net.Addr // the context controls the lifecycle of the mux @@ -112,7 +116,7 @@ func (mux *UDPMux) GetListenAddresses() []net.Addr { // It creates a net.PacketConn for a given ufrag if an existing one cannot be found. // We differentiate IPv4 and IPv6 addresses, since a remote is can be reachable at multiple different // UDP addresses of the same IP address family (eg. server-reflexive addresses and peer-reflexive addresses). -func (mux *UDPMux) GetConn(ufrag string, addr net.Addr) (net.PacketConn, error) { +func (mux *UDPMux) GetConn(localUfrag string, addr net.Addr) (net.PacketConn, error) { a, ok := addr.(*net.UDPAddr) if !ok { return nil, fmt.Errorf("unexpected address type: %T", addr) @@ -122,7 +126,7 @@ func (mux *UDPMux) GetConn(ufrag string, addr net.Addr) (net.PacketConn, error) return nil, io.ErrClosedPipe default: isIPv6 := ok && a.IP.To4() == nil - _, conn := mux.getOrCreateConn(ufrag, isIPv6, mux, addr) + _, conn := mux.getOrCreateConn(localUfrag, isIPv6, mux, addr) return conn, nil } } @@ -210,18 +214,18 @@ func (mux *UDPMux) processPacket(buf []byte, addr net.Addr) (processed bool) { return false } - serverUfrag, clientUfrag, err := ufragsFromSTUNMessage(msg) + localUfrag, remoteUfrag, remotePwd, err := credentialsFromSTUNMessage(msg) if err != nil { - log.Debug("could not parse STUN username", "error", err) + log.Debug("dropping STUN binding request with invalid credentials", "error", err) return false } - connCreated, conn := mux.getOrCreateConn(serverUfrag, isIPv6, mux, udpAddr) + connCreated, conn := mux.getOrCreateConn(localUfrag, isIPv6, mux, udpAddr) if connCreated { select { - case mux.queue <- Candidate{Addr: udpAddr, Ufrag: serverUfrag, RemoteUfrag: clientUfrag}: + case mux.queue <- Candidate{Addr: udpAddr, LocalUfrag: localUfrag, RemoteUfrag: remoteUfrag, RemotePwd: remotePwd}: default: - log.Debug("queue full, dropping incoming candidate", "ufrag", serverUfrag, "addr", udpAddr) + log.Debug("queue full, dropping incoming candidate", "ufrag", localUfrag, "addr", udpAddr) conn.Close() return false } @@ -246,46 +250,73 @@ func (mux *UDPMux) Accept(ctx context.Context) (Candidate, error) { } type ufragConnKey struct { - ufrag string - isIPv6 bool + // localUfrag is the local (server) ICE ufrag. pion retrieves a muxed + // connection by its local ufrag, so the mux keys on it. + localUfrag string + isIPv6 bool } -// ufragsFromSTUNMessage returns the local (server) and remote (client) ufrag -// from the STUN USERNAME attribute. For a connectivity check from client A to -// server B the USERNAME is "B_ufrag:A_ufrag" (RFC 8445 §7.2.2, +// credentialsFromSTUNMessage parses and validates the ICE credentials carried in +// a STUN binding request's USERNAME attribute. For a connectivity check from +// client A to server B the USERNAME is "B_ufrag:A_ufrag" (RFC 8445 section 7.2.2, // ":" from the sender's perspective). B's pion ICE -// agent retrieves a muxed connection by its local (server) ufrag, so we key the -// mux on B_ufrag (the part before the ':'). +// agent retrieves a muxed connection by its local (server) ufrag, so the mux +// keys on B_ufrag (the part before the ':'). An ufrag never contains a ':' (it +// is not an ice-char), so splitting on the first ':' is unambiguous. // -// In WebRTC Direct v1 the two ufrags are identical. In v2 they differ: -// B_ufrag is "libp2p+webrtc+v2/" and A_ufrag is the client ufrag. -// An ufrag never contains a ':' (RFC 8445 ice-char), so splitting on the first -// ':' is unambiguous. -func ufragsFromSTUNMessage(msg *stun.Message) (serverUfrag, clientUfrag string, err error) { +// Both ufrags come from an attacker-controlled STUN packet and are later +// templated into the inferred SDP offer and pion's ICE credentials. They are +// validated here, before the mux allocates any connection state or queues a +// candidate, so a malformed or unsupported request never reaches the listener: +// +// - both ufrags must be valid ICE username fragments (ice-char, length 4..256, +// RFC 8839 section 5.4); +// - the WebRTC Direct version is selected from the server ufrag prefix. It also +// determines how the client's ICE password (needed to render the client's SDP +// offer) is recovered: v1 shares one value (client password == client ufrag), +// v2 encodes the password into the server ufrag as "libp2p+webrtc+v2/". +// An unrecognized version is rejected, never treated as v1. +// +// See https://github.com/libp2p/specs/blob/master/webrtc/webrtc-direct.md. +func credentialsFromSTUNMessage(msg *stun.Message) (localUfrag, remoteUfrag, remotePwd string, err error) { attr, err := msg.Get(stun.AttrUsername) if err != nil { - return "", "", err + return "", "", "", err } - index := bytes.Index(attr, []byte{':'}) - if index == -1 { - return "", "", fmt.Errorf("invalid STUN username attribute") + local, remote, found := strings.Cut(string(attr), ":") + if !found { + return "", "", "", fmt.Errorf("invalid STUN username attribute") } - return string(attr[:index]), string(attr[index+1:]), nil -} - -// RemoveConnByUfrag removes the connection associated with the ufrag and all the -// addresses associated with that connection. This method is called by pion when -// a peerconnection is closed. -func (mux *UDPMux) RemoveConnByUfrag(ufrag string) { - if ufrag == "" { - return + if !isICEUfrag(local) || !isICEUfrag(remote) { + return "", "", "", fmt.Errorf("invalid ICE ufrag in STUN username") } + switch { + case strings.HasPrefix(local, UfragPrefixV2): + pwd := strings.TrimPrefix(local, UfragPrefixV2) + // The recovered value becomes the inferred offer's ice-pwd, so it must be + // a valid ICE password (ice-char, length 22..256) per RFC 8839 section 5.4. + if !isICEPwd(pwd) { + return "", "", "", fmt.Errorf("invalid v2 ufrag %q: recovered client password is not a valid ICE password", local) + } + return local, remote, pwd, nil + case strings.HasPrefix(local, UfragPrefixV1): + return local, remote, remote, nil + default: + return "", "", "", fmt.Errorf("unsupported WebRTC Direct version in ufrag %q", local) + } +} +// RemoveConnByUfrag removes the connection associated with the local (server) +// ufrag and all the addresses associated with that connection. This method is +// called by pion when a peerconnection is closed, always with the connection's +// local ufrag, which credentialsFromSTUNMessage has already validated as +// non-empty. +func (mux *UDPMux) RemoveConnByUfrag(localUfrag string) { mux.mx.Lock() defer mux.mx.Unlock() for _, isIPv6 := range [...]bool{true, false} { - key := ufragConnKey{ufrag: ufrag, isIPv6: isIPv6} + key := ufragConnKey{localUfrag: localUfrag, isIPv6: isIPv6} if conn, ok := mux.ufragMap[key]; ok { delete(mux.ufragMap, key) for _, addr := range mux.ufragAddrMap[key] { @@ -297,8 +328,8 @@ func (mux *UDPMux) RemoveConnByUfrag(ufrag string) { } } -func (mux *UDPMux) getOrCreateConn(ufrag string, isIPv6 bool, _ *UDPMux, addr net.Addr) (created bool, _ *muxedConnection) { - key := ufragConnKey{ufrag: ufrag, isIPv6: isIPv6} +func (mux *UDPMux) getOrCreateConn(localUfrag string, isIPv6 bool, _ *UDPMux, addr net.Addr) (created bool, _ *muxedConnection) { + key := ufragConnKey{localUfrag: localUfrag, isIPv6: isIPv6} mux.mx.Lock() defer mux.mx.Unlock() @@ -315,7 +346,7 @@ func (mux *UDPMux) getOrCreateConn(ufrag string, isIPv6 bool, _ *UDPMux, addr ne return false, conn } - conn := newMuxedConnection(mux, ufrag) + conn := newMuxedConnection(mux, localUfrag) mux.ufragMap[key] = conn mux.addrMap[addr.String()] = conn mux.ufragAddrMap[key] = append(mux.ufragAddrMap[key], addr) diff --git a/p2p/transport/webrtc/udpmux/mux_test.go b/p2p/transport/webrtc/udpmux/mux_test.go index f132113062..e7ec008900 100644 --- a/p2p/transport/webrtc/udpmux/mux_test.go +++ b/p2p/transport/webrtc/udpmux/mux_test.go @@ -43,6 +43,14 @@ func getSTUNBindingRequestWithUsername(username string) *stun.Message { return msg } +// v1Ufrag builds a valid WebRTC Direct v1 ICE ufrag from a short seed. Tests +// that drive the mux through the real inbound STUN path (via setupMapping) need +// credentials that credentialsFromSTUNMessage accepts; bare seeds like "a" have +// no version prefix and would be dropped. +func v1Ufrag(seed string) string { + return UfragPrefixV1 + seed +} + // In WebRTC Direct v2 the STUN USERNAME carries distinct server and client // ufrags ("server_ufrag:client_ufrag"). The mux must key on the server (local) // ufrag, since that is the ufrag pion uses to retrieve the muxed connection, @@ -54,7 +62,8 @@ func TestAcceptV2DistinctUfrags(t *testing.T) { m.Start() defer m.Close() - serverUfrag := "libp2p+webrtc+v2/browserClientPassword1234" + clientPwd := "browserClientPassword1234" + serverUfrag := UfragPrefixV2 + clientPwd clientUfrag := "browserClientUfrag" from := newPacketConn(t) @@ -64,11 +73,44 @@ func TestAcceptV2DistinctUfrags(t *testing.T) { cand, err := m.Accept(context.Background()) require.NoError(t, err) - require.Equal(t, serverUfrag, cand.Ufrag) + require.Equal(t, serverUfrag, cand.LocalUfrag) require.Equal(t, clientUfrag, cand.RemoteUfrag) + // v2 recovers the client's ICE password from the server ufrag. + require.Equal(t, clientPwd, cand.RemotePwd) require.Equal(t, from.LocalAddr(), cand.Addr) } +// A STUN binding request whose USERNAME fails validation must be dropped before +// the mux allocates a connection or queues a candidate, so nothing reaches the +// listener via Accept. +func TestAcceptRejectsInvalidCredentials(t *testing.T) { + for _, username := range []string{ + "nocolon", // no ':' separator + ":" + v1Ufrag("a"), // empty server ufrag + "ab:" + v1Ufrag("a"), // server ufrag too short + "noprefix:noprefix", // valid ice-chars but unknown version + UfragPrefixV2 + "short:" + v1Ufrag("a"), // v2 recovered password too short + } { + t.Run(username, func(t *testing.T) { + c := newPacketConn(t) + defer c.Close() + m := NewUDPMux(c) + m.Start() + defer m.Close() + + from := newPacketConn(t) + msg := getSTUNBindingRequestWithUsername(username) + _, err := from.WriteTo(msg.Raw, m.GetListenAddresses()[0]) + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + _, err = m.Accept(ctx) + require.ErrorIs(t, err, context.DeadlineExceeded) + }) + } +} + func newPacketConn(t *testing.T) net.PacketConn { t.Helper() udpPort0 := &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0} @@ -85,7 +127,7 @@ func TestAccept(t *testing.T) { m.Start() defer m.Close() - ufrags := []string{"a", "b", "c", "d"} + ufrags := []string{v1Ufrag("a"), v1Ufrag("b"), v1Ufrag("c"), v1Ufrag("d")} conns := make([]net.PacketConn, len(ufrags)) for i, ufrag := range ufrags { conns[i] = newPacketConn(t) @@ -94,7 +136,7 @@ func TestAccept(t *testing.T) { for i, ufrag := range ufrags { c, err := m.Accept(context.Background()) require.NoError(t, err) - require.Equal(t, c.Ufrag, ufrag) + require.Equal(t, c.LocalUfrag, ufrag) require.Equal(t, c.Addr, conns[i].LocalAddr()) } @@ -122,7 +164,7 @@ func TestGetConn(t *testing.T) { m.Start() defer m.Close() - ufrags := []string{"a", "b", "c", "d"} + ufrags := []string{v1Ufrag("a"), v1Ufrag("b"), v1Ufrag("c"), v1Ufrag("d")} conns := make([]net.PacketConn, len(ufrags)) for i, ufrag := range ufrags { conns[i] = newPacketConn(t) @@ -131,7 +173,7 @@ func TestGetConn(t *testing.T) { for i, ufrag := range ufrags { c, err := m.Accept(context.Background()) require.NoError(t, err) - require.Equal(t, c.Ufrag, ufrag) + require.Equal(t, c.LocalUfrag, ufrag) require.Equal(t, c.Addr, conns[i].LocalAddr()) } @@ -178,7 +220,7 @@ func TestRemoveConnByUfrag(t *testing.T) { defer m.Close() // Map each ufrag to two addresses - ufrag := "a" + ufrag := v1Ufrag("a") count := 10 conns := make([]net.PacketConn, count) for i := range 10 { @@ -199,7 +241,7 @@ func TestRemoveConnByUfrag(t *testing.T) { m.RemoveConnByUfrag(ufrag) // All connections should now be associated with b - ufrag = "b" + ufrag = v1Ufrag("b") for i := range 10 { setupMapping(t, ufrag, conns[i], m) } @@ -214,7 +256,7 @@ func TestRemoveConnByUfrag(t *testing.T) { } // Should be different even if the address is the same - mc1, err := m.GetConn("a", conns[0].LocalAddr()) + mc1, err := m.GetConn(v1Ufrag("a"), conns[0].LocalAddr()) require.NoError(t, err) if mc1 == mc { t.Fatalf("expected the two connections to be different") @@ -230,7 +272,7 @@ func TestMuxedConnection(t *testing.T) { msgCount := 3 connCount := 3 - ufrags := []string{"a", "b", "c"} + ufrags := []string{v1Ufrag("a"), v1Ufrag("b"), v1Ufrag("c")} addrUfragMap := make(map[string]string) ufragConnsMap := make(map[string][]net.PacketConn) for _, ufrag := range ufrags { @@ -306,7 +348,7 @@ func TestAddrsPerUfragCap(t *testing.T) { require.NoError(t, err) } - key := ufragConnKey{ufrag: ufrag, isIPv6: false} + key := ufragConnKey{localUfrag: ufrag, isIPv6: false} m.mx.Lock() require.Len(t, m.ufragAddrMap[key], maxAddrsPerUfrag) require.Len(t, m.addrMap, maxAddrsPerUfrag) diff --git a/p2p/transport/webrtc/udpmux/muxed_connection.go b/p2p/transport/webrtc/udpmux/muxed_connection.go index 4d560b68b3..62fb87b130 100644 --- a/p2p/transport/webrtc/udpmux/muxed_connection.go +++ b/p2p/transport/webrtc/udpmux/muxed_connection.go @@ -20,26 +20,26 @@ const queueLen = 128 // muxedConnection provides a net.PacketConn abstraction // over packetQueue and adds the ability to store addresses -// from which this connection (indexed by ufrag) received -// data. +// from which this connection (indexed by its local ufrag) +// received data. type muxedConnection struct { - ctx context.Context - cancel context.CancelFunc - queue chan packet - mux *UDPMux - ufrag string + ctx context.Context + cancel context.CancelFunc + queue chan packet + mux *UDPMux + localUfrag string } var _ net.PacketConn = &muxedConnection{} -func newMuxedConnection(mux *UDPMux, ufrag string) *muxedConnection { +func newMuxedConnection(mux *UDPMux, localUfrag string) *muxedConnection { ctx, cancel := context.WithCancel(mux.ctx) return &muxedConnection{ - ctx: ctx, - cancel: cancel, - queue: make(chan packet, queueLen), - mux: mux, - ufrag: ufrag, + ctx: ctx, + cancel: cancel, + queue: make(chan packet, queueLen), + mux: mux, + localUfrag: localUfrag, } } @@ -83,7 +83,7 @@ func (c *muxedConnection) Close() error { // must trigger the other. // Doing this here ensures we don't need to call both RemoveConnByUfrag // and close on all code paths. - c.mux.RemoveConnByUfrag(c.ufrag) + c.mux.RemoveConnByUfrag(c.localUfrag) return nil } diff --git a/p2p/transport/webrtc/udpmux/ufrag.go b/p2p/transport/webrtc/udpmux/ufrag.go new file mode 100644 index 0000000000..b504ffb93b --- /dev/null +++ b/p2p/transport/webrtc/udpmux/ufrag.go @@ -0,0 +1,62 @@ +package udpmux + +// The "libp2p+webrtc+" namespace selects the WebRTC Direct handshake version via +// the ICE username fragment. v1 ("libp2p+webrtc+v1/") munges the SDP; v2 +// ("libp2p+webrtc+v2/") does not, because Chromium's WebRTC-NoSdpMangleUfrag +// field trial rejects an offer whose ICE ufrag was munged. A v2 client instead +// leaves its local credentials untouched and puts its own ICE password in the +// server ufrag as "libp2p+webrtc+v2/", which the server reads back +// from the STUN USERNAME with no SDP exchange. A server MUST reject a version it +// does not recognize rather than guess one. v2 was introduced in +// https://github.com/libp2p/specs/pull/715; the spec lives at +// https://github.com/libp2p/specs/blob/master/webrtc/webrtc-direct.md and the +// js-libp2p implementation at https://github.com/libp2p/js-libp2p/pull/3480. +const ( + UfragPrefixV1 = "libp2p+webrtc+v1/" + UfragPrefixV2 = "libp2p+webrtc+v2/" +) + +const ( + // ICE credential bounds, per RFC 8839 section 5.4 + // (ufrag = 4*256ice-char, password = 22*256ice-char). + iceUfragMinLen = 4 + icePwdMinLen = 22 + iceCredentialMax = 256 +) + +// ice-char is ASCII-only, so any string that passes isICECharString has exactly +// one byte per character. The len() byte count used by isICEUfrag/isICEPwd then +// equals both the ice-char count and the UTF-16 length that js-libp2p measures, +// so the two implementations accept the same credentials; non-ice-char input +// (including any multi-byte UTF-8) is rejected by both regardless of how each +// counts length. + +// isICEChar reports whether b is in the RFC 8839 ice-char set +// (ALPHA / DIGIT / "+" / "/"). +func isICEChar(b byte) bool { + return (b >= 'a' && b <= 'z') || + (b >= 'A' && b <= 'Z') || + (b >= '0' && b <= '9') || + b == '+' || b == '/' +} + +func isICECharString(s string) bool { + for i := 0; i < len(s); i++ { + if !isICEChar(s[i]) { + return false + } + } + return true +} + +// isICEUfrag reports whether s is a syntactically valid ICE username fragment +// (RFC 8839 section 5.4: ufrag = 4*256ice-char). +func isICEUfrag(s string) bool { + return len(s) >= iceUfragMinLen && len(s) <= iceCredentialMax && isICECharString(s) +} + +// isICEPwd reports whether s is a syntactically valid ICE password +// (RFC 8839 section 5.4: password = 22*256ice-char). +func isICEPwd(s string) bool { + return len(s) >= icePwdMinLen && len(s) <= iceCredentialMax && isICECharString(s) +} diff --git a/p2p/transport/webrtc/udpmux/ufrag_test.go b/p2p/transport/webrtc/udpmux/ufrag_test.go new file mode 100644 index 0000000000..fb9edc769a --- /dev/null +++ b/p2p/transport/webrtc/udpmux/ufrag_test.go @@ -0,0 +1,94 @@ +package udpmux + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// ICE username fragments and passwords are parsed from an attacker-controlled +// STUN USERNAME and templated into the inferred client SDP offer, so they must +// be validated against the ice-char set and length bounds in RFC 8839 section +// 5.4 before use. +func TestICECredentialValidation(t *testing.T) { + v2Ufrag := UfragPrefixV2 + strings.Repeat("a", 24) + require.True(t, isICEUfrag("abcd")) // 4 chars, minimum ufrag + require.True(t, isICEUfrag("libp2p+webrtc+v1/abcdEFGH")) // '+' and '/' are ice-char + require.True(t, isICEUfrag(v2Ufrag)) + require.True(t, isICEPwd(strings.Repeat("a", 22))) // 22 chars, minimum password + + // charset violations (e.g. CRLF/SDP injection attempts) + require.False(t, isICEUfrag("abc\r\na=candidate:x")) + require.False(t, isICEUfrag("ab:cd")) // ':' is not an ice-char + require.False(t, isICEPwd(strings.Repeat("a", 21)+"\n")) + + // length violations + require.False(t, isICEUfrag("abc")) // 3 < 4 + require.False(t, isICEUfrag("")) // empty + require.False(t, isICEPwd(strings.Repeat("a", 21))) // 21 < 22 + require.False(t, isICEUfrag(strings.Repeat("a", 257))) // > 256 + + // multi-byte UTF-8 input: the len() byte count differs from the character + // count (and from the UTF-16 length js-libp2p measures), but both reject it + // on the charset check, so the length-representation difference between the + // implementations never changes the decision. + require.False(t, isICEUfrag("abécd")) // 'é' is 2 UTF-8 bytes and not ice-char + require.False(t, isICEUfrag("ab😀cd")) // emoji: 4 UTF-8 bytes / 2 UTF-16 units +} + +// credentialsFromSTUNMessage runs before the mux allocates any state, so every +// malformed or unsupported STUN USERNAME must be rejected here rather than +// surfaced as a candidate. +func TestCredentialsFromSTUNMessage(t *testing.T) { + v1 := UfragPrefixV1 + "abcdEFGH" + v2Pwd := strings.Repeat("p", 24) + v2 := UfragPrefixV2 + v2Pwd + clientUfrag := "browserClientUfrag" + + for _, tc := range []struct { + name string + username string + localUfrag string + remoteUfrag string + remotePwd string + wantErr bool + }{ + { + // v1 shares one value: the client password equals the client ufrag. + name: "valid v1", + username: v1 + ":" + v1, + localUfrag: v1, + remoteUfrag: v1, + remotePwd: v1, + }, + { + // v2 recovers the client password from the server ufrag. + name: "valid v2", + username: v2 + ":" + clientUfrag, + localUfrag: v2, + remoteUfrag: clientUfrag, + remotePwd: v2Pwd, + }, + {name: "missing colon", username: v1, wantErr: true}, + {name: "empty local ufrag", username: ":" + clientUfrag, wantErr: true}, + {name: "empty remote ufrag", username: v1 + ":", wantErr: true}, + {name: "local ufrag too short", username: "ab:" + clientUfrag, wantErr: true}, + {name: "remote ufrag not ice-char", username: v1 + ":ab\r\ncd", wantErr: true}, + {name: "unknown version prefix", username: "abcdEFGH:abcdEFGH", wantErr: true}, + {name: "v2 recovered password too short", username: UfragPrefixV2 + "short:" + clientUfrag, wantErr: true}, + } { + t.Run(tc.name, func(t *testing.T) { + msg := getSTUNBindingRequestWithUsername(tc.username) + local, remote, pwd, err := credentialsFromSTUNMessage(msg) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tc.localUfrag, local) + require.Equal(t, tc.remoteUfrag, remote) + require.Equal(t, tc.remotePwd, pwd) + }) + } +}