New issue
Advanced search Search tips

Issue 648326 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Remove |security_style| from SSLStatus

Project Member Reported by est...@chromium.org, Sep 19 2016

Issue description

SSLStatus contains a SecurityStyle member. SecurityStateModel uses the |security_style|, in addition to some other information from the SSLStatus, to compute a SecurityLevel for the page. (SecurityLevel is a finer-grained enum containing some states that reflect Chrome-specific policy.)

However, where things get confusing is that SecurityStateModel converts SecurityLevels back into SecurityStyles for the purpose of conveying page security state to DevTools. So in some cases, the SSLStatus will have a SecurityStyle which gets converted into a SecurityLevel which gets converted back into a *different* SecurityStyle than we started with.

To alleviate the confusingness, I propose that we remove the |security_style| member from SSLStatus. |security_style| is a simple calculation from a couple of pieces of information that are already on the SSLStatus (see GetSecurityStyleForResource), so embedders can easily recompute it themselves.

I think this will be a good simplification as it will leave SecurityStyle with the single purpose of conveying security state from the embedder to DevTools. (As opposed to the two purposes it has now: conveying security state from //content to the embedder *and* conveying security state with embedder-specific policy applied from the embedder to DevTools.)
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 11 2016

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

commit 47ba9c7d92885d137c51b462d8af87e3f3a6ecdd
Author: estark <estark@chromium.org>
Date: Tue Oct 11 15:40:11 2016

Remove SSLStatus::security_style member and content::SecurityStyle

Previously, //content used SSLStatus::security_style to express an
opinion about the overall security state of the
page. SecurityStateModel/ChromeSecurityStateModelClient used this
SecurityStyle as a starting point, applying Chrome-specific policies
such as SHA1 deprecation to produce a SecurityLevel, which corresponds
to the lock icon. Then, for conveying the security state of a page back
to DevTools, the SecurityLevel would get converted back into a
SecurityStyle (often a different SecurityStyle than what //content
assigned in SSLStatus::security_style). Additionally, SecurityStyles are
used to convey per-request security info to DevTools.

This CL removes SSLStatus::security_style, so that //content no longer
assigns an overall security state to a page. This means that
SecurityStyles are now only used to convey security
information (per-page or per-request) to DevTools. We only convert from
SecurityLevels to SecurityStyles, and never the other way around.

Since content::SecurityStyle no longer serves any purpose besides
ferrying information to devtools, I removed content::SecurityStyle and
just use blink::WebSecurityStyle everywhere (which has been pulled out
into its own file, from blink::WebURLResponse::SecurityStyle).

This should hopefully make SecurityLevel/SecurityStyle less confusing,
and remove the temptation to use SSLStatus::security_style
as an indicator of overall page security state instead of the
embedder-specific SecurityLevel.

BUG= 648326 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/android_webview/native/aw_autofill_client.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/chrome/browser/DEPS
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/chrome/browser/android/policy/policy_auditor.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/chrome/browser/ssl/bad_clock_blocking_page.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/chrome/browser/ssl/captive_portal_blocking_page.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/chrome/browser/ssl/captive_portal_blocking_page_browsertest.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/chrome/browser/ssl/chrome_security_state_model_client.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/chrome/browser/ssl/chrome_security_state_model_client.h
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/chrome/browser/ssl/chrome_security_state_model_client_browser_tests.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/chrome/browser/ssl/ssl_blocking_page.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/chrome/browser/ssl/ssl_browser_tests.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/chrome/browser/ui/autofill/chrome_autofill_client.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/chrome/browser/ui/browser.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/chrome/browser/ui/browser.h
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/components/security_state/security_state_model.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/components/security_state/security_state_model.h
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/components/security_state/security_state_model_client.h
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/components/security_state/security_state_model_unittest.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/browser/DEPS
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/browser/devtools/protocol/security_handler.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/browser/devtools/protocol/security_handler.h
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/browser/frame_host/navigation_entry_impl_unittest.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/browser/loader/navigation_resource_handler.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/browser/ssl/ssl_manager.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/child/web_url_loader_impl.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/common/BUILD.gn
[delete] https://crrev.com/d03a1b8771ca27457673aae76b4ce56c1771cf82/content/common/security_style_util.cc
[delete] https://crrev.com/d03a1b8771ca27457673aae76b4ce56c1771cf82/content/common/security_style_util.h
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/public/browser/security_style_explanations.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/public/browser/security_style_explanations.h
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/public/browser/ssl_status.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/public/browser/ssl_status.h
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/public/browser/web_contents_delegate.cc
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/public/browser/web_contents_delegate.h
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/public/browser/web_contents_observer.h
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/public/common/BUILD.gn
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/content/public/common/common_param_traits_macros.h
[delete] https://crrev.com/d03a1b8771ca27457673aae76b4ce56c1771cf82/content/public/common/security_style.h
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/ios/chrome/browser/ssl/ios_chrome_security_state_model_client.h
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/ios/chrome/browser/ssl/ios_chrome_security_state_model_client.mm
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/third_party/WebKit/Source/platform/exported/WebURLResponse.cpp
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/third_party/WebKit/public/BUILD.gn
[add] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/third_party/WebKit/public/platform/WebSecurityStyle.h
[modify] https://crrev.com/47ba9c7d92885d137c51b462d8af87e3f3a6ecdd/third_party/WebKit/public/platform/WebURLResponse.h

Comment 2 by est...@chromium.org, Oct 11 2016

Labels: M-56
Status: Fixed (was: Assigned)
Components: -Security>UX Internals>PageSecurityState
Components: Internals>Permissions>CrowdConsent
Components: -Internals>Permissions>CrowdConsent

Sign in to add a comment