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 } diff --git a/p2p/transport/webrtc/listener.go b/p2p/transport/webrtc/listener.go index 4616f0cfb6..0d3701bb83 100644 --- a/p2p/transport/webrtc/listener.go +++ b/p2p/transport/webrtc/listener.go @@ -135,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 } @@ -196,9 +196,15 @@ func (l *listener) setupConnection( } }() + // 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) - settingEngine.SetICECredentials(candidate.Ufrag, candidate.Ufrag) + settingEngine.SetICECredentials(serverUfrag, serverUfrag) settingEngine.SetLite(true) settingEngine.SetICEUDPMux(l.mux) settingEngine.SetIncludeLoopbackCandidate(true) @@ -224,9 +230,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, candidate.RemoteUfrag, candidate.RemotePwd), Type: webrtc.SDPTypeOffer, }); err != nil { return nil, err @@ -244,7 +252,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..e474c0094b 100644 --- a/p2p/transport/webrtc/sdp_test.go +++ b/p2p/transport/webrtc/sdp_test.go @@ -68,16 +68,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..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,6 +90,14 @@ type WebRTCTransport struct { listenUDP func(network string, laddr *net.UDPAddr) (net.PacketConn, error) + // 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 peerConnectionTimeouts iceTimeouts @@ -100,6 +109,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 @@ -162,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 { @@ -301,17 +331,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 1: + localUfrag = genUfrag() + localPwd = localUfrag + serverUfrag = localUfrag + case 2: + localUfrag, localPwd = genV2ClientCredentials() + serverUfrag = udpmux.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 +400,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 +472,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 udpmux.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..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,8 +31,19 @@ const ReceiveBufSize = 1500 const maxAddrsPerUfrag = 32 type Candidate struct { - Ufrag string - Addr *net.UDPAddr + // 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 LocalUfrag; in v2 the two differ. + RemoteUfrag string + // 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, @@ -56,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 @@ -105,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) @@ -115,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 } } @@ -203,18 +214,18 @@ func (mux *UDPMux) processPacket(buf []byte, addr net.Addr) (processed bool) { return false } - ufrag, err := ufragFromSTUNMessage(msg) + localUfrag, remoteUfrag, remotePwd, err := credentialsFromSTUNMessage(msg) if err != nil { - log.Debug("could not find STUN username", "error", err) + log.Debug("dropping STUN binding request with invalid credentials", "error", err) return false } - connCreated, conn := mux.getOrCreateConn(ufrag, isIPv6, mux, udpAddr) + connCreated, conn := mux.getOrCreateConn(localUfrag, isIPv6, mux, udpAddr) if connCreated { select { - case mux.queue <- Candidate{Addr: udpAddr, Ufrag: ufrag}: + case mux.queue <- Candidate{Addr: udpAddr, LocalUfrag: localUfrag, RemoteUfrag: remoteUfrag, RemotePwd: remotePwd}: default: - log.Debug("queue full, dropping incoming candidate", "ufrag", ufrag, "addr", udpAddr) + log.Debug("queue full, dropping incoming candidate", "ufrag", localUfrag, "addr", udpAddr) conn.Close() return false } @@ -239,44 +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 } -// 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) { +// 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 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. +// +// 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+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] { @@ -288,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() @@ -306,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 1fce6501e3..e7ec008900 100644 --- a/p2p/transport/webrtc/udpmux/mux_test.go +++ b/p2p/transport/webrtc/udpmux/mux_test.go @@ -31,6 +31,86 @@ 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 +} + +// 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, +// 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() + + clientPwd := "browserClientPassword1234" + serverUfrag := UfragPrefixV2 + clientPwd + 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.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} @@ -47,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) @@ -56,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()) } @@ -84,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) @@ -93,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()) } @@ -140,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 { @@ -161,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) } @@ -176,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") @@ -192,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 { @@ -268,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) + }) + } +}