825925 - Use of dynamic codec number 109 in RTP can lead to negotiation failures

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P3)

Description

[Adam Roach [:abr]]

This is a bit of an odd corner case in the SIPCC code. The crystal-box summary of the issue is: if a dynamic payload type is received in an SDP offer and represents an unknown codec (not just unsupported but actually unknown), *and* that number happens to collide with the internal enumeration we use *and* the value it collides with happens to represent a supported codec, then the negotiation code currently treats that line as if it represents that supported codec (rather than the unknown codec).

In the current implementation, the only place this can arise is with dynamic payload type 109, which we use internally to represent Opus. In this case, the chances of failure are further diminished by the fact that Opus must use a clock rate of 48000 and have two channels indicated in the SDP (and is ignored otherwise).

Because of the rather complicated circumstances described above, the chances of this flaw arising accidentally in the field are extremely low. However, as we support additional codecs, the chances of this bug exhibiting goes up significantly. Consequently, we really do want to clean this handling up before we add any additional codecs to the mix.

As a trivial test case, the following offer should result in an offer that includes PCMU, but not opus:

v=0
o=- 1 1 IN IP4 148.147.200.251
s=-
b=AS:64
t=0 0
a=fingerprint:sha-256 F3:FA:20:C0:CD:48:C4:5F:02:5F:A5:D3:21:D0:2D:48:7B:31:60:5C:5A:D8:0D:CD:78:78:6C:6D:CE:CC:0C:67
m=audio 9000 RTP/AVP 109 0 8 126
c=IN IP4 148.147.200.251
b=TIAS:64000
a=rtpmap:109 FROG/48000/2
a=rtpmap:0 PCMU/8000
a=rtpmap:8 PCMA/8000
a=rtpmap:126 telephone-event/8000
a=candidate:0 1 udp 2130706432 148.147.200.251 9000 typ host
a=candidate:0 2 udp 2130706432 148.147.200.251 9005 typ host
a=ice-ufrag:cYuakxkEKH+RApYE
a=ice-pwd:bwtpzLZD+3jbu8vQHvEa6Xuq
a=sendrecv

The current negotiation code will return an answer which incorrectly indicates:

a=rtpmap:109 opus/48000/2

Comment 1

[Adam Roach [:abr]]

Attachment: Test case for dynamic type 109 with unknown codec

Comment 2

[Cullen Jennings]

For upstream tracking, this is CSCud97788

Comment 3

[Byron Campen [:bwc]]

Pretty sure this will work now, but can verify.

Comment 4

[Byron Campen [:bwc]]

The existing test-case VideoNegotiationFails in signaling_unittests verifies that this bug is fixed.