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

Wrong extension id for encrypted headers #1575

Closed
fancycode opened this Issue Apr 9, 2019 · 5 comments

Comments

Projects
None yet
2 participants
@fancycode
Copy link
Contributor

fancycode commented Apr 9, 2019

In some cases, Janus is sending an answer with a wrong extension id if encrypted extension headers are used. You can test this by enabling the flag "Negotiation with encrypted header extensions for SRTP in WebRTC" in chrome://flags.

Offer from Chrome:

...
m=video 9 UDP/TLS/RTP/SAVPF 96 97 98 99 100 101 102 122 127 121 125 107 108 109 124 120 123
...
a=extmap:2 urn:ietf:params:rtp-hdrext:toffset
a=extmap:3 http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time
a=extmap:4 urn:3gpp:video-orientation
a=extmap:5 http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01
a=extmap:6 http://www.webrtc.org/experiments/rtp-hdrext/playout-delay
a=extmap:7 http://www.webrtc.org/experiments/rtp-hdrext/video-content-type
a=extmap:8 http://www.webrtc.org/experiments/rtp-hdrext/video-timing
a=extmap:10 http://tools.ietf.org/html/draft-ietf-avtext-framemarking-07
a=extmap:12 http://www.webrtc.org/experiments/rtp-hdrext/color-space
a=extmap:13 urn:ietf:params:rtp-hdrext:encrypt urn:ietf:params:rtp-hdrext:toffset
a=extmap:11 urn:ietf:params:rtp-hdrext:encrypt urn:3gpp:video-orientation
a=extmap:9 urn:ietf:params:rtp-hdrext:encrypt http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01
a=extmap:0 urn:ietf:params:rtp-hdrext:encrypt http://www.webrtc.org/experiments/rtp-hdrext/playout-delay
a=extmap:0 urn:ietf:params:rtp-hdrext:encrypt http://www.webrtc.org/experiments/rtp-hdrext/video-content-type
...

Answer from Janus:

...
m=video 9 UDP/TLS/RTP/SAVPF 96
...
a=extmap:11 urn:3gpp:video-orientation
a=extmap:0 http://www.webrtc.org/experiments/rtp-hdrext/playout-delay
...

Note that the extensions got mapped to the id of the encrypted version from the offer (that has the urn:ietf:params:rtp-hdrext:encrypt as prefix before the actual extension.

The problem is caused by the code that checks if the extension name is part of the m-line attribute in

} else if(videoroom->videoorient_ext && m->type == JANUS_SDP_VIDEO && strstr(a->value, JANUS_RTP_EXTMAP_VIDEO_ORIENTATION)) {

In this case this matches both the unencrypted and the encrypted extension and the last matching id (here the encrypted) is used.
(On a side note, the id 0 means that an unused number should be assigned in the answer, so replying with extmap:0 is invalid, too).

When setting the answer, Chrome fails with "Failed to set remote answer sdp: Failed to set remote video description send parameters.".

@lminiero

This comment has been minimized.

Copy link
Member

lminiero commented Apr 11, 2019

We don't support encrypted RTP extensions yet, and they require ad-hoc negotiation as you pointed out. I'll keep the issue open as a memo that we need to implement it, but I'm not sure when that will happen: AFAIK they're still hidden behind a flag anyway.

@fancycode

This comment has been minimized.

Copy link
Contributor Author

fancycode commented Apr 12, 2019

It's fine to not support encrypted extensions, that's why Chrome is offering the same extensions both encrypted and unencrypted. However it's not allowed to answer with the unencrypted extension but with the id of the encrypted extension:

Offer:

a=extmap:4 urn:3gpp:video-orientation
a=extmap:11 urn:ietf:params:rtp-hdrext:encrypt urn:3gpp:video-orientation

Answer:

a=extmap:11 urn:3gpp:video-orientation

The solution for now would be to ignore all encrypted extensions (that contain urn:ietf:params:rtp-hdrext:encrypt) and not use their ids for the answer.

@fancycode

This comment has been minimized.

Copy link
Contributor Author

fancycode commented Apr 12, 2019

Thinking more about this, re-assigning ids should be fine (i.e. using 11 or any other number for urn:3gpp:video-orientation in my example), but replying with an id of 0 is not. For now the fix would be to ignore the encrypted extensions (which luckily are all using the dynamic id 0).
However if Chrome would change the order in the SDP and use assigned ids for the encrypted extensions and 0 for the unencrypted, this would no longer work and you would need to assign an id in Janus for the answer.

fancycode added a commit to fancycode/janus-gateway that referenced this issue Apr 12, 2019

@fancycode

This comment has been minimized.

Copy link
Contributor Author

fancycode commented Apr 12, 2019

Fix available in #1581.

@lminiero

This comment has been minimized.

Copy link
Member

lminiero commented Apr 15, 2019

Closing as it should be fixed in the commit above.

@lminiero lminiero closed this Apr 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.