I'm curious as to how marking HTTP as "Not secure" will be implemented for 68.

Today, If we deem the site for a Bookmark App to be dangerous, e.g. expired cert, we will bring down the location bar (basically and uneditable omnibox, see attached image).

We do this by retrieving the SecurityInfo for the tab and checking if SecurityLevel is "DANGEROUS"[1]. If the change to HTTP as "Not secure" means the SecurityLevel becomes "DANGEROUS" for http sites, then I think this is lower priority since we will have an indicator. (Albeit a really ugly indicator).

[1] https://cs.chromium.org/chromium/src/chrome/browser/ui/extensions/hosted_app_browser_controller.cc?type=cs&q=shouldshowlocationbar&sq=package:chromium&l=131

Deleted: Screen Shot 2018-02-12 at 1.50.56 PM.png 1.4 MB

	Screen Shot 2018-02-12 at 1.50.56 PM.png 1.4 MB View Download

I'm going to try and see if I can quickly fix this. I'm more than happy to have an ugly address bar indicator pop down if you are running a http web app. First of all, we want people migrating to https. Secondly, we want them migrating to PWAs.

Fix is very easy. Regardless of what the SecurityLevel is, we just change the "is on the same origin" check to require https. If it's not https, it's considered off-origin so we show the location bar.

Attached screenshot of the fix on Chrome OS. The window on the left is https://example.com; the window on the right is http://example.com.

Deleted: Screenshot from 2018-02-12 15-26-09.png 48.7 KB

	Screenshot from 2018-02-12 15-26-09.png 48.7 KB View Download

(Aside: Looks like on the latest build, the "Not secure" is showing up already, even in 66. I thought this was not scheduled until 68, but maybe it's going to be in Canary/Dev but switched off in Beta/Stable until 68. This change should work irrespective of that.)

CL is up: https://crrev.com/c/913191.

This certainly doesn't sound like Severity Medium and I don't see any reason this needs to be View-Restricted or tracked as a security bug.

Eric: I suggested this be filed as a security bug. What concerns me is not so much that we show the Not Secure warning for http, but rather that we show the security indicator for mixed content or content loaded with security indicators. Per #1 it looks like we're already showing the location bar for DANGEROUS, so this is probably not as big an issue as I thought originally. (I thought we just never show a security indicator, which I think of as Medium severityas "A bug that allows web content to tamper with trusted browser UI.")

Could we show the location bar whenever the security level is not SECURE?

> Could we show the location bar whenever the security level is not SECURE?

I guess that would be a more robust way of doing what I already did in my patch (which is to show Location if EITHER it's DANGEROUS (existing check) OR it's not https).

Would we include SECURE, EV_SECURE and SECURE_WITH_POLICY_INSTALLED_CERT? I'm happy to change to that logic.

Re #10: I think so. We have the security_state::IsSslCertificateValid(SecurityLevel security_level) function that does this.

Matt does the existing code listen for updates to the security level, e.g. if mixed content is loaded on the page dynamically? It looks like it fires on NavigationStateChanged but I'm not sure if that fires for any security level change. One way to do that is to implement WebContentsObserver::DidChangeVisibleSecurityStyle.

Sorry, WebContentsObserver::DidChangeVisibleSecurity*State*

#12 not sure, but it would be a follow-up to fix that.

Can we make sure any UI changes, in particular to hosted apps which I believe this affects, go through UI review?

#15 emailed them.

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

commit e49689cd0ec555a2bf5aa579f94d56c8e161dfe3
Author: Matt Giuca <mgiuca@chromium.org>
Date: Mon Feb 19 06:33:44 2018

Hosted/Bookmark apps now show the location bar for http connections.

HostedAppTest now use https for all tests other than http-specific ones
(otherwise, all tests would show a location bar, and we wouldn't be
properly testing any of the other behaviour).

Bug:  811158 
Change-Id: I0fe220055fb6574c3950f65971e06feadaeabcd9
Reviewed-on: https://chromium-review.googlesource.com/913191
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537588}
[modify] https://crrev.com/e49689cd0ec555a2bf5aa579f94d56c8e161dfe3/chrome/browser/ui/extensions/hosted_app_browser_controller.cc
[modify] https://crrev.com/e49689cd0ec555a2bf5aa579f94d56c8e161dfe3/chrome/browser/ui/extensions/hosted_app_browsertest.cc
[rename] https://crrev.com/e49689cd0ec555a2bf5aa579f94d56c8e161dfe3/chrome/test/data/extensions/https_app_no_www/manifest.json

This should make "http" origin pages show the location bar. But we don't yet dynamically show the location bar for mixed content or other non-secure situations in an "https" scheme. So keeping this bug open.

See: https://chromium-review.googlesource.com/c/chromium/src/+/923226

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

commit 09fa83a759eb20ade48ab85b06c21d3ff05edba4
Author: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Date: Thu Apr 12 08:14:46 2018

desktop-pwas: show location bar whenever the site is not considered secure

Changes the logic to show the location bar any time a site is not secure
e.g. if it displays mixed content, has a mixed content form, etc.

Before, we would only show the location bar for dangerous sites e.g.
invalid certificate, or if the site was not using https.

Based on https://crrev.com/c/923226 by mgiuca.

Bug:  811158 
Change-Id: I72b59eaab031f20924057d1c7ec614f221c52c61
Reviewed-on: https://chromium-review.googlesource.com/1001075
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550098}
[modify] https://crrev.com/09fa83a759eb20ade48ab85b06c21d3ff05edba4/chrome/browser/ssl/ssl_browsertest.cc
[modify] https://crrev.com/09fa83a759eb20ade48ab85b06c21d3ff05edba4/chrome/browser/ssl/ssl_browsertest_util.cc
[modify] https://crrev.com/09fa83a759eb20ade48ab85b06c21d3ff05edba4/chrome/browser/ssl/ssl_browsertest_util.h
[modify] https://crrev.com/09fa83a759eb20ade48ab85b06c21d3ff05edba4/chrome/browser/ui/browser.cc
[modify] https://crrev.com/09fa83a759eb20ade48ab85b06c21d3ff05edba4/chrome/browser/ui/extensions/hosted_app_browser_controller.cc
[modify] https://crrev.com/09fa83a759eb20ade48ab85b06c21d3ff05edba4/chrome/browser/ui/extensions/hosted_app_browsertest.cc

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot