Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(34)

Issue 2704623002: Add non-application-data SSL messages to NetLog. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months, 1 week ago by davidben
Modified:
6 months, 1 week ago
Reviewers:
svaldez, eroman
CC:
chromium-reviews, net-reviews_chromium.org, cbentzel+watch_chromium.org, bnc+watch_chromium.org, mmenke, eroman
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add non-application-data SSL messages to NetLog. Right now we usually ask the reporter to give us a full byte-level log, but this unnecessarily leaks a lot of information. In TLS 1.2, these messages are all (save the Finished, which is not interesting) sent in the clear anyway. This also doesn't work for renegotiation which we've usually been blind for. In TLS 1.3, these messages are now encrypted, which means even our prior strategy wouldn't work. It's also somewhat silly to ask users for full byte logging when we're only interested in protocol-level information. Per the bug, Certificate messages we send (client certs) are filtered out unless full byte logging is enabled. BUG=676353 Review-Url: https://codereview.chromium.org/2704623002 Cr-Commit-Position: refs/heads/master@{#465603} Committed: https://chromium.googlesource.com/chromium/src/+/cef9e211832bf2d1f01f750296238fed28e416f0

Patch Set 1 #

Patch Set 2 : . #

Total comments: 4

Patch Set 3 : eroman comments #

Patch Set 4 : tweak comment #

Messages

Total messages: 31 (19 generated)
davidben
eroman: What do you think? Too noisy?
8 months, 1 week ago (2017-02-17 21:04:42 UTC) #8
eroman
Can you include a sample log file generated with this data? I haven't reviewed the ...
8 months, 1 week ago (2017-02-17 21:30:18 UTC) #9
davidben
On 2017/02/17 21:30:18, eroman wrote: > Can you include a sample log file generated with ...
8 months, 1 week ago (2017-02-17 21:40:09 UTC) #10
davidben
Also +svaldez to get his views.
8 months, 1 week ago (2017-02-17 21:40:37 UTC) #12
svaldez
On 2017/02/17 21:40:37, davidben wrote: > Also +svaldez to get his views. Any chance type ...
8 months, 1 week ago (2017-02-17 22:06:15 UTC) #13
davidben
On 2017/02/17 22:06:15, svaldez wrote: > On 2017/02/17 21:40:37, davidben wrote: > > Also +svaldez ...
8 months, 1 week ago (2017-02-17 22:16:32 UTC) #14
eroman
Can you tell me more about what data will be in these blobs? In particular ...
8 months, 1 week ago (2017-02-17 22:16:51 UTC) #15
davidben
On 2017/02/17 22:16:51, eroman wrote: > Can you tell me more about what data will ...
6 months, 1 week ago (2017-04-18 20:51:17 UTC) #22
davidben
On 2017/04/18 20:51:17, davidben wrote: > On 2017/02/17 22:16:51, eroman wrote: > > Can you ...
6 months, 1 week ago (2017-04-18 20:53:43 UTC) #23
eroman
lgtm > Or do we get to ignore it now? The UI is deprecated but ...
6 months, 1 week ago (2017-04-18 21:07:23 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2704623002/60001
6 months, 1 week ago (2017-04-19 14:56:11 UTC) #28
commit-bot: I haz the power
6 months, 1 week ago (2017-04-19 15:00:35 UTC) #31
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/cef9e211832bf2d1f01f75029623...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa