New issue
Advanced search Search tips

Issue 698313 link

Starred by 4 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

RTCCertificate: Add missing attribute and method

Reported by acmesqua...@gmail.com, Mar 3 2017

Issue description

UserAgent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:51.0) Gecko/20100101 Firefox/51.0

Steps to reproduce the problem:
RTCCertificate doesn't match spec.

What is the expected behavior?
interface RTCCertificate {
    readonly attribute DOMTimeStamp                    expires;
    readonly attribute FrozenArray<RTCDtlsFingerprint> fingerprints;
    AlgorithmIdentifier getAlgorithm();
};

dictionary RTCDtlsFingerprint {
    DOMString algorithm;
    DOMString value;
};

https://www.w3.org/TR/webrtc/#rtccertificate-interface

What went wrong?
Not implemented

Did this work before? No 

Does this work in other browsers? Yes

Chrome version: 58.0.3026.3 dev (64-bit)  Channel: dev
OS Version: 
Flash Version:

 

Comment 1 by ajha@chromium.org, Mar 6 2017

Labels: Needs-Triage-M58
Labels: TE-NeedsTriageHelp
Owner: hbos@chromium.org
Status: Assigned (was: Unconfirmed)
Cc: ligim...@chromium.org
Labels: -TE-NeedsTriageHelp ReleaseBlock-Stable M-59
M58 stable is approaching soon, can we have a fix before M59 hits stable.

Comment 5 by hbos@chromium.org, Apr 11 2017

Status: Started (was: Assigned)
I'll see what I can do.

Comment 6 by hbos@chromium.org, Apr 11 2017

Works in other browsers? I don't see it on Firefox stable and this says others have not implemented anything except Chrome & Firefox having expires:
https://developer.microsoft.com/en-us/microsoft-edge/platform/catalog/?page=1&q=RTCCertificate

Comment 7 by hbos@chromium.org, Apr 11 2017

Looks trivial to implement fingerprints, we have this information available in stats already.

AlgorithmIdentifier could be stored on generateCertificate and its string trivially serialized (e.g. for IndexedDB save/load).

If a certificate is generated by RTCPeerConnection I'm not sure if an AlgorithmIdentifier should be deduced or if null should be provided since the webrtc implementation generated the certificate without necessarily calling generateCertificate. We have not implemented RTCPeerConnection.getConfiguration() though so there is no way to get ahold of RTCCertificate objects by other means than to call generateCertificate. Thus AlgorithmIdentifier is currently always knowable. Filed spec bug: https://github.com/w3c/webrtc-pc/issues/1120

RTCDtlsTransport.getRemoteCertificates() returns ArrayBuffers, not RTCCertificates, so we don't have to worry about how to get ahold of AlgorithmIdentifiers for remote certificates, though I would expect to return null if that case was possible since we don't know what the remote webrtc implementation is doing for certificate generation.

Comment 8 by hbos@chromium.org, Apr 11 2017

Correction: AlgorithmIdentifier is not necessarily a string, it's a DictionaryOrString, serialization would have to store dictionaries or strings.

Comment 9 by hbos@chromium.org, Apr 11 2017

WRT serialization and AlgorithmIdentifier, filed: https://github.com/w3c/webrtc-pc/issues/1121

Comment 10 by hbos@chromium.org, Apr 12 2017

Labels: -M-59 M-60
This will need an intent to ship.

Comment 11 by hbos@chromium.org, May 5 2017

RTCCertificate was decided should only apply for certificates that was generated with RTCPeerConnection.generateCertificate(). RTCCertificate.getAlgorithm() was removed due to not being very useful when only applicable to locally generated certificates; if you care about the algorithm identifier you can easily keep track/store it yourself.

RTCCertificate.fingerprints has turned into getFingerprints(). Implementation: https://codereview.chromium.org/2828563002/

Intent to Ship: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/h8qqS5sI-ig
hbos@,
Friendly ping to get an update on this issue as it is marked as RBS.
Thank you..!!

Comment 13 by hbos@chromium.org, May 23 2017

Labels: -ReleaseBlock-Stable
Oh no, I forgot about this issue. No activity on the intent for a long time. It would be a shame not to get this in before the cut considering it has no new security implications and the CL has been ready to land for a long time (web platform tests can be added later).

In either case, I don't see this as a blocker for release. Thread bumped.
Project Member

Comment 14 by bugdroid1@chromium.org, Jun 8 2017

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

commit 149d40a4e451675bbed7a093e56d40fb11f73196
Author: hbos <hbos@chromium.org>
Date: Thu Jun 08 14:40:35 2017

RTCCertificate.getFingerprints added (exposed to the web).

Spec: https://rawgit.com/w3c/webrtc-pc/master/webrtc.html#dom-rtccertificate-getfingerprints

Intent to Implement & Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/h8qqS5sI-ig

BUG=698313, 609108

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

[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/chrome/browser/media/webrtc/webrtc_browsertest.cc
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/chrome/browser/media/webrtc/webrtc_browsertest_base.cc
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/chrome/browser/media/webrtc/webrtc_browsertest_base.h
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/chrome/test/data/webrtc/indexeddb.js
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/chrome/test/data/webrtc/peerconnection.js
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/content/renderer/media/rtc_certificate.cc
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/content/renderer/media/rtc_certificate.h
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-generateCertificate-expected.txt
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-generateCertificate.html
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/third_party/WebKit/LayoutTests/platform/mac/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/third_party/WebKit/Source/modules/modules_idl_files.gni
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/third_party/WebKit/Source/modules/peerconnection/RTCCertificate.cpp
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/third_party/WebKit/Source/modules/peerconnection/RTCCertificate.h
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/third_party/WebKit/Source/modules/peerconnection/RTCCertificate.idl
[add] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/third_party/WebKit/Source/modules/peerconnection/RTCDtlsFingerprint.idl
[modify] https://crrev.com/149d40a4e451675bbed7a093e56d40fb11f73196/third_party/WebKit/public/platform/WebRTCCertificate.h

Cc: hbos@chromium.org
Labels: -Pri-2 -M-60 Pri-3
Owner: ----
Status: Available (was: Started)

Sign in to add a comment