Closed Bug 1279004 Opened 9 years ago Closed 9 years ago

Don't decode SRTCP packets with the wrong SSRC

Categories

(Core :: WebRTC: Audio/Video, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: drno, Assigned: drno)

Details

Attachments

(1 file)

According to https://tools.ietf.org/html/rfc3711#section-3.4 the first 8 bytes of a SRTCP are not encrypted. But our current code first decodes the SRTCP message, but then calls FilterSenderReport() on it https://dxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp?from=MediaPipeline.cpp%3A1000#1013 which only looks at the first 8 bytes. So we spare some CPU cycles by doing decryption only after doing filtering.
Comment on attachment 8761344 [details] Bug 1279004: do RTCP decryption after filtering. https://reviewboard.mozilla.org/r/58578/#review55560 r+ but fix the issue opened here or in a different bug. ::: media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp:997 (Diff revision 1) > + // We do not filter RTCP for send pipelines, since the webrtc.org code for > + // senders already has logic to ignore RRs that do not apply. Perhaps so, but decrypting them isn't zero cost. that said, this is basically ok. ::: media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp:999 (Diff revision 1) > + if (filter_ && direction_ == RECEIVE) { > + if (!filter_->FilterSenderReport(data, len)) { > + MOZ_MTLOG(ML_NOTICE, "Dropping incoming RTCP packet; filtered out"); > + return; > + } > + } Per rfc 5506 3.4.3 (reduced-size RTCP) an SRTCP packet does NOT have to start with an RR or SR, but just has to have a normal RTCP packet header which has the SSRC in the unencrypted portion (which makes this moving of the code possible to begin with). So the check for SENDER_REPORT_T in FilterSenderReport needs to change (and the function name is incorrect as well). https://tools.ietf.org/html/rfc5506#section-3.4.3
Attachment #8761344 - Flags: review?(rjesup) → review+
Comment on attachment 8761344 [details] Bug 1279004: do RTCP decryption after filtering. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58578/diff/1-2/
https://reviewboard.mozilla.org/r/58578/#review55560 > Per rfc 5506 3.4.3 (reduced-size RTCP) an SRTCP packet does NOT have to start with an RR or SR, but just has to have a normal RTCP packet header which has the SSRC in the unencrypted portion (which makes this moving of the code possible to begin with). So the check for SENDER_REPORT_T in FilterSenderReport needs to change (and the function name is incorrect as well). > https://tools.ietf.org/html/rfc5506#section-3.4.3 Created bug 1279153 and added a comment reference into the code to that new bug.
Pushed by drno@ohlmeier.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/d76d3fe00cc1 do RTCP decryption after filtering. r=jesup
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: