Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fails to parse (munged) SDP message #164

Closed
thomaseizinger opened this issue May 4, 2023 · 12 comments
Closed

Fails to parse (munged) SDP message #164

thomaseizinger opened this issue May 4, 2023 · 12 comments

Comments

@thomaseizinger
Copy link
Collaborator

I am trialing to use this library for rust-libp2p as a replacement for webrtc-rs. The protocol we implement using WebRTC is using ice-lite and we rely on SDP munging.

The SDP offer I am trying to feed into str0m errors on the parsing step.

This is my SDP message:

v=0
o=- 0 0 IN IP4 172.17.0.1
s=-
c=IN IP4 172.17.0.1
t=0 0
m=application 9999 UDP/DTLS/SCTP webrtc-datachannel
a=mid:0
a=ice-options:ice2
a=ice-ufrag:libp2p+webrtc+v1/a75469cf670c4079f8c06af4a963c8a1:libp2p+webrtc+v1/a75469cf670c4079f8c06af4a963c8a1
a=ice-pwd:libp2p+webrtc+v1/a75469cf670c4079f8c06af4a963c8a1:libp2p+webrtc+v1/a75469cf670c4079f8c06af4a963c8a1
a=fingerprint:sha-256 FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF
a=setup:actpass
a=sctp-port:5000
a=max-message-size:16384

The parser fails with:

Unexpected `a`\nExpected `c`\nsdp line\n

We have this code working in multiple other languages and browser bindings. From my limited research, this SDP message is still correct. Is the parser perhaps too strict here?

Any help is much appreciated!

@xnorpx
Copy link
Collaborator

xnorpx commented May 4, 2023

are those ice credentials correct formatted?

@xnorpx
Copy link
Collaborator

xnorpx commented May 5, 2023

seems like the parser wants a c line after the m line

let sdp_offer = "v=0\r
o=- 0 0 IN IP4 172.17.0.1\r
s=-\r
t=0 0\r
m=application 9999 UDP/DTLS/SCTP webrtc-datachannel\r
c=IN IP4 172.17.0.1\r
a=mid:0\r
a=ice-options:ice2\r
a=ice-ufrag:libp2p+webrtc+v1/a75469cf670c4079f8c06af4a963c8a1\r
a=ice-pwd:libp2p+webrtc+v1/a75469cf670c4079f8c06af4a963c8a1\r
a=fingerprint:sha-256 FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF:FF\r
a=setup:actpass\r
a=sctp-port:5000\r
a=max-message-size:16384\r
";
let offer = SdpOffer::from_sdp_string(sdp_offer).unwrap();

@xnorpx
Copy link
Collaborator

xnorpx commented May 5, 2023

so the fix is probably to make the conn line optional either in session or mediadescription.

@algesten
Copy link
Owner

algesten commented May 5, 2023

Hi @thomaseizinger welcome to st0m!

You're right. The spec says:

A session description MUST contain either at least one "c=" line in each media description or a single "c=" line at the session level. It MAY contain a single session-level "c=" line and additional media-level "c=" line(s) per-media-description, in which case the media-level values override the session-level settings for the respective media.

However the c-line is totally useless in WebRTC (str0m just writes c=IN IP4 0.0.0.0), and I'm certainly not interested in enforcing pointless standards for their own sake – let's make it optional everywhere! 🙌

@algesten
Copy link
Owner

algesten commented May 5, 2023

I made a fix for this in #165 and with that I can parse the above SDP.

@thomaseizinger I'm wondering, do you expect the c-line on session level to be used here? I notice you have no a=candidate line, so the only connectivity information is that IP address on the c-line.

str0m doesn't currently use the c-line, but it would be trivially easy to extract a host candidate for the c-line as a fallback (if the value is anything but 127.0.0.1 or 0.0.0.0).

@thomaseizinger
Copy link
Collaborator Author

I made a fix for this in #165 and with that I can parse the above SDP.

Awesome, thank you for the quick fix!

@thomaseizinger I'm wondering, do you expect the c-line on session level to be used here? I notice you have no a=candidate line, so the only connectivity information is that IP address on the c-line.

I think so yes (I am not too familiar with the internals of WebRTC). We've specified our protocol here: https://github.com/libp2p/specs/blob/master/webrtc/webrtc-direct.md#browser-to-public-server

See item 6:

B sets the connection field (c) to the IP and port of the incoming request c=IN .

Essentially, the setup we have is a publicly reachable node that listens for STUN requests and we want to establish a WebRTC connection to that same peer.

str0m doesn't currently use the c-line, but it would be trivially easy to extract a host candidate for the c-line as a fallback (if the value is anything but 127.0.0.1 or 0.0.0.0).

I think we will need that but I can report back once I continue my testing.

@thomaseizinger
Copy link
Collaborator Author

Thanks, this is fixed, I can now parse the SDP!

@thomaseizinger
Copy link
Collaborator Author

thomaseizinger commented May 12, 2023

After feeding the SDP into it, I am getting the following output:

2023-05-12T07:47:36.455616Z  INFO str0m::ice::agent: Set remote credentials: IceCreds { ufrag: "libp2p+webrtc+v1/b01d95e45dee6a3f4e77bb87081eb97b:libp2p+webrtc+v1/b01d95e45dee6a3f4e77bb87081eb97b", pass: "libp2p+webrtc+v1/b01d95e45dee6a3f4e77bb87081eb97b:libp2p+webrtc+v1/b01d95e45dee6a3f4e77bb87081eb97b" }
2023-05-12T07:47:36.455660Z  INFO str0m: DTLS setup is: false
2023-05-12T07:47:36.455720Z  INFO str0m::sctp: Uninited => AwaitRemoteAssociation
2023-05-12T07:47:36.455760Z DEBUG str0m::channel: Allocate channel id: ChannelId(0)
2023-05-12T07:47:36.455801Z TRACE str0m::ice::agent: Enqueueing event: IceRestart(IceCreds { ufrag: "77hg", pass: "ADebVBR6nBhkiq5R8a1iSa" })
2023-05-12T07:47:36.455829Z  INFO str0m::ice::agent: State change (new connection): New -> Checking
2023-05-12T07:47:36.455845Z TRACE str0m::ice::agent: Enqueueing event: IceConnectionStateChange(Checking)
2023-05-12T07:47:36.455864Z TRACE str0m::ice::agent: Stop timeout since ice-lite do no checks
2023-05-12T07:47:36.455871Z TRACE str0m::sctp: Handle timeout: Instant { tv_sec: 4949, tv_nsec: 489601509 }
2023-05-12T07:47:36.455882Z DEBUG str0m::channel: Associate stream id ChannelId(0) => 1
2023-05-12T07:47:36.455902Z TRACE str0m::ice::agent: Poll event: Some(IceRestart(IceCreds { ufrag: "77hg", pass: "ADebVBR6nBhkiq5R8a1iSa" }))
2023-05-12T07:47:36.455911Z TRACE str0m::ice::agent: Poll event: Some(IceConnectionStateChange(Checking))
2023-05-12T07:47:36.455918Z DEBUG str0m: IceConnectionStateChange(Checking)

After that, it repeats (I am guessing because the browser attempts to open multiple connections).

@thomaseizinger I'm wondering, do you expect the c-line on session level to be used here? I notice you have no a=candidate line, so the only connectivity information is that IP address on the c-line.

str0m doesn't currently use the c-line, but it would be trivially easy to extract a host candidate for the c-line as a fallback (if the value is anything but 127.0.0.1 or 0.0.0.0).

@algesten Is this related to the above? Are things hanging because we don't extract candidates?

@thomaseizinger
Copy link
Collaborator Author

So I think I fixed a few more things and now I am stuck here:

2023-05-12T10:42:00.251207Z  INFO chat: Received STUN from libp2p+webrtc+v1/200fe8610ae91eada12381fcd2bac620:libp2p+webrtc+v1/200fe8610ae91eada12381fcd2bac620
2023-05-12T10:42:00.514123Z  INFO str0m::ice::agent: Add remote candidate: Candidate(host=192.168.1.16:49340 prio=2130706175)
2023-05-12T10:42:00.514251Z  INFO str0m::ice::agent: Add local candidate: Candidate(host=192.168.1.16:9999 prio=2130706175)
2023-05-12T10:42:00.514318Z TRACE str0m::ice::agent: Calculated local preference: 65534
2023-05-12T10:42:00.514404Z TRACE str0m::ice::agent: Form pair local: Candidate(host=192.168.1.16:9999 prio=2130706175) remote: Candidate(host=192.168.1.16:49340 prio=2130706175)
2023-05-12T10:42:00.514443Z DEBUG str0m::ice::agent: Add new pair CandidatePair(0-0 prio=9151313343271665150 state=Waiting attempts=0 unanswered=0 remote=0 last=None nom=None)
2023-05-12T10:42:00.514497Z  INFO str0m::ice::agent: Set remote credentials: IceCreds { ufrag: "libp2p+webrtc+v1/200fe8610ae91eada12381fcd2bac620", pass: "libp2p+webrtc+v1/200fe8610ae91eada12381fcd2bac620" }
2023-05-12T10:42:00.514537Z DEBUG str0m::channel: Allocate channel id: ChannelId(0)
2023-05-12T10:42:00.514654Z TRACE str0m::ice::agent: Enqueueing event: IceRestart(IceCreds { ufrag: "fvXG", pass: "gIKOgs5HnbLoONmA7ZL6fx" })
2023-05-12T10:42:00.514728Z  INFO str0m::ice::agent: State change (new connection): New -> Checking
2023-05-12T10:42:00.514752Z TRACE str0m::ice::agent: Enqueueing event: IceConnectionStateChange(Checking)
2023-05-12T10:42:00.514786Z DEBUG str0m::ice::agent: Remove failed pair: CandidatePair(0-0 prio=9151313343271665150 state=Waiting attempts=0 unanswered=0 remote=0 last=None nom=None)
2023-05-12T10:42:00.514818Z  INFO str0m::ice::agent: State change (no possible pairs): Checking -> Disconnected
2023-05-12T10:42:00.514839Z TRACE str0m::ice::agent: Enqueueing event: IceConnectionStateChange(Disconnected)
2023-05-12T10:42:00.514865Z TRACE str0m::ice::agent: Stop timeout since ice-lite do no checks
2023-05-12T10:42:00.514926Z TRACE str0m::ice::agent: Poll event: Some(IceRestart(IceCreds { ufrag: "fvXG", pass: "gIKOgs5HnbLoONmA7ZL6fx" }))
2023-05-12T10:42:00.514959Z TRACE str0m::ice::agent: Poll event: Some(IceConnectionStateChange(Checking))
2023-05-12T10:42:00.515006Z DEBUG str0m: IceConnectionStateChange(Checking)

I am using the direct_api now but it still doesn't work. I'll keep playing around but some guidance would be appreciated. I am using a modified version of the chat example as a playground: https://github.com/thomaseizinger/str0m/tree/libp2p-webrtc-playground, together with this client: https://tomaka.github.io/test-browser-node/index.html

@xnorpx
Copy link
Collaborator

xnorpx commented May 12, 2023

@thomaseizinger

Maybe open a new issue with the topic use str0m in rust libp2p.

In that one can you describe how the overall system looks like. Is the str0m side act like client/server or both? What are the other peers, only browsers or other webrtc implementations.

Can you also try to replicate the problem you have with an integration test where you let one side of str0m behave like your other client.

@algesten
Copy link
Owner

@thomaseizinger is that code supposed to "just work" with a valid webrtc peer?

@thomaseizinger
Copy link
Collaborator Author

@thomaseizinger

Maybe open a new issue with the topic use str0m in rust libp2p.

Done: #166

@thomaseizinger is that code supposed to "just work" with a valid webrtc peer?

We use SDP munging but other than that, it is a compliant WebRTC use afaik. We do use the first channel for a dedicated handshake but I don't think that should be relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants