New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 647561 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature
Team-Security-UX

Blocked on:
issue 647558

Blocking:
issue 647754



Sign in to add a comment

Send HTTP-bad policy to DevTools

Project Member Reported by f...@chromium.org, Sep 16 2016

Issue description

ChromeSecurityStateModelClient::GetSecurityStyle() should convert the page’s SecurityLevel back into the SecurityStyle that most closely represents it, along with explanations. This is a lossy operation. For a security level of INSECURE_VERBOSE, GetSecurityStyle() will represent this as SECURITY_STYLE_AUTHENTICATION_BROKEN, and add a string to the broken_explanations vector to explain that the page is marked as insecure because it contains password or credit card fields.

Assigning initially to emilyschechter@ in case she has a string in mind.
 
Please add this string: "In Chrome M56 (Jan 2017), this page will be marked as "not secure" in the URL bar. For more information see https://goo.gl/zmWq3m"
Owner: f...@chromium.org
assigning back to you for triage (actually adding the string)

Comment 3 by est...@chromium.org, Sep 16 2016

Other Emily: that's intended to be a console log message, right? Do you also have ideas for what the DevTools security panel should look like when on an HTTP-bad-due-to-pwd-or-cc page?

Comment 4 by est...@chromium.org, Sep 16 2016

Cc: lgar...@chromium.org
Components: Platform>DevTools>Security

Comment 5 by f...@chromium.org, Sep 16 2016

Blocking: 647754

Comment 6 by f...@chromium.org, Sep 17 2016

Cc: f...@chromium.org
Owner: emilyschechter@chromium.org
emilyschechter@, we need a little more info. For reference, here's a screenshot of what it looks like for mixed content.

* At the top of DevTools: should it say "This page is not secure" (i) or the harsher red triangle state?

* What should the section header be named?

* We can use the string "In Chrome M56 (Jan 2017), this page will be marked as 'not secure' in the URL bar. For more information see https://goo.gl/zmWq3m" for now. What should we show once it's launched?
Screen Shot 2016-09-16 at 5.10.14 PM.png
176 KB View Download

Comment 7 by f...@chromium.org, Sep 17 2016

Strawman proposal:

* It will have the (i) at the top

* Section header will be named "Private Content Found"

* Before launch, string will read: "The site includes {a password, credit card information} over HTTP, so a warning will be added to the URL bar in Chrome 56 (Jan 2017). For more information see $link."

* Once launched, string will read: "The site includes {a password, credit card information} over HTTP, so a warning has been added to the URL bar. For more information see $link."

===

And for the Console, a small tweak to emilyschechter's proposed string: 
"This page loads a {password, credit card} form field over HTTP. In Chrome 56 (Jan 2017), Chrome will show a warning in the URL bar for this page.  For more information see https://goo.gl/zmWq3m"

Comment 8 by est...@chromium.org, Sep 17 2016

I hate myself for saying this, but including a link (or any kind of markup) affects how we do the plumbing. Including a plain string (no markup) in the security panel is the easiest to do, because we just add a string to the corresponding explanations vector, and that string gets shipped down to devtools. Those strings aren't supposed to contain markup by law. (i.e. last I heard, devtools maintainers didn't want the devtools protocol to contain messages that contain markup)

If we want to include a link, I think we can still do it, it's just a little more complicated. Instead of adding a string to an explanations vector, we'd follow the example of mixed content: add bools for pwd/cc detection to SecurityStyleExplanations, along with a SecurityStyle member to indicate what icon devtools should use to flag pwd/cc detection. Then devtools itself would generate the string to display, and it can be however fancy we want (i.e. include a link).

Comment 9 by f...@chromium.org, Sep 18 2016

alternately we could provide a URL and not linkify it

i assume designers will throw rotten fruit at me for the suggestion but just putting it on the table

Comment 10 by f...@chromium.org, Sep 18 2016

(another option -- print the URL to the console, but not to devtools. it actually feels kinda weird to me to link to a blog post from inside devtools)
Cc: maxwalker@chromium.org
Ahhh. Yes. I was just thinking of the console.log message, hence the blog post link. Sorry for the confusion!

+ maxwalker for any more suggestions

+1 to felt's strawman with a few tweaks. Do you guys like the word "input"?

* use (i) icon
* Section header "Private User Data Input"

Dev tools UI (no links):
* Before: The site includes {a password, credit card information} input over HTTP, so a warning will be added to the URL bar in Chrome 56 (Jan 2017).
After: The site includes {a password, credit card information} input over HTTP, so a warning has been added to the URL bar.


Console.log:

* Before launch, string will read: "The site includes {a password, credit card information} input over HTTP, so a warning will be added to the URL bar in Chrome 56 (Jan 2017). For more information see $link1. For feedback please use $link2"

* Once launched, string will read: "The site includes {a password, credit card information} input over HTTP, so a warning has been added to the URL bar. For more information see $link1. For feedback please use $link2"

link1 = https://security.googleblog.com/2016/09/moving-towards-more-secure-web.html or short link
link2 = https://bugs.chromium.org/p/chromium/issues/detail?id=578317

#11 sgtm!
Note that (1) there are other embedders than Chrome.

Also, what if a page has both a password fields and a credit card field? Should we log separately?
Also, I recommend against linking to the main HTTP-bad bug unless we want passionate devs on our tails. I suggest linking them to a security-dev discussion at least.

> * It will have the (i) at the top
Agreed. This icon should match the page security indicator.

> * Section header will be named "Private Content Found"
"Private Content Found" is inconsistent with "Mixed Content". None of the other summaries contain a verb, either.
Could we just say "Private Content"?

Comment 15 by f...@chromium.org, Sep 20 2016

> Also, what if a page has both a password fields and a credit card field? Should we log separately?

I think we should. My placeholders were meant to mean "one of this set" where we should potentially show both. What do you think?

Not sure I like the wording "input over HTTP", because that makes it sound like the concern is simply the form data being *submitted* over HTTP, whereas in reality this warning is being shown for all insecure pages which include credit card / password fields regardless of what protocol that data is being submitted over, correct? Even pages served over HTTPS which include mixed content will show this warning, right?

With that in mind, here's my attempt at wording the message:

> This insecure page includes input field(s) for {a password, credit card information}. A warning for this will be added to the URL bar in Chrome 56 (Jan 2017).

> Not sure I like the wording "input over HTTP", because that makes it sound like the concern is simply the form data being *submitted* over HTTP

There are various ways for the data to be "leaked" or submitted over HTTP, even before you "submit" the form. Any sensitive input on HTTP should be treated with extreme caution.

> Even pages served over HTTPS which include mixed content will show this warning, right?

See Issue 554509.
re#14 -- I think just "private content" is too vague. lgarron@ do you have any issue with my suggestion of "Private User Data Input"?

also re#14 -- I want to give devs *someplave* to leave feedback (maybe they will find legit edge cases). Is there a thread you'd suggest? If not, I can also send a security-dev thread about this :)
#17: Yep, thank you for confirming that my understanding is correct. To reiterate, I'm suggesting the message in the DevTools/console be worded something along the lines of

> This insecure page includes input field(s) for {a password, credit card information}...

instead of

> The site includes {a password, credit card information} input over HTTP...

because in my mind the latter wording (input over HTTP) could *incorrectly* be interpreted by users as merely talking about what protocol the input data is transmitted over (i.e. the `target` of the form), whereas in reality the security level of the page containing password / credit card fields is what really triggers the warning.
Owner: est...@chromium.org
I'm working on a console message.
Project Member

Comment 21 by bugdroid1@chromium.org, Oct 17 2016

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

commit f8b269e58af3d7413986fb3196ce9e3f2d67a72f
Author: estark <estark@chromium.org>
Date: Mon Oct 17 19:17:14 2016

Add a console messsage for HTTP-bad

This CL logs a console message (at most once per navigation) when
ChromeSecurityStateModelClient::GetSecurityInfo() returns a security
level of HTTP_SHOW_WARNING. This message will allow us to start testing
when password/credit card detection will trigger a "Not secure" warning
in the omnibox, and will allow developers to start testing their sites.

BUG= 647561 
TEST=In chrome://flags, set #mark-non-secure-as to "Display a verbose
state when password or credit card fields are detected on an HTTP
page". Visit an HTTP page with a password field, like
http://nytimes.com. Open Developer Tools and observe a warning message
about the "Not secure" warning in the console.

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

[modify] https://crrev.com/f8b269e58af3d7413986fb3196ce9e3f2d67a72f/chrome/browser/ssl/chrome_security_state_model_client.cc
[modify] https://crrev.com/f8b269e58af3d7413986fb3196ce9e3f2d67a72f/chrome/browser/ssl/chrome_security_state_model_client.h
[modify] https://crrev.com/f8b269e58af3d7413986fb3196ce9e3f2d67a72f/chrome/browser/ssl/chrome_security_state_model_client_browser_tests.cc
[modify] https://crrev.com/f8b269e58af3d7413986fb3196ce9e3f2d67a72f/chrome/browser/ui/browser.cc
[modify] https://crrev.com/f8b269e58af3d7413986fb3196ce9e3f2d67a72f/chrome/browser/ui/browser.h
[add] https://crrev.com/f8b269e58af3d7413986fb3196ce9e3f2d67a72f/chrome/test/data/ssl/page_with_frame.html
[modify] https://crrev.com/f8b269e58af3d7413986fb3196ce9e3f2d67a72f/components/web_contents_delegate_android/web_contents_delegate_android.cc
[modify] https://crrev.com/f8b269e58af3d7413986fb3196ce9e3f2d67a72f/components/web_contents_delegate_android/web_contents_delegate_android.h
[modify] https://crrev.com/f8b269e58af3d7413986fb3196ce9e3f2d67a72f/content/browser/ssl/ssl_manager_unittest.cc
[modify] https://crrev.com/f8b269e58af3d7413986fb3196ce9e3f2d67a72f/content/public/browser/web_contents_delegate.h

Status: Started (was: Assigned)
Project Member

Comment 23 by bugdroid1@chromium.org, Oct 19 2016

Project Member

Comment 24 by bugdroid1@chromium.org, Oct 20 2016

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

commit 7b819cf77ecd601c18c126a264b6f71dfd0974db
Author: reillyg <reillyg@chromium.org>
Date: Thu Oct 20 22:46:01 2016

Initialize SecurityInfo::displayed_private_user_data_input_on_http.

This field must be initialized in the SecurityInfo constructor or else
MSan gets mad and behavior becomes undefined.

BUG= 647561 

Review-Url: https://chromiumcodereview.appspot.com/2433373004
Cr-Commit-Position: refs/heads/master@{#426625}

[modify] https://crrev.com/7b819cf77ecd601c18c126a264b6f71dfd0974db/components/security_state/security_state_model.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Oct 21 2016

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

commit 19e9536a6e51332042bc9824121ca0b37fef41ae
Author: estark <estark@chromium.org>
Date: Fri Oct 21 00:34:16 2016

Adjust HTTP-bad console messages

When I added a console message originally, I had missed a later comment
in the bug suggesting that we use different messages for before and
after launch. Thus this CL uses one console message for when the omnibox
warning is actually showing, and one for when it's not but will be in
the future.

BUG= 647561 

Review-Url: https://chromiumcodereview.appspot.com/2432933004
Cr-Commit-Position: refs/heads/master@{#426659}

[modify] https://crrev.com/19e9536a6e51332042bc9824121ca0b37fef41ae/chrome/browser/ssl/chrome_security_state_model_client.cc
[modify] https://crrev.com/19e9536a6e51332042bc9824121ca0b37fef41ae/chrome/browser/ssl/chrome_security_state_model_client_browser_tests.cc

Labels: M-56
Status: Fixed (was: Started)
Components: -Security>UX

Sign in to add a comment