Skip to content

Commit

Permalink
Set the ufrag/pwd according to the spec (#2924)
Browse files Browse the repository at this point in the history
Seeing the small discussion in the WebRTC implementation PR, I realized
that smoldot doesn't actually say what is in the spec now. This PR
changes it.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
tomaka and mergify[bot] authored Oct 31, 2022
1 parent b5ffb56 commit fba43ad
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
4 changes: 4 additions & 0 deletions bin/wasm-node/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
- Fix errors showing in the browser's console about WebSockets being already in the CLOSING or CLOSED state. ([#2925](https://github.com/paritytech/smoldot/pull/2925))
- No longer panic when a WebRTC connection fails to open due to the browser calling callbacks in an unexpected order. ([#2936](https://github.com/paritytech/smoldot/pull/2936))

### Fixed

- When opening a WebRTC connection, the ufrag and password of SDP requests are now properly set according to the WebRTC libp2p specification. ([#2924](https://github.com/paritytech/smoldot/pull/2924))

## 0.7.3 - 2022-10-19

### Changed
Expand Down
20 changes: 12 additions & 8 deletions bin/wasm-node/javascript/src/index-browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,13 +342,18 @@ export function start(options?: ClientOptions): Client {
console.error("Local offer doesn't contain UDP data channel. WebRTC connections will likely fail. Please report this issue.");
}
// According to the libp2p WebRTC spec, the ufrag and pwd are the same
// randomly-generated string. We modify the local description to ensure that.
const pwd = sdpOffer.match(/^a=ice-pwd:(.+)$/m);
if (pwd != null) {
sdpOffer = sdpOffer.replace(/^a=ice-ufrag.*$/m, 'a=ice-ufrag:' + pwd[1]);
} else {
// randomly-generated string on both sides, and must be prefixed with
// `libp2p-webrtc-v1:`. We modify the local description to ensure that.
// While we could randomly generate a new string, we just grab the one that the
// browser has generated, in order to make sure that it respects the constraints
// of the ICE protocol.
const browserGeneratedPwd = sdpOffer.match(/^a=ice-pwd:(.+)$/m)?.at(1);
if (browserGeneratedPwd === undefined) {
console.error("Failed to set ufrag to pwd. WebRTC connections will likely fail. Please report this issue.");
}
const ufragPwd = "libp2p+webrtc+v1/" + browserGeneratedPwd;
sdpOffer = sdpOffer.replace(/^a=ice-ufrag.*$/m, 'a=ice-ufrag:' + ufragPwd);
sdpOffer = sdpOffer.replace(/^a=ice-pwd.*$/m, 'a=ice-pwd:' + ufragPwd);
await pc!.setLocalDescription({ type: 'offer', sdp: sdpOffer });

// Transform certificate hash into fingerprint (upper-hex; each byte separated by ":").
Expand Down Expand Up @@ -390,10 +395,9 @@ export function start(options?: ClientOptions): Client {
"a=ice-options:ice2" + "\n" +
// ICE username and password, which are used for establishing and
// maintaining the ICE connection. (RFC8839)
// MUST match ones used by the answerer (server).
// These values are set according to the libp2p WebRTC specification.
"a=ice-ufrag:" + remoteCertMultibase + "\n" +
"a=ice-pwd:" + remoteCertMultibase + "\n" +
"a=ice-ufrag:" + ufragPwd + "\n" +
"a=ice-pwd:" + ufragPwd + "\n" +
// Fingerprint of the certificate that the server will use during the TLS
// handshake. (RFC8122)
// MUST be derived from the certificate used by the answerer (server).
Expand Down

0 comments on commit fba43ad

Please sign in to comment.