Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(451)

Issue 2156763003: Extend the webRequest.onCompleted event details object with TLS/SSL information

Created:
4 years, 5 months ago by rolandshoemaker
Modified:
3 years, 8 months ago
CC:
chromium-apps-reviews_chromium.org, chromium-reviews, estark, extensions-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Extend the webRequest.onCompleted event details object with TLS/SSL information If a request has a valid ssl_info field a new object, sslInfo, is added to the details object passed back to the onCompleted event listener. This object contains information about the underlying transport that was established including both the sent and built certificate chains. If a response is loaded from the cache it will not contain the sent chain since it is not stored along with the response (as far as I can tell). I'm slightly concerned that adding the raw bodies of the sent and built certificate chains may have a negative performance impact since they are relatively large byte buffers but this is a required feature for HTTPS Everywhere. For a while I toyed with the idea of adding a JS method to the returned sslInfo object that would return the raw bodies but looking at the existing code I couldn't figure out how this would be done (or if it is even possible). Variable naming in the sslInfo object generally reflects the names used internally for the corresponding variable but there are some probably a few places where better names could be chosen for interoperability. BUG=628819

Patch Set 1 #

Total comments: 2

Patch Set 2 : Remove questionably useful fields & add feature switch #

Total comments: 26

Patch Set 3 : Rebase cleanup #

Patch Set 4 : Trim more fields and use composed cipher suite name #

Total comments: 1

Patch Set 5 : Use BoringSSL's SSL_CIPHER_get_rfc_name #

Total comments: 8

Patch Set 6 : Consistently use key constants for dict fields and simplify validation error reporting #

Total comments: 18
Unified diffs Side-by-side diffs Delta from patch set Stats (+379 lines, -1 line) Patch
M AUTHORS View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A extensions/browser/api/web_request/DEPS View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M extensions/browser/api/web_request/web_request_api.cc View 1 2 2 chunks +4 lines, -0 lines 1 comment Download
M extensions/browser/api/web_request/web_request_api_constants.h View 1 2 3 4 5 1 chunk +27 lines, -0 lines 0 comments Download
M extensions/browser/api/web_request/web_request_api_constants.cc View 1 2 3 4 5 1 chunk +26 lines, -0 lines 1 comment Download
M extensions/browser/api/web_request/web_request_api_helpers.h View 1 2 3 4 5 3 chunks +8 lines, -0 lines 4 comments Download
M extensions/browser/api/web_request/web_request_api_helpers.cc View 1 2 3 4 5 3 chunks +89 lines, -0 lines 8 comments Download
M extensions/browser/api/web_request/web_request_event_details.h View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
M extensions/browser/api/web_request/web_request_event_details.cc View 1 2 3 4 5 2 chunks +48 lines, -0 lines 4 comments Download
M extensions/common/api/web_request.json View 1 2 3 4 5 2 chunks +157 lines, -1 line 0 comments Download
M extensions/common/feature_switch.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M extensions/common/feature_switch.cc View 1 2 3 chunks +6 lines, -0 lines 0 comments Download
M extensions/common/switches.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M extensions/common/switches.cc View 1 2 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 34 (5 generated)
estark
Thanks for doing this! Based on https://dev.chromium.org/developers/design-documents/extensions/proposed-changes/apis-under-development I'd suggest emailing apps-dev@chromium.org to get approval for ...
4 years, 5 months ago (2016-07-18 23:05:31 UTC) #1
rolandshoemaker
On 2016/07/18 23:05:31, estark wrote: > Thanks for doing this! Based on > https://dev.chromium.org/developers/design-documents/extensions/proposed-changes/apis-under-development > ...
4 years, 5 months ago (2016-07-25 04:09:31 UTC) #2
rolandshoemaker
https://codereview.chromium.org/2156763003/diff/1/extensions/browser/api/web_request/web_request_api_constants.h File extensions/browser/api/web_request/web_request_api_constants.h (right): https://codereview.chromium.org/2156763003/diff/1/extensions/browser/api/web_request/web_request_api_constants.h#newcode54 extensions/browser/api/web_request/web_request_api_constants.h:54: extern const char kSSLInfoKey[]; On 2016/07/18 23:05:31, estark wrote: ...
4 years, 5 months ago (2016-07-25 19:08:47 UTC) #3
palmer
meacer or rdevlin.cronin, are you good people to review this?
4 years, 4 months ago (2016-08-04 23:56:13 UTC) #5
Devlin
On 2016/08/04 23:56:13, palmer (OOO thru August)) wrote: > meacer or rdevlin.cronin, are you good ...
4 years, 4 months ago (2016-08-09 23:39:35 UTC) #6
Devlin
On 2016/08/09 23:39:35, Devlin wrote: > On 2016/08/04 23:56:13, palmer (OOO thru August)) wrote: > ...
4 years, 4 months ago (2016-08-09 23:46:44 UTC) #7
elawrence
This looks super-cool to me. I believe this would also enable one of the extensions ...
4 years, 4 months ago (2016-08-10 14:46:48 UTC) #8
davidben
https://codereview.chromium.org/2156763003/diff/20001/extensions/common/api/web_request.json File extensions/common/api/web_request.json (right): https://codereview.chromium.org/2156763003/diff/20001/extensions/common/api/web_request.json#newcode157 extensions/common/api/web_request.json:157: "enum": ["UNKNOWN", "SSL 2.0", "SSL 3.0", "TLS 1.0", "TLS ...
4 years, 4 months ago (2016-08-10 19:42:14 UTC) #10
rolandshoemaker
https://codereview.chromium.org/2156763003/diff/20001/extensions/common/api/web_request.json File extensions/common/api/web_request.json (right): https://codereview.chromium.org/2156763003/diff/20001/extensions/common/api/web_request.json#newcode157 extensions/common/api/web_request.json:157: "enum": ["UNKNOWN", "SSL 2.0", "SSL 3.0", "TLS 1.0", "TLS ...
4 years, 4 months ago (2016-08-15 03:38:33 UTC) #11
davidben
https://codereview.chromium.org/2156763003/diff/20001/extensions/common/api/web_request.json File extensions/common/api/web_request.json (right): https://codereview.chromium.org/2156763003/diff/20001/extensions/common/api/web_request.json#newcode167 extensions/common/api/web_request.json:167: "enum": ["UNKNOWN", "NULL", "RSA", "RSA_EXPORT", "DH_DSS_EXPORT", "DH_DSS", "DH_RSA_EXPORT", "DH_RSA", ...
4 years, 4 months ago (2016-08-15 19:09:34 UTC) #12
alex.gaynor
https://codereview.chromium.org/2156763003/diff/20001/extensions/common/api/web_request.json File extensions/common/api/web_request.json (right): https://codereview.chromium.org/2156763003/diff/20001/extensions/common/api/web_request.json#newcode167 extensions/common/api/web_request.json:167: "enum": ["UNKNOWN", "NULL", "RSA", "RSA_EXPORT", "DH_DSS_EXPORT", "DH_DSS", "DH_RSA_EXPORT", "DH_RSA", ...
4 years, 4 months ago (2016-08-19 15:53:36 UTC) #14
rolandshoemaker
On 2016/08/19 15:53:36, alex.gaynor wrote: > https://codereview.chromium.org/2156763003/diff/20001/extensions/common/api/web_request.json > File extensions/common/api/web_request.json (right): > > https://codereview.chromium.org/2156763003/diff/20001/extensions/common/api/web_request.json#newcode167 > ...
4 years, 3 months ago (2016-08-29 20:28:44 UTC) #15
palmer
Friendly ping. I'm back and available to review, if that helps. :)
4 years, 1 month ago (2016-11-03 18:13:39 UTC) #16
rolandshoemaker
Sorry for the three month pause, I've updated and paired back some of the excess ...
4 years ago (2016-12-07 06:19:29 UTC) #17
davidben
https://codereview.chromium.org/2156763003/diff/60001/net/ssl/ssl_cipher_suite_names.cc File net/ssl/ssl_cipher_suite_names.cc (right): https://codereview.chromium.org/2156763003/diff/60001/net/ssl/ssl_cipher_suite_names.cc#newcode427 net/ssl/ssl_cipher_suite_names.cc:427: } I haven't looked at the rest yet, but ...
4 years ago (2016-12-07 15:31:45 UTC) #18
rolandshoemaker
On 2016/12/07 15:31:45, davidben wrote: > https://codereview.chromium.org/2156763003/diff/60001/net/ssl/ssl_cipher_suite_names.cc > File net/ssl/ssl_cipher_suite_names.cc (right): > > https://codereview.chromium.org/2156763003/diff/60001/net/ssl/ssl_cipher_suite_names.cc#newcode427 > ...
4 years ago (2016-12-07 19:56:39 UTC) #19
palmer
Some comments. Also, update the commit message to remove the "first time" commentary. :) https://codereview.chromium.org/2156763003/diff/80001/extensions/browser/api/web_request/web_request_api_helpers.cc ...
3 years, 11 months ago (2017-01-21 01:16:30 UTC) #20
rolandshoemaker
3 years, 10 months ago (2017-01-31 05:05:14 UTC) #23
palmer
Getting closer! Some nits and comments. The next thing to figure out is who, among ...
3 years, 10 months ago (2017-01-31 05:42:32 UTC) #24
Ryan Sleevi
I'm more uncomfortable with pursuing this than I did in the past, so I'm sorry ...
3 years, 10 months ago (2017-01-31 21:37:57 UTC) #25
rolandshoemaker
(sorry getting back to this has taken so long, we had some internal back and ...
3 years, 8 months ago (2017-03-29 21:24:28 UTC) #26
alex.gaynor
My specific, immediate, use-cases would be addressed by ciphersuite and TLS version information you listed ...
3 years, 8 months ago (2017-03-29 23:15:29 UTC) #27
Ryan Sleevi
Thanks! I think this is super-useful for continued discussion! On 2017/03/29 21:24:28, rolandshoemaker wrote: > ...
3 years, 8 months ago (2017-03-31 15:19:09 UTC) #28
Ryan Sleevi
Oh, and as for how to provide useful next steps: I think the privacy aspect ...
3 years, 8 months ago (2017-03-31 15:20:57 UTC) #29
alex.gaynor
I think there's a relatively simple solution to the TLS version and protocol solutions: they're ...
3 years, 8 months ago (2017-04-01 20:36:13 UTC) #30
Ryan Sleevi
On 2017/04/01 20:36:13, alex.gaynor wrote: > I think there's a relatively simple solution to the ...
3 years, 8 months ago (2017-04-03 13:53:56 UTC) #31
palmer
I'm sorry to have been so absent from this. I got mired in several million ...
3 years, 8 months ago (2017-04-03 23:00:38 UTC) #32
Ryan Sleevi
Throw up a Github, let's small-group iterate it ala incubation, figure out the unknowns, and ...
3 years, 8 months ago (2017-04-03 23:07:44 UTC) #33
rolandshoemaker
3 years, 8 months ago (2017-04-04 00:39:36 UTC) #34
I've thrown up a slightly re-formatted version of my arguments above with key
names and types for the suggest fields. It could still use a little cleanup and
a bit more reasoning about the problem this solves/risks it presents but I
thought I'd send out the link now so that we can get the discussions rolling in
a slightly better forum as soon as possible. See you over on GitHub!

https://github.com/EFForg/webrequest-tlsinfo-api/blob/master/proposal.md

On 2017/04/03 23:07:44, Ryan Sleevi wrote:
> Throw up a Github, let's small-group iterate it ala incubation, figure out
> the unknowns, and try to explicitly reach out to The Right Folks for
> feedback. That'll presumably involve folks from extensions and privacy, but
> may also need to rope in some of the folks doing some of the other (Web
> platform-y) features to see how things like this are being tackled in
> NetInfo.
> 
> After that, we'll circulate it broadly to chromium-dev@ (where we try to
> circulate 'design docs', but this is more a design doc one pager, which is
> just the right level for broader chromium-dev) and begin implementing in
> parallel.
> 
> On Mon, Apr 3, 2017 at 7:00 PM, <mailto:palmer@chromium.org> wrote:
> 
> > I'm sorry to have been so absent from this. I got mired in several million
> > other
> > things...
> >
> > But yeah, the Github + discussion on one of the Chromium mailing lists
> > would be
> > good. Maybe https://groups.google.com/a/chromium.org/forum/#!forum/privacy
> > and/or chromium-dev?
> >
> > https://codereview.chromium.org/2156763003/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Powered by Google App Engine
This is Rietveld 408576698