Display "Not secure" verbose state for data: URLs

Display "Not secure" verbose state for data: URLs

data: URLs don't define a secure context, and are a vector for spoofing.
Display a "Not secure" badge for all data URLs, regardless of whether
they show a password or credit card field.

BUG=684811
Review-Url: https://codereview.chromium.org/2648353005
Cr-Commit-Position: refs/heads/master@{#446536}
Committed: https://chromium.googlesource.com/chromium/src/+/effd1d519ff74fb1eb998e844f19d5079222b0fd

[meacer, 2017-01-25 18:41:55]

The CQ bit was checked by meacer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 18:42:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-25 19:25:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-25 19:25:24]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[meacer, 2017-01-25 21:17:14]

The CQ bit was checked by meacer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 21:17:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[meacer, 2017-01-25 22:16:48]

Description was changed from

==========
Mark data: URLs as not secure

BUG=684811
==========

to

==========
Display "Not secure" verbose state for data: URLs

data: URLs don't define a secure context, and are a vector for spoofing.
Display a "Not secure" badge for all data URLs, regardless of whether
they show a password or credit card field.

BUG=684811
==========

[meacer, 2017-01-25 22:18:02]

meacer@chromium.org changed reviewers:
+ estark@chromium.org

[meacer, 2017-01-25 22:18:02]

estark: PTAL?

[commit-bot: I haz the power, 2017-01-26 00:08:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-26 00:08:12]

Dry run: This issue passed the CQ dry run.

[estark, 2017-01-26 22:46:28]

lgtm

https://codereview.chromium.org/2648353005/diff/20001/chrome/browser/ssl/secu...
File chrome/browser/ssl/security_state_tab_helper_browser_tests.cc (right):

https://codereview.chromium.org/2648353005/diff/20001/chrome/browser/ssl/secu...
chrome/browser/ssl/security_state_tab_helper_browser_tests.cc:790:
InjectScript(contents);
Is this necessary? I would think it's only necessary if HTTP_SHOW_WARNING is
triggered by a password field.

https://codereview.chromium.org/2648353005/diff/20001/components/security_sta...
File components/security_state/core/security_state.cc (right):

https://codereview.chromium.org/2648353005/diff/20001/components/security_sta...
components/security_state/core/security_state.cc:135: if
(url.SchemeIs(url::kDataScheme))
Could you please add a comment here explaining why we consider data scheme not
secure?

[meacer, 2017-01-26 23:49:24]

Thanks!

https://codereview.chromium.org/2648353005/diff/20001/chrome/browser/ssl/secu...
File chrome/browser/ssl/security_state_tab_helper_browser_tests.cc (right):

https://codereview.chromium.org/2648353005/diff/20001/chrome/browser/ssl/secu...
chrome/browser/ssl/security_state_tab_helper_browser_tests.cc:790:
InjectScript(contents);
On 2017/01/26 22:46:28, estark wrote:
> Is this necessary? I would think it's only necessary if HTTP_SHOW_WARNING is
> triggered by a password field.

Done.

https://codereview.chromium.org/2648353005/diff/20001/components/security_sta...
File components/security_state/core/security_state.cc (right):

https://codereview.chromium.org/2648353005/diff/20001/components/security_sta...
components/security_state/core/security_state.cc:135: if
(url.SchemeIs(url::kDataScheme))
On 2017/01/26 22:46:28, estark wrote:
> Could you please add a comment here explaining why we consider data scheme not
> secure?

Done.

[meacer, 2017-01-26 23:49:27]

The CQ bit was checked by meacer@chromium.org

[meacer, 2017-01-26 23:49:27]

The patchset sent to the CQ was uploaded after l-g-t-m from estark@chromium.org
Link to the patchset: https://codereview.chromium.org/2648353005/#ps40001
(title: "estark comments")

[commit-bot: I haz the power, 2017-01-26 23:51:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-27 02:14:16]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485474567580880,
"parent_rev": "5b819e65bf4cd955610efb49ba26540d87c8e304", "commit_rev":
"effd1d519ff74fb1eb998e844f19d5079222b0fd"}

[commit-bot: I haz the power, 2017-01-27 02:15:02]

Description was changed from

==========
Display "Not secure" verbose state for data: URLs

data: URLs don't define a secure context, and are a vector for spoofing.
Display a "Not secure" badge for all data URLs, regardless of whether
they show a password or credit card field.

BUG=684811
==========

to

==========
Display "Not secure" verbose state for data: URLs

data: URLs don't define a secure context, and are a vector for spoofing.
Display a "Not secure" badge for all data URLs, regardless of whether
they show a password or credit card field.

BUG=684811

Review-Url: https://codereview.chromium.org/2648353005
Cr-Commit-Position: refs/heads/master@{#446536}
Committed:
https://chromium.googlesource.com/chromium/src/+/effd1d519ff74fb1eb998e844f19...
==========

[commit-bot: I haz the power, 2017-01-27 02:15:03]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/effd1d519ff74fb1eb998e844f19...