Closed Bug 1161317 Opened 9 years ago Closed 9 years ago

Incorrect encryption of RTCP Packets when using unidirectional PeerConnections

Categories

(Core :: WebRTC: Networking, defect, P1)

38 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox37 --- unaffected
firefox38 + fixed
firefox38.0.5 + fixed
firefox39 + fixed
firefox40 + fixed
firefox-esr38 --- fixed

People

(Reporter: ggb, Assigned: bwc)

References

Details

Attachments

(2 files, 1 obsolete file)

When using unidirectional PeerConnections (the sender has a MediaStream attached, the receiver doesn't) the RTCP generated by the sender is not encrypted properly.  At least it is a problem with the SR packets, which makes not possible to synchronize audio and video on the receiver side. 

How to reproduce it:
1) Activate NSPR logs including mediapipeline:9
2) Open this page https://dl.dropboxusercontent.com/u/1806026/ff_unidirectional.html
3) Find SRTCP errors in the logs: "Error unprotecting SRTCP packet error=7"
FWIW We think that maybe those SR messages are not even encrypted.  Find attachment of a wireshark capture where you are able to see the real value of the NTP field
Attached image SR_capture.png
Attached file MozReview Request: bz://1161317/bwc (obsolete) —
/r/8125 - Bug 1161317: Fix bug where sendonly video RTCP would be treated as outgoing RTP.

Pull down this commit:

hg pull -r 8f39f1eff1f6568885ede279dbd4670d67348f84 https://reviewboard-hg.mozilla.org/gecko/
Verified that the patch at least fixes the error logging.
Comment on attachment 8601206 [details]
MozReview Request: bz://1161317/bwc

https://treeherder.mozilla.org/#/jobs?repo=try&revision=90e8085bef88
Attachment #8601206 - Flags: review?(rjesup)
Comment on attachment 8601206 [details]
MozReview Request: bz://1161317/bwc

https://reviewboard.mozilla.org/r/8123/#review6895

r+!
Attachment #8601206 - Flags: review?(rjesup) → review+
[Tracking Requested - why for this release]:
This is a significant regression where important RTCP packets in unidirectional streams are being lost, potentially causing degraded lipsync and other issues (perhaps failed error recovery, depending on what packets are not sent correctly).

This regressed in 38 due to the landing of bug 1130534
Assignee: nobody → docfaraday
Blocks: 1130534
Rank: 5
Component: WebRTC: Audio/Video → WebRTC: Networking
Priority: -- → P1
Whiteboard: [webrtc-uplift]
[Tracking Requested - why for this release]:
We really want this in any ESR release based on 38
Comment on attachment 8601206 [details]
MozReview Request: bz://1161317/bwc

Approval Request Comment
[Feature/regressing bug #]:

   Bug 1130534

[User impact if declined]:

   Problems with audio/video sync in webrtc services that use unidirectional media.

[Describe test coverage new/current, TreeHerder]:

   Unfortunately, the testing we have around RTCP is rudimentary at best. Fixing this will be more involved, so it does not make sense to block this fix.

[Risks and why]: 

   Extremely low, since this is a typo correction that causes video RTCP for sendonly use the same code-path that RTCP already uses otherwise.

[String/UUID change made/needed]:

   None.
Attachment #8601206 - Flags: approval-mozilla-beta?
Attachment #8601206 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/4b6544010bdb
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
This is fixed
Comment on attachment 8601206 [details]
MozReview Request: bz://1161317/bwc

Approved for uplift to aurora, small typo fix, and we definitely want to have encryption working.
Attachment #8601206 - Flags: approval-mozilla-esr38?
Attachment #8601206 - Flags: approval-mozilla-aurora?
Attachment #8601206 - Flags: approval-mozilla-aurora+
Comment on attachment 8601206 [details]
MozReview Request: bz://1161317/bwc

OK, let's take it in release (38.0.5), esr 38.1.0 & the 38.0 relbranch release ( GECKO380_2015050320_RELBRANCH )
Attachment #8601206 - Flags: approval-mozilla-release+
Attachment #8601206 - Flags: approval-mozilla-esr38?
Attachment #8601206 - Flags: approval-mozilla-esr38+
Attachment #8601206 - Flags: approval-mozilla-beta?
Attachment #8601206 - Attachment is obsolete: true
Attachment #8620227 - Flags: review+
You need to log in before you can comment on or make changes to this bug.