SecurityModel::SecurityInfo::security_bits inconsistent with documentation |
|||||
Issue descriptionVersion: 52 OS: Linux What steps will reproduce the problem? (1) Alter high level code e.g. LocationBarView to: WebContents* web_contents = delegate_->GetWebContents(); auto* client = ChromeSecurityStateModelClient::FromWebContents(web_contents); const security_state::SecurityStateModel::SecurityInfo& security_info(client->GetSecurityInfo()); some_log << security_info.security_bits; (2) Visit theonion.com or any HTTP-only site. What is the expected output? 'security_bits' should (eventually) settle on a positive power of 2. What do you see instead? It remains -1. It appears to mimic SecurityStateModel::SecurityLevel::NONE which bundles 'unknown' with 'HTTP'.
,
Aug 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/69be1c3956d4dbff1062172e3a9ec19191dbc52f commit 69be1c3956d4dbff1062172e3a9ec19191dbc52f Author: estark <estark@chromium.org> Date: Sat Aug 27 11:08:43 2016 Fix |security_bits| comment on SecurityStateModel BUG=636380 Review-Url: https://codereview.chromium.org/2282833002 Cr-Commit-Position: refs/heads/master@{#414924} [modify] https://crrev.com/69be1c3956d4dbff1062172e3a9ec19191dbc52f/components/security_state/security_state_model.h
,
Aug 27 2016
,
Sep 13 2016
,
Sep 13 2016
felt, did you mean to mark this Available?
,
Sep 13 2016
estark, the problem is that the bug sort of refers to 2 things: The comment and the behavior. Yes, the comment has been fixed but the model still has coarse resolution. If it would be more helpful, I could fork a new bug, specifically referring to the behavior.
,
Sep 13 2016
Ah, I see. My personal preference would be to leave |security_bits| as it is, and add a new SecurityLevel value to distinguish Unknown from None (HTTP). The NavigationEntry already has a SecurityStyle value for unknown that we convert to NONE, so I think it would be easy to map SECURITY_STYLE_UNKNOWN to a new Unknown SecurityLevel value rather than mapping it to NONE as we currently do. (https://cs.chromium.org/chromium/src/chrome/browser/ssl/chrome_security_state_model_client.cc?sq=package:chromium&rcl=1473751756&l=362)
,
Sep 14 2016
I also think we should separate out UNKNOWN and NONE states. I was just taking a look at https://bugs.chromium.org/p/chromium/issues/detail?id=640975 and the core problem there too is that there isn't a way to distinguish between the two, so we end up with NONE in a situation where we should really be UNKNOWN: SecurityStateModel::SecurityLevel ToolbarModelImpl::GetSecurityLevel( bool ignore_editing) const { // When editing or empty, assume no security style. return ((input_in_progress() && !ignore_editing) || !ShouldDisplayURL()) ? SecurityStateModel::NONE : delegate_->GetSecurityLevel(); }
,
Sep 14 2016
(And I'm sure there are other places in the code where this happens)
,
Sep 14 2016
here's another place: https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java?rcl=0&l=1212 if (getCurrentTab() == null) return ConnectionSecurityLevel.NONE; return getCurrentTab().getSecurityLevel();
,
Sep 14 2017
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 1
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by ssamanoori@chromium.org
, Aug 12 2016