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

Issue 611698 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 601850



Sign in to add a comment

Experiment flag WebRTC-EnableWebRtcEcdsa relies on rtc::KT_DEFAULT being KT_RSA

Project Member Reported by hbos@chromium.org, May 13 2016

Issue description

This flag controls if ECDSA or RSA certificates are to be used by default (if no other certificate is specified by the application) when constructing a peer connection.

https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/media/rtc_peer_connection_handler.cc&sq=package:chromium&type=cs&l=958&rcl=1463094995

Current code does "if flag, generate ECDSA" meaning if flag is false chromium does nothing and webrtc will asynchronously generate a KT_DEFAULT certificate for us. To override the default a certificate has to be geneated synchronously before the peer connection is created. (Alternatively we need to be able to support creating rtc::RTCCertificate objects before they have finished being generated which would likely be complicated with the current code.)

With the WebRTC-EnableWebRtcEcdsa experiment it should do ECDSA if true and RSA if false. Now RSA is only used as long as KT_DEFAULT=KT_RSA. We want KT_DEFAULT=KT_ECDSA. Problems with addressing this:
- Relying on webrtc crypto code (SSLIdentity::Generate or rtc::RTCCertificateGenerator::GenerateCertificate) for RSA-1024 when flag is false is slower than using content::PeerConnectionIdentityStore.
- Waiting for PeerConnectionIdentityStore to complete on the main thread causes a deadlock (see CL: https://codereview.chromium.org/1972853003/).

Currently the plan is to enforce KT_DEFAULT=KT_RSA on chromium builds until the ECDSA is the default, the experiment is over, and this flag is removed. Other solutions are possible.
 

Comment 1 by hbos@chromium.org, May 13 2016

Blockedon: 601850
Project Member

Comment 2 by bugdroid1@chromium.org, May 16 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/8ae8ab4e2364821eb9b4c7cf4d9718e78f79432f

commit 8ae8ab4e2364821eb9b4c7cf4d9718e78f79432f
Author: hbos <hbos@webrtc.org>
Date: Mon May 16 09:45:40 2016

Makes ECDSA the default certificate to use (generated if no other certificates
are specified when constructing a peer connection, at:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc/api/webrtcsessiondescriptionfactory.cc&q=webrtcsessiondescriptionfactory&sq=package:chromium&type=cs&l=191).

This does not affect WEBRTC_BUILD_CHROMIUM builds whose ECDSA launch is handled
separately: https://crbug.com/601850 (req. @chromium acc).

BUG= chromium:611698 ,  webrtc:5795 

Review-Url: https://codereview.webrtc.org/1972383002
Cr-Commit-Position: refs/heads/master@{#12756}

[modify] https://crrev.com/8ae8ab4e2364821eb9b4c7cf4d9718e78f79432f/webrtc/base/sslidentity.h

Project Member

Comment 3 by bugdroid1@chromium.org, May 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/d98f6e000a871942dbb143dafe7a0d165eb24e21

commit d98f6e000a871942dbb143dafe7a0d165eb24e21
Author: Henrik Boström <hbos@webrtc.org>
Date: Tue May 17 10:36:11 2016

Fixed typo. KT_DEFAULT different based on WEBRTC_CHROMIUM_BUILD
instead of WEBRTC_BUILD_CHROMIUM.

BUG= chromium:611698 ,  webrtc:5795 
R=perkj@webrtc.org

Review URL: https://codereview.webrtc.org/1986053005 .

Cr-Commit-Position: refs/heads/master@{#12771}

[modify] https://crrev.com/d98f6e000a871942dbb143dafe7a0d165eb24e21/webrtc/base/sslidentity.h

Project Member

Comment 4 by bugdroid1@chromium.org, May 17 2016

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

commit cf97b45f5d1af1f0aa3dd6e76150c5c778c691d1
Author: hbos <hbos@chromium.org>
Date: Tue May 17 15:02:43 2016

WebRTC: Launch ECDSA as default certificate.

Launch bug: https://bugs.chromium.org/p/chromium/issues/detail?id=601850

content_features.cc:
Flip the default value of WebRTC-EnableWebRtcEcdsa to enabled.

rtc_peer_connection_handler.cc:
Move "if flag" code into a helper function. DCHECK that KT_DEFAULT == KT_RSA to make the experiment's assumption about this explicit.

BUG=601850,  611698 
NOCOMMIT=True

Review-Url: https://codereview.chromium.org/1979553002
Cr-Commit-Position: refs/heads/master@{#394128}

[modify] https://crrev.com/cf97b45f5d1af1f0aa3dd6e76150c5c778c691d1/content/public/common/content_features.cc
[modify] https://crrev.com/cf97b45f5d1af1f0aa3dd6e76150c5c778c691d1/content/renderer/media/rtc_peer_connection_handler.cc

Project Member

Comment 5 by bugdroid1@chromium.org, May 26 2016

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

commit bd0f07eb0c705b800aa424ef9feef11b7181aa7e
Author: hbos <hbos@chromium.org>
Date: Thu May 26 10:55:47 2016

ECDSA default certificate in WebRTC: Remove experiment flag kWebRtcEcdsaDefault.

The experiment behind this flag ends in M52. The flag has already been defaulted
to true. This removes the flag and makes ECDSA the default certificate
unconditionally.

BUG=601850,  611698 

Review-Url: https://codereview.chromium.org/2011443002
Cr-Commit-Position: refs/heads/master@{#396156}

[modify] https://crrev.com/bd0f07eb0c705b800aa424ef9feef11b7181aa7e/chrome/app/generated_resources.grd
[modify] https://crrev.com/bd0f07eb0c705b800aa424ef9feef11b7181aa7e/chrome/browser/about_flags.cc
[modify] https://crrev.com/bd0f07eb0c705b800aa424ef9feef11b7181aa7e/content/public/common/content_features.cc
[modify] https://crrev.com/bd0f07eb0c705b800aa424ef9feef11b7181aa7e/content/renderer/media/rtc_peer_connection_handler.cc

Project Member

Comment 6 by bugdroid1@chromium.org, May 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/41c5cd02fc7eb9601edb317b21b3d07bdaafbc37

commit 41c5cd02fc7eb9601edb317b21b3d07bdaafbc37
Author: hbos <hbos@webrtc.org>
Date: Thu May 26 22:23:16 2016

Making ECDSA the default certificate regardless of WEBRTC_CHROMIUM_BUILD.

Due to the experiment in chromium relying on KT_DEFAULT = KT_RSA (bug:
 crbug.com/611698 ) a conditional was introduced. Now that the experiment is
ending and the experiment flag has been removed we can make KT_DEFAULT=KT_ECDSA
unconditionally.

BUG= chromium:611698 

Review-Url: https://codereview.webrtc.org/2009533003
Cr-Commit-Position: refs/heads/master@{#12935}

[modify] https://crrev.com/41c5cd02fc7eb9601edb317b21b3d07bdaafbc37/webrtc/base/sslidentity.h

Project Member

Comment 7 by bugdroid1@chromium.org, May 27 2016

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

commit 2c8d51533a9a774650bc2f728cee2c9e97fb70a4
Author: hbos <hbos@chromium.org>
Date: Fri May 27 14:13:47 2016

Remove rtc_peer_connection_handler.cc's OverrideDefaultCertificate
which was used to override the default DTLS certificate to use in WebRTC to
ECDSA. This is no longer needed since KT_DEFAULT == KT_ECDSA, meaning ECDSA
is already the default certificate and calling this function does nothing.

BUG=601850,  611698 

Review-Url: https://codereview.chromium.org/2016323002
Cr-Commit-Position: refs/heads/master@{#396460}

[modify] https://crrev.com/2c8d51533a9a774650bc2f728cee2c9e97fb70a4/content/renderer/media/rtc_peer_connection_handler.cc
[modify] https://crrev.com/2c8d51533a9a774650bc2f728cee2c9e97fb70a4/content/renderer/media/webrtc/peer_connection_dependency_factory.cc
[modify] https://crrev.com/2c8d51533a9a774650bc2f728cee2c9e97fb70a4/content/renderer/media/webrtc/peer_connection_dependency_factory.h

Comment 8 by hbos@chromium.org, May 27 2016

Status: Verkf (was: Started)

Comment 9 by hbos@chromium.org, May 27 2016

Status: Verified (was: Verkf)

Sign in to add a comment