New issue
Advanced search Search tips

Issue 705670 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Team-Security-UX


Show other hotlists

Hotlists containing this issue:
EnamelAndFriendsFixIt


Sign in to add a comment

Missing Explanation for SafeBrowsing block on Security Panel

Project Member Reported by elawrence@chromium.org, Mar 27 2017

Issue description

Chrome Version: 59.0.3053.0

What steps will reproduce the problem?
(1) Visit a SafeBrowsing-blocked page (e.g. https://testsafebrowsing.appspot.com/s/phishing.html)
(2) Click through the interstitial
(3) Open DevTools' Security tab

What is the expected result? "Explanation" of Unsafe state appears

What happens instead? Only status text explains source of the problem, other explanations are all green.

We need to add a broken_explanation.


src/components/security_state/content/content_utils.cc
  if (security_info.malicious_content_status !=
      security_state::MALICIOUS_CONTENT_STATUS_NONE) {
    security_style_explanations->summary =
        l10n_util::GetStringUTF8(IDS_SAFEBROWSING_WARNING);
  }
 
MissingDangerousExplanation.png.png
47.2 KB View Download
Labels: Hotlist-GoodFirstBug
Status: Fixed (was: Assigned)
This appears to be fixed.
Status: Available (was: Fixed)
This isn't actually fixed, and v63 looks as it does in the screenshot.
v63 doesn't show any bullets for me.

So the remainder of this bug is to add that bullet?
The bullets appear after you click through the interstitial. But yes, the entirety of this bug is to add a negative explanation to join the positive ones. Basically, we should pull the parenthetical from the summary and drop it down into a bullet.
Description: Show this description

Comment 7 by est...@chromium.org, Nov 10 2017

Labels: Hotlist-EnamelAndFriendsFixIt

Comment 8 by cthomp@chromium.org, Nov 16 2017

Owner: cthomp@chromium.org
Status: Started (was: Available)

Comment 9 by cthomp@chromium.org, Nov 17 2017

Here's a screenshot of a draft UI for showing this bullet. The explanations switch on the malicious_content_status of security state, and are mostly copied from the safe browsing interstitial messages.

I've uploaded a CL with this change to https://crrev.com/c/775904
new_bullet.png
56.7 KB View Download
Cc: srahim@chromium.org emilyschechter@chromium.org
srahim and emilyschechter: could you advise on a bullet point to put in the Devtools security panel when a page has been flagged by Safe Browsing? See the screenshot in Comment 9 which says "Flagged by Google Safe Browsing" followed by text from the interstitial. I'm inclined to say we should just keep it simple and say "This page has been flagged by Google Safe Browsing." and leave it at that. WDYT? Thanks!
It would be super cool if you could link to the public tool that gives details on why the page was flagged by SB -- https://transparencyreport.google.com/safe-browsing/search

That tool will enable them to find out why it was flagged.
Re comment #11: I know this sounds ridiculous, but it would be a lot of work to include anything other than a plain string.
We could add a clickable link in a console notification, or add a non-clickable shortlink in the Security panel?
Friendly ping to srahim@ and emilyschechter@: Do you have any advise on the strings contents, or should we just go with the simple strings estark@ posted in #10?
I think the simple string + enable to learn why with c#13 proposal for short link.

"This page has been flagged by Google Safe Browsing. To learn more about the Safe Browsing status of the site, please see [short link]."
Should that be a g.co link since we're pointing to the official safe browsing tool?
It looks like there is an existing short link g.co/safebrowsing, which goes to the main overview page https://transparencyreport.google.com/safe-browsing, but there is not an existing short link that points directly to the search tool. Do you think we should we request one, or just reuse the existing link to the overview page?
Ideally request one I think, since it's not immediately clear how to get from overview to site status page
Hi - To minimize redundancy of text, we could do something like this:

Red text: "This page is dangerous" 

Bullet:
- Flagged by Google Safe Browsing
To check this page's status, visit [short link to site status page]

To align with the other main-summaries in the security panel, I think we should keep a short explanation in the parenthetical. For example, a cert error shows "This page is not secure (broken HTTPS)." and a secure page shows "This page is secure (valid HTTPS)."

So perhaps:

Red text: "This page is dangerous (flagged by Google Safe Browsing)."
Bullet:
- Flagged by Google Safe Browsing
To check this page's status, visit g.co/safebrowsingstatus.

I've updated the CL to match this, but if there are strong opinions to minimize redundancy, I can remove the parenthetical.
Project Member

Comment 21 by bugdroid1@chromium.org, Dec 8 2017

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

commit 7bcbb4fde244ad53eb4531cfbf2422fa861f9057
Author: Christopher Thompson <cthomp@chromium.org>
Date: Fri Dec 08 01:35:30 2017

Add bullet for SafeBrowsing message in Dev Tools Security panel

This adds a bullet with summary and description for SafeBrowsing
warnings to the Security panel. These are mostly copied from the text
of the SafeBrowsing interstitial pages, and are cased on the
malicious content status (so phishing/malware/UwS each get different
description text, with a default for any other case).

Bug:  705670 
Change-Id: If083e7412de311474379ecac7936f64ac2857fad
Reviewed-on: https://chromium-review.googlesource.com/775904
Reviewed-by: Emily Stark <estark@chromium.org>
Commit-Queue: Christopher Thompson <cthomp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522670}
[modify] https://crrev.com/7bcbb4fde244ad53eb4531cfbf2422fa861f9057/components/security_state/content/content_utils.cc
[modify] https://crrev.com/7bcbb4fde244ad53eb4531cfbf2422fa861f9057/components/security_state/content/content_utils_unittest.cc
[modify] https://crrev.com/7bcbb4fde244ad53eb4531cfbf2422fa861f9057/components/security_state_strings.grdp

Status: Fixed (was: Started)

Sign in to add a comment