New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 835958 link

Starred by 14 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Performing a subsequent offer/answer causes SRTCP (and possibly SRTP) decryption errors

Project Member Reported by deadbeef@chromium.org, Apr 23 2018

Issue description

Chrome Version: 67.0.3396.10

What steps will reproduce the problem?
(1) Create PeerConnection between Chrome 67 and an older version of Chrome, or another browser.
(2) On the Chrome 67 side, perform another offer/answer. Can reproduce simply by setting the previous local/remote descriptions with no modification.

What is the expected result?

No changes should occur.

What happens instead?

SRTCP decryption errors appear in the log, like:

[1:25:0423/124314.210852:ERROR:srtptransport.cc(249)] Failed to unprotect RTCP packet: size=70, type=200

Please use labels and text to provide additional information.

A regression in M67 caused us to reset the SRTP context any time an SDP answer is set, resetting the SRTCP index and SRTP rollover counter. This means that the non-M67 endpoint will fail to decrypt SRTCP packets with a replay detection error, until the index sent by the M67 side reaches the value it was before. For example: If a call is going for around 5 minutes, then a new offer/answer is set, SRTCP decryption errors will probably occur for around 5 minutes before they recover.

It's still possible to receive media without RTCP working. But it's vital for congestion control and loss recovery and other things, so this is a severe regression.

It's also possible that if a 16-bit RTP sequence counter has rolled over, SRTP packets will start failing to be decrypted as well due to the reset rollover counter, meaning media would stop flowing. This is obviously more likely to occur for a long-running call, but if it does occur, this bug ends up having a much more noticeable impact.

You can reproduce the SRTCP decryption errors in just one browser using this fiddle (it only does a new offer/answer for one PeerConnection, meaning one side resets its SRTP context while the other doesn't): https://jsfiddle.net/0pnn9qe1/

Just look for the "Failed to unprotect RTCP packet" error message in the log to confirm that the issue is occurring.

See this corresponding bug in the webrtc bug repo: https://bugs.chromium.org/p/webrtc/issues/detail?id=8996

The issue has already been fixed and verified in Canary. I'm creating this bug to request a merge to M67, as it severely impacts WebRTC functionality (especially for long-lasting connections, as described above).
 
Project Member

Comment 1 by sheriffbot@chromium.org, Apr 24 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 2 by gov...@chromium.org, Apr 24 2018

Pls merge your change to M67 branch 3396 ASAP so we can pick it up for next M67 Beta release. Thank you.
Labels: -Merge-Approved-67 merge-merged-67
Merged: https://webrtc-review.googlesource.com/c/src/+/71881
Cc: deadbeef@chromium.org jansson@chromium.org chfremer@chromium.org emir...@chromium.org rantonysamy@chromium.org
 Issue 751833  has been merged into this issue.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 11 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/bae103126c5bdaf1361bcff4750eb5ebe10020ee

commit bae103126c5bdaf1361bcff4750eb5ebe10020ee
Author: Zhi Huang <zhihuang@webrtc.org>
Date: Mon Jun 11 23:06:26 2018

Add a flag to actively reset the SRTP parameters

Add a new flag to RtcConfiguration. By setting that flag to true, the
SRTP parameters will be reset whenever the DTLS transports are reset
after every offer/answer negotiation.

This should only be used as a workaround for the linked bug, if the
application knows that the other party is affected (for instance,
using a version number).

Bug:  chromium:835958 
Change-Id: Ifb4b99f68dc272507728ab59c07627f0d1b9c605
Reviewed-on: https://webrtc-review.googlesource.com/81642
Commit-Queue: Zhi Huang <zhihuang@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23570}
[modify] https://crrev.com/bae103126c5bdaf1361bcff4750eb5ebe10020ee/api/peerconnectioninterface.h
[modify] https://crrev.com/bae103126c5bdaf1361bcff4750eb5ebe10020ee/pc/dtlssrtptransport.cc
[modify] https://crrev.com/bae103126c5bdaf1361bcff4750eb5ebe10020ee/pc/dtlssrtptransport.h
[modify] https://crrev.com/bae103126c5bdaf1361bcff4750eb5ebe10020ee/pc/dtlssrtptransport_unittest.cc
[modify] https://crrev.com/bae103126c5bdaf1361bcff4750eb5ebe10020ee/pc/jseptransportcontroller.cc
[modify] https://crrev.com/bae103126c5bdaf1361bcff4750eb5ebe10020ee/pc/jseptransportcontroller.h
[modify] https://crrev.com/bae103126c5bdaf1361bcff4750eb5ebe10020ee/pc/peerconnection.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 12 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/6c789e08d522de2d76b6a3c6fb58b10e79aee403

commit 6c789e08d522de2d76b6a3c6fb58b10e79aee403
Author: Zhi Huang <zhihuang@webrtc.org>
Date: Tue Jun 12 00:56:07 2018

Revert "Add a flag to actively reset the SRTP parameters"

This reverts commit bae103126c5bdaf1361bcff4750eb5ebe10020ee.

Reason for revert: Merge native code change with Android and Objc wrapper.

Original change's description:
> Add a flag to actively reset the SRTP parameters
> 
> Add a new flag to RtcConfiguration. By setting that flag to true, the
> SRTP parameters will be reset whenever the DTLS transports are reset
> after every offer/answer negotiation.
> 
> This should only be used as a workaround for the linked bug, if the
> application knows that the other party is affected (for instance,
> using a version number).
> 
> Bug:  chromium:835958 
> Change-Id: Ifb4b99f68dc272507728ab59c07627f0d1b9c605
> Reviewed-on: https://webrtc-review.googlesource.com/81642
> Commit-Queue: Zhi Huang <zhihuang@webrtc.org>
> Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#23570}

TBR=deadbeef@webrtc.org,zhihuang@webrtc.org

Change-Id: Ibd7a3b8f45ff8df4af33d758f8fd3e2d5158e8e2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:835958 
Reviewed-on: https://webrtc-review.googlesource.com/83080
Reviewed-by: Zhi Huang <zhihuang@webrtc.org>
Commit-Queue: Zhi Huang <zhihuang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23571}
[modify] https://crrev.com/6c789e08d522de2d76b6a3c6fb58b10e79aee403/api/peerconnectioninterface.h
[modify] https://crrev.com/6c789e08d522de2d76b6a3c6fb58b10e79aee403/pc/dtlssrtptransport.cc
[modify] https://crrev.com/6c789e08d522de2d76b6a3c6fb58b10e79aee403/pc/dtlssrtptransport.h
[modify] https://crrev.com/6c789e08d522de2d76b6a3c6fb58b10e79aee403/pc/dtlssrtptransport_unittest.cc
[modify] https://crrev.com/6c789e08d522de2d76b6a3c6fb58b10e79aee403/pc/jseptransportcontroller.cc
[modify] https://crrev.com/6c789e08d522de2d76b6a3c6fb58b10e79aee403/pc/jseptransportcontroller.h
[modify] https://crrev.com/6c789e08d522de2d76b6a3c6fb58b10e79aee403/pc/peerconnection.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b82211f711ad605b06c8339f48067d2571ccb95

commit 3b82211f711ad605b06c8339f48067d2571ccb95
Author: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Jun 12 06:32:23 2018

Roll src/third_party/webrtc a46bd4b9c745..6c789e08d522 (2 commits)

https://webrtc.googlesource.com/src.git/+log/a46bd4b9c745..6c789e08d522


git log a46bd4b9c745..6c789e08d522 --date=short --no-merges --format='%ad %ae %s'
2018-06-12 zhihuang@webrtc.org Revert "Add a flag to actively reset the SRTP parameters"
2018-06-11 zhihuang@webrtc.org Add a flag to actively reset the SRTP parameters


Created with:
  gclient setdep -r src/third_party/webrtc@6c789e08d522

The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng

BUG= chromium:835958 , chromium:835958 
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Idfa6fe9e019ff3637355b3ba9f1bf862f31ced1d
Reviewed-on: https://chromium-review.googlesource.com/1096635
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#566340}
[modify] https://crrev.com/3b82211f711ad605b06c8339f48067d2571ccb95/DEPS

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 12 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/b57e169f3c63204af8929835bd9e708726b49296

commit b57e169f3c63204af8929835bd9e708726b49296
Author: Zhi Huang <zhihuang@webrtc.org>
Date: Tue Jun 12 20:32:00 2018

Add a flag to actively reset the SRTP parameters

Add a new flag to RtcConfiguration. By setting that flag to true, the
SRTP parameters will be reset whenever the DTLS transports are reset
after every offer/answer negotiation.

The flag is added to Android and Objc wrapper as well.

This should only be used as a workaround for the linked bug, if the
application knows that the other party is affected (for instance,
using a version number).

TBR=sakal@webrtc.org, denicija@webrtc.org

Bug:  chromium:835958 
Change-Id: I6db025e1c69bf83e1b1908f7df4627430db9920c
Reviewed-on: https://webrtc-review.googlesource.com/83101
Commit-Queue: Zhi Huang <zhihuang@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23587}
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/api/peerconnectioninterface.h
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/pc/dtlssrtptransport.cc
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/pc/dtlssrtptransport.h
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/pc/dtlssrtptransport_unittest.cc
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/pc/jseptransport.cc
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/pc/jseptransport.h
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/pc/jseptransportcontroller.cc
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/pc/jseptransportcontroller.h
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/pc/peerconnection.cc
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/sdk/android/api/org/webrtc/PeerConnection.java
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/sdk/android/src/jni/pc/peerconnection.cc
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/sdk/objc/Framework/Classes/PeerConnection/RTCConfiguration.mm
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/sdk/objc/Framework/Headers/WebRTC/RTCConfiguration.h
[modify] https://crrev.com/b57e169f3c63204af8929835bd9e708726b49296/sdk/objc/Framework/UnitTests/RTCPeerConnectionTest.mm

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/852af1c95f971f154463d6c243332395c904b88d

commit 852af1c95f971f154463d6c243332395c904b88d
Author: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Jun 13 06:10:20 2018

Roll src/third_party/webrtc 9614a313b836..241d0c16c0d3 (18 commits)

https://webrtc.googlesource.com/src.git/+log/9614a313b836..241d0c16c0d3


git log 9614a313b836..241d0c16c0d3 --date=short --no-merges --format='%ad %ae %s'
2018-06-13 qingsi@google.com Remove ContinualGatheringPolicy::GATHER_CONTINUALLY_AND_RECOVER.
2018-06-13 shampson@webrtc.org Updated PeerConnection integration test to fix race condition.
2018-06-13 buildbot@webrtc.org Roll chromium_revision ef538e3112..9df92afb16 (566490:566630)
2018-06-12 qingsi@google.com Refactor the regathering of candidates in P2PTransportChannel.
2018-06-12 zhihuang@webrtc.org Add a flag to actively reset the SRTP parameters
2018-06-12 danilchap@webrtc.org Reland "Use absl::optional instead or rtc::Optional"
2018-06-12 buildbot@webrtc.org Roll chromium_revision ffaf1e2ba6..ef538e3112 (565764:566490)
2018-06-12 orphis@webrtc.org Add HeaderExtensions to RtpParameters
2018-06-12 alexnarest@webrtc.org Enable send side audio TWCC only if WebRTC-Audio-ForceNoTWCC is not enabled.
2018-06-12 mbonadei@webrtc.org Explicitly setting use_lld=false on MSVC bots.
2018-06-12 peterhanspers@webrtc.org Metal renderer does not handle i420 frames correctly.
2018-06-12 andersc@webrtc.org Make rtc_software_fallback_wrappers target visible.
2018-06-12 phensman@webrtc.org Release ADM after passing it to PCF in AppRTC
2018-06-12 kwiberg@webrtc.org LogMessage::UpdateMinLogSeverity: Don't ignore all but the last stream
2018-06-12 philipel@webrtc.org Don't update internal state of the FrameBuffer2 when an undecodable frame is inserted.
2018-06-12 jonasolsson@webrtc.org Remove stringstreams from p2p/
2018-06-12 jonasolsson@webrtc.org remove unused UNSHIPPED trace macros
2018-06-12 sakal@webrtc.org Delay the creation of the platform thread in TestAudioDeviceModule.


Created with:
  gclient setdep -r src/third_party/webrtc@241d0c16c0d3

The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng

BUG=chromium:None,chromium:None,chromium:None,chromium:None,chromium:835958,chromium:None,chromium:None,chromium:844313,chromium:b/79961243
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: I7b4bcb1330e3b4206e7a0d20aa3bb776e04949b2
Reviewed-on: https://chromium-review.googlesource.com/1098237
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#566734}
[modify] https://crrev.com/852af1c95f971f154463d6c243332395c904b88d/DEPS

Sign in to add a comment