IsOriginSecure() does not seem to honor origin whitelist in browser process |
|||
Issue descriptionBug 916915 describes an issue where cache_storage is using IsOriginSecure() in the browser process and hitting unexpected behavior on whitelisted http origins. It seems the renderer honors the whitelist, but calling IsOriginSecure() in the browser process does not. Is this expected? IsOriginSecure() is defined in content/common, so it seems like it should work in the browser process. It calls into IsWhitelistedSecure(), but there is this comment that suggests the whitelist will only be valid in the renderer where switches are set: https://cs.chromium.org/chromium/src/chrome/common/secure_origin_whitelist.cc?l=148-153&rcl=dd5298e5f90f99e33ce9fe56646d8db618edcc6f I could add some extra code to look at prefs and run ParseWhitelist() manually in cache_storage, but it seems like IsOriginSecure() should probably be doing this for all callers. What do you think?
,
Dec 20
The cache_storage code is only triggered after a renderer has sent a mojo interface. So maybe the policy should always be available at that point without waiting for a callback? Its possible I don't understand what you mean, though. It feels weird, though, for a security policy method in content/common to behavior differently in different processes. It seems at a minimum this should be documented here: https://cs.chromium.org/chromium/src/content/public/common/origin_util.h?l=15-20&rcl=dd5298e5f90f99e33ce9fe56646d8db618edcc6f But even if that's the case, I see lots of uses in content/browser. Like clear-site-data, etc. It seems like these should all behave consistently which the browser's view of what a secure origin is.
,
Dec 20
Ahh, I'm slowly paging this all back into my head (it's been several months since I made the changes for handling the whitelist in the browser process).
I recall there being an issue with accessing the profile and prefs in content/. Specifically, the current way that the SecurityStateTabHelper works is to access the prefs via the WebContents that it is attached to:
Profile* profile =
Profile::FromBrowserContext(web_contents()->GetBrowserContext());
PrefService* prefs = profile->GetPrefs();
Having the policy work for key browser UI (the security indicators in the omnibox) was considered the highest priority, so there may be some rough edges remaining. If there's a good way to merge the prefs and command line flags in the content origin_util code instead, that would probably be the best approach. I'd be happy to help migrate that over.
,
Dec 20
+vogelheim@ who originally implemented the flag and policy.
,
Jan 16
(6 days ago)
Debugging this some more due to Bug 920819. I'm trying to reason out why content::IsOriginSecure doesn't pick up the new secure origins when it queries getSecureOriginsAndPatterns() in https://cs.chromium.org/chromium/src/content/common/origin_util.cc?l=49&rcl=9060c69e3ef55ff72cdd9f9a82458fefd375f115. Following how I think this gets initialized: 1) content_main_runner_impl.cc calls RegisterContentSchemes() here https://cs.chromium.org/chromium/src/content/app/content_main_runner_impl.cc?l=752&rcl=9060c69e3ef55ff72cdd9f9a82458fefd375f115 2) RegisterContentSchemes() calls GetContentClient()->AddAdditionalSchemes() here https://cs.chromium.org/chromium/src/content/common/url_schemes.cc?l=56&rcl=9060c69e3ef55ff72cdd9f9a82458fefd375f115 3) AddAdditionalSchemes() pulls the whitelist here (for Chrome only) https://cs.chromium.org/chromium/src/chrome/common/chrome_content_client.cc?l=625&rcl=65ce745899bd2f85f51dff584138e29deba78c7f, but... 4) GetWhitelist() just pulls from the command-line arguments https://cs.chromium.org/chromium/src/chrome/common/secure_origin_whitelist.cc?l=147&rcl=65ce745899bd2f85f51dff584138e29deba78c7f, **which are not set from the policy for the browser process** 3) It then sets the |MutableSecureOriginsAndPatterns| here https://cs.chromium.org/chromium/src/content/common/url_schemes.cc?l=115&rcl=9060c69e3ef55ff72cdd9f9a82458fefd375f115 4) content::IsOriginSecure() ends up calling content::GetSecureOriginsAndPatterns() here https://cs.chromium.org/chromium/src/content/common/origin_util.cc?l=50&rcl=9060c69e3ef55ff72cdd9f9a82458fefd375f115 which should result in the same set of secure origins set up in the content client. For renderers, this is passed through setting command line flags during initialization (these can't be set for the browser process because they are immutable), so the initialization actually works (and why we can just pass a callback to content::IsOriginSecure() into Blink stuff). So we're back to needing to access the enterprise policy somehow, which is only accessible via Prefs (I think) in the browser process, and include them in how we return the result of GetWhitelist() for IsOriginSecure(). Since this gets originally set up in chrome/common/chrome_content_client.cc:AddAdditionalSchemes(), maybe there's a way to handle this that's allowed in chrome/common/ (and will be already parsed and accessible during this initialization). Another possibility is that once the enterprise policy is being read in and the prefs set, we could apply the secure origin list back to the GetMutableSecureOriginsAndPatterns() storage. I'll try to dig around to see if that would be feasible. (At best, it would avoid having to invert this and have complicated logic to pass down the policy back into content/ everywhere.) |
|||
►
Sign in to add a comment |
|||
Comment 1 by cthomp@chromium.org
, Dec 20