New issue
Advanced search Search tips

Issue 654600 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

DevTools security panel says "broken HTTPS" on malware/phishing sites

Project Member Reported by est...@chromium.org, Oct 10 2016

Issue description

Visit https://ianfette.org, open the security panel. Observe that it says "This page is insecure (broken HTTPS)."

This is on both the interstitial and after you click through.

To fix this, I think we might want to add a "main_explanation" or some such to content::SecurityStyleExplanations, and have DevTools display the main_explanation string instead of making it up itself. Then, for malware/phishing pages, we could send "This page is valid HTTPS but has malware or phishing" instead of "This page is insecure (broken HTTPS)".

This would also be helpful for bugs like issue 543864, where we might want to display a different "main explanation" for chrome:// or other non-https secure origins than we do for https.
 

Comment 1 by vakh@chromium.org, Oct 21 2016

Labels: SafeBrowsing-Triaged
Cc: elawrence@chromium.org est...@chromium.org
Labels: Team-Security-UX
Owner: ----
elawrence: this is the bug I was mentioning this morning. Here's what I was thinking we can do to fix this:

- Add a 'summary' string in SecurityStyleExplanations [1] and to the securityStateChanged event in the DevTools protocol [2].

- In SecurityPanel.js, instead of choosing a summary string based on the security style, take the 'summary' string from the securityStateChanged event. [3]

- Populate the SecurityStyleExplanations 'summary' string in ChromeSecurityStateModelClient::GetSecurityStyle. [4] In most cases we'd use the same strings as SecurityPanel.js is using now. But when |security_info.fails_malware_check| is true and there are no certificate errors, we could set the 'summary' string to something like "This page contains malware or a phishing scam." (or something. I'm not good at strings.)


[1] 2https://cs.chromium.org/chromium/src/content/public/browser/security_style_explanations.h?q=SecurityStyleExplanations&sq=package:chromium&l=28

[2] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/inspector/browser_protocol.json?q=browser_protocol.json&sq=package:chromium&l=948

[3] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js?q=SecurityPanel.js&sq=package:chromium&l=653

[4] https://cs.chromium.org/chromium/src/chrome/browser/ssl/chrome_security_state_model_client.cc?sq=package:chromium&rcl=1478435172&l=181
Owner: elawrence@chromium.org

Comment 4 by est...@chromium.org, Nov 21 2016

Ping! Eric have you had a chance to look at this yet?
Status: Started (was: Assigned)
Oh, yes, I started looking at this.
Components: -Security>UX

Comment 7 by est...@chromium.org, Nov 28 2016

[4] is now https://cs.chromium.org/chromium/src/components/security_state/content/content_utils.cc?sq=package:chromium&rcl=1480335437&l=181

(formerly ChromeSSMClient::GetSecurityStyle, now security_state::GetSecurityStyle)
Adding screenshot of before and after fix.
BeforeAndAfter.png
105 KB View Download
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 16 2016

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

commit 7a5785c7cb84a522fc8b148bb80afbd74acbc695
Author: elawrence <elawrence@chromium.org>
Date: Fri Dec 16 00:45:15 2016

Override DevTools security summary when a Safe Browsing warning shows.

When a Safe Browsing warning is showing, the Developer Tools Security
panel should show custom text in the summary area.

BUG= 654600 

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

[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/chrome/browser/ssl/security_state_tab_helper_browser_tests.cc
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/components/security_state/content/content_utils.cc
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/components/security_state_strings.grdp
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/content/browser/devtools/protocol/security_handler.cc
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/content/public/browser/security_style_explanations.cc
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/content/public/browser/security_style_explanations.h
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/headless/lib/headless_web_contents_browsertest.cc
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/active-and-passive-subresources-with-cert-errors.html
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/active-subresource-with-cert-errors.html
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/blocked-mixed-content-and-subresources-with-cert-errors.html
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-active-and-passive-reload.html
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-and-subresources-with-cert-errors.html
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/mixed-content-reload.html
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/passive-subresource-with-cert-errors.html
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-all-resources-secure.html
[add] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-blocked-mixed-content-and-malicious-expected.txt
[add] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-blocked-mixed-content-and-malicious.html
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-blocked-mixed-content.html
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-explanation-ordering.html
[add] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-secure-but-malicious-expected.txt
[add] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-secure-but-malicious.html
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/Source/core/inspector/browser_protocol.json
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/Source/devtools/front_end/security/SecurityModel.js
[modify] https://crrev.com/7a5785c7cb84a522fc8b148bb80afbd74acbc695/third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js

Status: Fixed (was: Started)

Sign in to add a comment