RTCP Compound Packets broken for WebRTC connections

[moorecj]

I am finding that the RTCP Compound packets for WebRTC connections are causing issues in the current master.

By the looks of it the RTCP Compound packets data is trying to be parsed and the data being passed in is bad or there is some RTCP Compound packet case is not being handled.

           //      RTCP Header Format example
            0                   1                   2                   3
            0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
            +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
            |V=2|P|    RC   |   PT=SR=200   |             Length            |
            +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Here is an example back packet being passed into the public RTCPCompoundPacket(byte[] packet) constructor packet{byte[70]}

//Valid Header: Version= 2, Padding =0, Report Count=1, Packet Type=201 (Receiver Report)  Length = 7 
[0]: 129   [1]: 201   [2]: 0     [3]: 7   
[4]: 0     [5]: 0     [6]: 0     [7]: 1 
[8]: 136   [9]: 126   [10]: 182  [11]: 6 
[12]: 82   [13]: 140  [14]: 28   [15]: 162
[16]: 62   [17]: 24   [18]: 92   [19]: 147
[20]: 54   [21]: 225  [22]: 39   [23]: 93 
[24]: 133  [25]: 49   [26]: 217  [27]: 96
[28]: 169  [29]: 69   [30]: 63   [31]: 183   <---Length is defined as number of 32bit words minus 1.   (7+1)*4 = 32 bytes
//Issue starts here:
[32]: 35  <---------  Version = 0, Padding = 1 // Not Valid Version
[33]: 202 <--------- Packet Type:  SDES //Valid Packet type
[34]: 4   [35]: 76   <---Length: 1100  //Not Valid Length
[36]: 152 [37]: 35  [38]: 100 [39]: 79 
[40]: 162 [41]: 225 [42]: 108 [43]: 5
[44]: 171 [45]: 126 [46]: 81  [47]: 18
[48]: 108 [49]: 128 [50]: 60  [51]: 65
[52]: 159 [53]: 249 [54]: 170 [55]: 65
[56]: 128 [57]: 0   [58]: 0   [59]: 46 
[60]: 137 [61]: 135 [62]: 124 [63]: 156
[64]: 1   [65]: 29  [66]: 81  [67]: 117
[68]: 253 [69]: 154

My thought is that at index 32 it does not conform to RTCP header format which makes me think the data is bad.

I am pretty sure this is directly related to this issue #714. As its causing causing random data to be processed and is hitting the 'BYE' command which always kills any video stream without audio.

Can anyone confirm that this is indeed bad data? Or maybe recognize what this addition data might be?

[NoOverflow]

This packet has SSRC=1 which doesn't (at least for my case) correspond to any track since the only (video) track I got is SSRC=0, this causes the media type to be invalid and therefore skip the secureContext.UnprotectRtcpPacket() part which results in a "Bye" packet.

If I force that SSRC to be 0, it goes through secureContext.UnprotectRtcpPacket() and the resulting packet has .Bye=null and a .Feedback != null. With this, the issue disappears. Now this isn't a fix in itself at all, just an observation, but it does concur with the fact that all of this logic was reworked in a6b16d634e4689984a690776e8b3aaa3a7f84870 (which I'm pretty sure after some tests, is the commit that introduced this problem).

My knowledge in RTCP is really limited so I'm going to have to dig in the RFC, because maybe an invalid media type is expected behaviour and my whole assumption is wrong.

Update: This whole SSRC - MediaType logic never existed before this commit (See the file before commit a6b16d634e4689984a690776e8b3aaa3a7f84870)

Buffer without UnprotectRtcpPacket call due to SSRC unknown, media type invalid: [0]: 129 [1]: 201 [2]: 0 [3]: 7 [4]: 0 [5]: 0 [6]: 0 [7]: 1 [8]: 209 [9]: 190 [10]: 39 [11]: 89 [12]: 229 [13]: 93 [14]: 225 [15]: 100 [16]: 164 [17]: 50 [18]: 94 [19]: 86 [20]: 60 [21]: 238 [22]: 81 [23]: 241 [24]: 102 [25]: 180 [26]: 169 [27]: 82 [28]: 246 [29]: 198 [30]: 241 [31]: 40 [32]: 146 [33]: 203 [34]: 120 [35]: 163 [36]: 67 [37]: 238 [38]: 131 [39]: 46 [40]: 137 [41]: 147 [42]: 39 [43]: 138 [44]: 132 [45]: 247 [46]: 11 [47]: 128 [48]: 41 [49]: 139 [50]: 99 [51]: 89 [52]: 102 [53]: 143 [54]: 213 [55]: 28 [56]: 128 [57]: 0 [58]: 0 [59]: 61 [60]: 136 [61]: 165 [62]: 1 [63]: 126 [64]: 187 [65]: 231 [66]: 43 [67]: 84 [68]: 6 [69]: 142

Resulting packet without UnprotectRtcpPacket call due to SSRC unknown, media type invalid: {SIPSorcery.Net.RTCPCompoundPacket} Bye: {SIPSorcery.Net.RTCPBye} Feedback: null ReceiverReport: {SIPSorcery.Net.RTCPReceiverReport} SDesReport: null SenderReport: null

Buffer WITH UnprotectRtcpPacket call with forced SSRC:
[0]: 129 [1]: 201 [2]: 0 [3]: 7 [4]: 0 [5]: 0 [6]: 0 [7]: 1 [8]: 25 [9]: 235 [10]: 12 [11]: 211 [12]: 0 [13]: 0 [14]: 0 [15]: 0 [16]: 0 [17]: 0 [18]: 87 [19]: 127 [20]: 0 [21]: 0 [22]: 1 [23]: 107 [24]: 46 [25]: 153 [26]: 0 [27]: 41 [28]: 0 [29]: 6 [30]: 155 [31]: 157 [32]: 143 [33]: 206 [34]: 0 [35]: 5 [36]: 0 [37]: 0 [38]: 0 [39]: 1 [40]: 0 [41]: 0 [42]: 0 [43]: 0 [44]: 82 [45]: 69 [46]: 77 [47]: 66 [48]: 1 [49]: 1 [50]: 97 [51]: 203 [52]: 25 [53]: 235 [54]: 12 [55]: 211

Resulting packet WITH UnprotectRtcpPacket call: {SIPSorcery.Net.RTCPCompoundPacket} Bye: null Feedback: {SIPSorcery.Net.RTCPFeedback} ReceiverReport: {SIPSorcery.Net.RTCPReceiverReport} SDesReport: null SenderReport: null

[moorecj]

Wow this looks like its the key!

From the Buffer WITH UnprotectRtcpPacket call with forced SSRC: The values from [32 - 55] look correct!

[32]: 143 [33]: 206 [34]: 0 [35]: 5 <--Valid header For PSFB | 206 | Payload-specific FB message [36]: 0 [37]: 0 [38]: 0 [39]: 1 [40]: 0 [41]: 0 [42]: 0 [43]: 0 [44]: 82 [45]: 69 [46]: 77 [47]: 66 [48]: 1 [49]: 1 [50]: 97 [51]: 203 [52]: 25 [53]: 235 [54]: 12 [55]: 211 <--Correct length

Does 1 make sense as an SSRC? It should be a random 32 bit number. Notice we both got SSRC = 1 separately. I will need to review the SSRC. Maybe SSRC just needs to be set correctly?

[NoOverflow]

I guess the next step is to investigate how a6b16d634e4689984a690776e8b3aaa3a7f84870 manages the SRTP packets mapping to crypto contexts and SSRCs because I strongly believe the issue could come from here.

[sipsorcery]

The reason for the invalid RTCP packets is because they are not being decrypted by the SRTP context. The reason they're not being decrypted is becuase the logic that chooses the stream, and its associated SRTP context, needs some improvement. I've done one small tweak which should help in cases where there is only one stream.

[moorecj]

I am currently looking into how this gets set. It seems we are not properly communicating our SSRC number to remote connections.

In the use case where I see this issue. Sipsorcery has local a stream and initiates the offer to a Chrome. The signaling all goes through and the streams start data flowing but the reporting from the browser has the SSRC set to 1. From my quick research this is the default from Chrome.

I am thinking SSRC in the initial SDP from Sipsorcery is what the browser should be marking the reports. Maybe there is a bug in format of the SSRC we are passing to the browser via the SDP?

I might be making wrong assumptions here.

[sipsorcery]

One thing to double check is whether it's a one way stream, i.e. sendonly from sipsorcery to Chrome or vice-versa. In those cases it may be that the SSRC of 1 is used for the sender report to indicate an inactive stream. I can't recall seeing that anywhere in the specs, but I could have easily missed it.

A recent change to the sipsorcery RTPSession to support more streams means it's now relying on the SSRC to determine the correct SRTP key to decrypt packets. I have a suspicion that won't work for RTCP Sender Reports with an inactive remote stream.

[grondinjc]

Is this still active in the recent version 6.0.8 ?