Log DevTools console message for subresources with Symantec certs |
|||||||||||||
Issue descriptionAs a follow-up to issue 763984 , we should log console message warnings for subresources loaded with Symantec certs. This requires plumbing the necessary information into Blink, unfortunately.
,
Oct 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3694c80b663b36ecf77da4e0ea16c177f1f443f1 commit 3694c80b663b36ecf77da4e0ea16c177f1f443f1 Author: Emily Stark <estark@google.com> Date: Fri Oct 06 05:30:21 2017 Log Symantec console warnings in Blink for main- and sub-resources We previously added DevTools logging, triggered from the browser process, for main resources that were loaded with Symantec certificates that are slated for distrust in Chrome 66/70. This CL attaches the necessary information to resource responses and passes it into Blink and does the logging from there, so that we log on subresources as well as main resources. (We must pass it into Blink because we can't associate a resource load with a particular document/frame in which to do the logging in the browser process.) We want to log on all embedders, but customize the message for Chrome, so once we get to Blink we call back out through ContentRendererClient to allow chrome to override the console message. Bug: 770406 Change-Id: I992294feb763211542fdf2fcde1cbaf84e9fcf61 Reviewed-on: https://chromium-review.googlesource.com/693315 Commit-Queue: Emily Stark <estark@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Cr-Commit-Position: refs/heads/master@{#506984} [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/chrome/browser/chrome_content_browser_client.h [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/chrome/browser/ssl/ssl_browser_tests.cc [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/chrome/renderer/chrome_content_renderer_client.cc [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/chrome/renderer/chrome_content_renderer_client.h [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/browser/loader/resource_loader.cc [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/browser/ssl/ssl_manager.cc [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/browser/ssl/ssl_manager_unittest.cc [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/child/web_url_loader_impl.cc [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/common/resource_messages.h [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/public/browser/content_browser_client.cc [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/public/browser/content_browser_client.h [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/public/common/resource_response.cc [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/public/common/resource_response_info.cc [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/public/common/resource_response_info.h [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/public/renderer/content_renderer_client.cc [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/public/renderer/content_renderer_client.h [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/renderer/render_frame_impl.cc [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/content/renderer/render_frame_impl.h [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/third_party/WebKit/Source/core/exported/LocalFrameClientImpl.cpp [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/third_party/WebKit/Source/core/exported/LocalFrameClientImpl.h [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/third_party/WebKit/Source/core/frame/LocalFrameClient.h [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/third_party/WebKit/Source/core/loader/DocumentLoader.cpp [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/third_party/WebKit/Source/platform/exported/WebURLResponse.cpp [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/third_party/WebKit/Source/platform/loader/fetch/ResourceResponse.cpp [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/third_party/WebKit/Source/platform/loader/fetch/ResourceResponse.h [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/third_party/WebKit/public/platform/WebURLResponse.h [modify] https://crrev.com/3694c80b663b36ecf77da4e0ea16c177f1f443f1/third_party/WebKit/public/web/WebFrameClient.h
,
Oct 6 2017
,
Oct 11 2017
> Note: we're currently implementing this for all subresources, which could be a little much and get really spammy for sites that have tons of Symantec subresources. After some # of messages, we could consider capping it and printing a generic message about "additional resources" instead of printing the URL for each one. More discussion here: https://chromium-review.googlesource.com/c/chromium/src/+/693315/3/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp#515 We're going to need to handle this for m63. The first two sites I opened in my ToT build today were crazily spammy. cnn.com has 106 symantec warnings logged and weather.com has 125. It's pretty rough. :) Can we just log once for each certificate? Or once per origin/domain?
,
Oct 11 2017
and FWIW capping at 10/20 messages is also an option, though I prefer it less than the above solutions.
,
Oct 11 2017
Solutions in comment #4 sound appropriate. We've already started receiving complaints about how often this fills up a console ( https://bugs.chromium.org/p/chromium/issues/detail?id=734088#c10 ), so I'm marking this as release block beta. estark@, have you started on the TODO? Another quick, short-term solution is to make these messages Verbose level in the interim.
,
Oct 12 2017
Planning on capping to 5 messages and limiting to main-frame for M63. Working on the CL now and mkwst should be able to review today in CEST.
,
Oct 12 2017
https://chromium-review.googlesource.com/c/chromium/src/+/715556 is in the CQ and makes the following changes: - logs a message specific to the origin, not the full resource URL, and dedups by hostname to prevent multiple messages for the same origin - caps out after 10 messages with a generic "Additional resources..." message - logs resources in subframes with Verbose level We'll probably want to do something better for M64, like list all the exact resources in the Security panel.
,
Oct 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/80940ee33c0ecc800934aea9ef76a8852f30435a commit 80940ee33c0ecc800934aea9ef76a8852f30435a Author: Emily Stark <estark@google.com> Date: Thu Oct 12 19:46:08 2017 Cap Symantec warning messages to prevent log spam Tweak the Symantec warning message to be about an origin rather than a specific resource URL so that we don't log more than once per hostname. Also cap to 10 messages, and log subframe resources at the Verbose level rather than Warning. TBR=jochen@chromium.org Bug: 770406 Change-Id: I0ecb8bcec0ee2b6d4b83105151654b1091e2299c Reviewed-on: https://chromium-review.googlesource.com/715556 Commit-Queue: Mike West <mkwst@chromium.org> Reviewed-by: Paul Irish <paulirish@chromium.org> Reviewed-by: Emily Stark <estark@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#508386} [modify] https://crrev.com/80940ee33c0ecc800934aea9ef76a8852f30435a/chrome/browser/ssl/ssl_browser_tests.cc [modify] https://crrev.com/80940ee33c0ecc800934aea9ef76a8852f30435a/chrome/renderer/chrome_content_renderer_client.cc [add] https://crrev.com/80940ee33c0ecc800934aea9ef76a8852f30435a/chrome/test/data/ssl/page_with_many_subresources.html [modify] https://crrev.com/80940ee33c0ecc800934aea9ef76a8852f30435a/third_party/WebKit/Source/core/exported/LocalFrameClientImpl.cpp [modify] https://crrev.com/80940ee33c0ecc800934aea9ef76a8852f30435a/third_party/WebKit/Source/core/exported/LocalFrameClientImpl.h [modify] https://crrev.com/80940ee33c0ecc800934aea9ef76a8852f30435a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
,
Oct 13 2017
Removing rb-beta for DevTools, now that capping messages has landed. Thank you estark@!
,
Oct 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/90efb744873acd59517c78a1ebe300668c7079a3 commit 90efb744873acd59517c78a1ebe300668c7079a3 Author: Emily Stark <estark@google.com> Date: Fri Oct 13 16:44:06 2017 Fix origin serialization in Symantec console message The message was using scheme+host+port but not properly accounting for default/absent ports, so origins were printing as, e.g., "https://weather.com:". Oops. Now the message actually constructs an origin and gets its string form and doesn't try to serialize an origin itself. TBR=jochen@chromium.org Bug: 770406 Change-Id: I3d6e3a27957935d14f3acd7206a62e04a041dc0c Reviewed-on: https://chromium-review.googlesource.com/718837 Commit-Queue: Emily Stark <estark@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#508724} [modify] https://crrev.com/90efb744873acd59517c78a1ebe300668c7079a3/chrome/renderer/chrome_content_renderer_client.cc [modify] https://crrev.com/90efb744873acd59517c78a1ebe300668c7079a3/third_party/WebKit/Source/core/exported/LocalFrameClientImpl.cpp
,
Oct 16 2017
Requesting a merge for the commit in comment 11. It's a small typo fix and I've verified the fix on canary.
,
Oct 16 2017
,
Oct 16 2017
,
Oct 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6b9948e3ab13b07ce2414af9c60d7262a106d3db commit 6b9948e3ab13b07ce2414af9c60d7262a106d3db Author: Emily Stark <estark@google.com> Date: Wed Oct 18 06:29:02 2017 Fix origin serialization in Symantec console message The message was using scheme+host+port but not properly accounting for default/absent ports, so origins were printing as, e.g., "https://weather.com:". Oops. Now the message actually constructs an origin and gets its string form and doesn't try to serialize an origin itself. TBR=jochen@chromium.org Bug: 770406 Change-Id: I3d6e3a27957935d14f3acd7206a62e04a041dc0c Reviewed-on: https://chromium-review.googlesource.com/718837 Commit-Queue: Emily Stark <estark@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#508724}(cherry picked from commit 90efb744873acd59517c78a1ebe300668c7079a3) Reviewed-on: https://chromium-review.googlesource.com/725259 Reviewed-by: Emily Stark <estark@chromium.org> Cr-Commit-Position: refs/branch-heads/3239@{#48} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [modify] https://crrev.com/6b9948e3ab13b07ce2414af9c60d7262a106d3db/chrome/renderer/chrome_content_renderer_client.cc [modify] https://crrev.com/6b9948e3ab13b07ce2414af9c60d7262a106d3db/third_party/WebKit/Source/core/exported/LocalFrameClientImpl.cpp
,
Oct 18 2017
how about highlighting these requests in the network panel, with a new toolbar filter option "warnings"?
,
Oct 19 2017
@zac, not a bad idea. but it'll require some fairly significant eng work to pull off. Can you file a new issue for this? It'll be interesting to see the rollout of these warnings. :)
,
Nov 10 2017
,
Nov 10 2017
|
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by est...@chromium.org
, Oct 4 2017