feat: webrtc-direct-v2#3480
Conversation
|
Thanks for driving this forward. WebRTC Direct v2 is an important piece of work for libp2p, and it's good to see the implementation and spec progressing together. Getting this into js-libp2p provides a clear reference for other implementations i.e python and moves us closer to a common interoperable approach across the ecosystem. The focus on testing and alignment with the spec will make adoption easier for maintainers across the different stacks. Looking forward to seeing this land and become the standard WebRTC Direct path for libp2p. Thanks again for the work on this. Johanna |
Refine the initial v2 implementation to match the now-precise spec (libp2p/specs#715) so it can't diverge from go-libp2p or other peers. - listener: dispatch on the ufrag prefix explicitly (v1, v2, or reject an unknown version) instead of treating anything non-v2 as v1. - validation: reject STUN USERNAME fragments outside the RFC 8839 ice-char set or length bounds before they reach SDP, and validate the recovered v2 client password. - dialer: fail fast when the local ICE password can't be read back, instead of dialing with an unprefixed ufrag the server rejects. - cleanup: drop the dead createOffer guard and the dead ice-pwd zero-padding (both unreachable; the padding could also desync the inferred offer from go-libp2p). - tests: cover the credential validation and the unprefixed-ufrag rejection.
# Conflicts: # packages/transport-webrtc/src/private-to-public/utils/sdp.ts # packages/transport-webrtc/test/transport.spec.ts
lidel
left a comment
There was a problem hiding this comment.
Thanks @dozyio for spec and implementing v2!
LGTM, I pushed some changes on top:
- Hardened the v2 path: explicit version dispatch on the ufrag prefix (reject
unknown versions rather than assuming v1), ICE credential validation against
the RFC 8839 ice-char set and length bounds on the STUNUSERNAMEbefore it
reaches SDP, fail-fast when the local ICE password can't be read back, and
removed some dead code. - Merged latest
main(resolved a couple of import conflicts from the.jsto.tsmove).
I also tested it end to end against real Chrome with munging disabled, and it
behaves as expected:
- v1 fails:
setLocalDescriptionrejects the munged ufrag ("SDP is modified
in a non-acceptable way") - v2 succeeds: Noise-authenticated on both the JS and Go sides
Repro:
- Chrome for Testing 150.0.7871.24, launched with
--force-fieldtrials=WebRTC-NoSdpMangleUfrag/Enabled/ - This branch's JS v2 dialer against the go-libp2p v2 server in libp2p/go-libp2p#3520
The rationale for the spec gaps we closed (server credentials, version dispatch,
ICE roles, RFC references) is written up in this review: libp2p/specs#715 (review)
seetadev
left a comment
There was a problem hiding this comment.
Approved. Great work by @dozyio and @lidel on driving WebRTC Direct v2 from both the specification and implementation sides. The implementation is well-structured, the test coverage is strong, and the alignment with the evolving spec provides a solid reference point for other libp2p implementations, including Go, Python, and future transport stacks.
Appreciate @lidel's follow-up changes and review. The validation checks, explicit version dispatch, RFC-aligned ICE credential handling, fail-fast behavior, cleanup of dead code, and successful end-to-end interoperability testing with Chrome and go-libp2p significantly strengthen the robustness and readiness of this implementation. These changes help ensure a safer migration path and improve long-term maintainability.
Thanks to @johannamoran for actively coordinating discussions across repositories and implementations, helping keep the specification, js-libp2p, go-libp2p, and broader ecosystem efforts aligned. Looking forward to bootstrapping transport interop testing.
Description
Initial implementation based on proposed spec libp2p/specs#715
Requires node-datachannel v0.32.3+
Change checklist