Issue metadata
Sign in to add a comment
|
CSP is not injected from uBlock Origin when a website is opened via a tile in chrome://newtab on a FIRST VISIT
Reported by
sscar...@gmail.com,
Dec 28
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3635.0 Safari/537.36 Steps to reproduce the problem: 1. Have the latest bleeding edge build of Chromium or take one from https://download-chromium.appspot.com/ 2. Open chrome.exe and have uBO installed or install it from https://chrome.google.com/webstore/detail/ublock-origin/cjpalhdlnbpafiamejdnhcphjbkeiagm?hl=en 3. In chrome://newtab, click on the Edit shortcut button which appears like 3 dots vertically stacked on an empty or an existing tile and fill the information like this - https://i.gyazo.com/64761dd41bd6345b82f7ad3219ca3e0d.png 4. Open chrome-extension://cjpalhdlnbpafiamejdnhcphjbkeiagm/dashboard.html and click on My Filters tab, add ||ghacks.net^$csp=default-src 'none' and click on Apply Changes and close the tab. 5. Close the browser and open it again, open chrome://newtab, click on the ghacks.net tile we created above and when the website opens it should appear broken because of the CSP filter we added in uBlock Origin in step 4, but it doesn't. The page loads resources successfully which shouldn't have happened. What is the expected behavior? resources should be blocked because of a CSP filter being present as mentioned in step 4. What went wrong? Resources are getting loaded successfully on a first visit despite of a CSP filter being present. WebStore page: uBlock Origin Did this work before? Yes 73.0.3635.0 Chrome version: 73.0.3635.0 Channel: canary OS Version: 10.0 Flash Version: This is regression as this didn't happened back in 73.0.3635.0
,
Dec 28
Did that bisect break this ?
,
Dec 28
Does the CSP get applied later on subsequent loads or does it not work at all even after the first load?
,
Dec 28
Works fine on a refresh of the page.
,
Dec 28
Subsequent intra-site navigation within the tab is processed by uBlock. Only the initial NTP entry click is affected.
,
Dec 28
But if I close the tab and go back to chrome://newtab, click on the ghacks tile and open it from there again, it happens again.
,
Dec 28
Adding rdevlin.cronin@ as owner of extensions.
,
Dec 28
Evidently the problem is caused by these two factors: 1. the NTP page has chrome://newtab origin which is off-limits for extensions 2. webRequest API requires the 'initiator' to be accessible for an extension since Chrome 72 r609787 While the intentions were good, the reported case certainly can't be working-as-intended.
,
Dec 28
So that's a side-effect of https://crrev.com/609787 ?
,
Dec 28
It seems to me that if 2. is indeed required, then the fact that this worked was a bug, not a feature. What the changes in crrev.com/c/1379217 did was to add the initiator origin of navigations to all the different ways a navigation can start. Before that change, the OpenURL IPC did not carry this information, which meant that the extensions subsystem likely couldn't enforce the initiator permissions. Now the extensions code can enforce it, since it is present and likely that is what has regressed it. I haven't verified in debugger, as I don't have an easy access to one right now, but I'm almost certain that would be the root cause. I'd leave it to rdevlin.cronin@ to chime in whether this is breaking behavior and we should be doing something about it, but from what I can see it looks like the desired behavior. woxxom@, thank you very much for helping troubleshoot this issue and providing the extra context!
,
Dec 28
Comment #9, yes, I think this is more attributable to https://crrev.com/609787, since it changed the extension API expectation/behavior.
,
Dec 28
The status still remains Unconfirmed. Were you not able to reproduce it ?
,
Dec 28
No, I personally was not able to reproduce it, since the edit menu given in the repro steps wasn't available on the Mac and I didn't have a Windows machine handy. I've left it unconfirmed, so extensions folks can indeed confirm this is the expected behavior or tell us otherwise.
,
Dec 28
Over to karandeepb@. This is definitely interesting. Checking for the initiator URL was intended to block extensions from intercepting requests from a page they don't have access to and indirectly modifying that page, but I don't think it should apply to top-level navigations. Karan, WDYT?
,
Dec 30
Addendum - It doesn't happen if I right click on the website tile and click on Open link in new tab.
,
Dec 30
..also with doesn't happen with Bookmarks too.
,
Dec 31
Yes, it won't happen in those cases, as there is no document that initiated the navigation, it was started by the browser process itself - both for the context menu and the bookmarks.
,
Jan 2
>> Over to karandeepb@. This is definitely interesting. Checking for the initiator URL was intended to block extensions from intercepting requests from a page they don't have access to and indirectly modifying that page, but I don't think it should apply to top-level navigations. Karan, WDYT? Yeah this seems to have been an oversight on our part. We should not be checking the initiator for main-frame navigation requests. In fact, I am not sure if we should be checking the initiator for even sub-frame navigation requests. It seems to me that it an extension has access to a frame, it should be possible for it to block/redirect it, regardless of its parent frame's origin. Thoughts?
,
Jan 7
,
Jan 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/455363f3a2cf47eb04c13bc65cb80e72387d2252 commit 455363f3a2cf47eb04c13bc65cb80e72387d2252 Author: Karan Bhatia <karandeepb@chromium.org> Date: Wed Jan 09 01:19:04 2019 WebRequest: Extensions intercepting navigation requests don't need access to initiator. This CL makes the following changes to the host permission model for the Web Request and Declarative Net Request APIs: - To intercept a navigation request (sub-frame or main-frame request), extensions don't need host permissions to the request initiator. They only need access to the request url. This is unlike sub-resource requests which need host permission to both the request url and initiator. - If an extension has access to a navigation request's initiator but it's access to the request url is withheld, the extension isn't implicitly granted access to the request. This is unlike sub-resource requests where the extension will be granted access. BUG=157736, 918137 , 851722 Change-Id: I851bfbe79ea737b59934779367d581cdbb6c5297 Reviewed-on: https://chromium-review.googlesource.com/c/1396564 Commit-Queue: Karan Bhatia <karandeepb@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Cr-Commit-Position: refs/heads/master@{#620977} [modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc [modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/chrome/browser/extensions/api/web_request/web_request_apitest.cc [modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/chrome/browser/extensions/api/web_request/web_request_permissions_unittest.cc [modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/chrome/common/extensions/docs/templates/intros/declarativeNetRequest.html [modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/chrome/common/extensions/docs/templates/intros/webRequest.html [modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/declarative_net_request/ruleset_manager.cc [modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/declarative_webrequest/webrequest_action.cc [modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/web_request/web_request_api.cc [modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/web_request/web_request_event_details.cc [modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/web_request/web_request_permissions.cc [modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/web_request/web_request_permissions.h [modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/web_request/web_request_permissions_unittest.cc
,
Jan 9
This should be fixed by c#20. I don't think this requires a merge to M72 since this only became an issue in combination with crrev.com/c/1379217 (m73), which added initiator info. to renderer initiated navigations to remote frames and those corresponding to window.open. Before that only local/same-process renderer initiated navigations had initiator info.
,
Jan 10
I want to test, so where did the fix landed version wise ?
,
Jan 10
It landed at 73.0.3666.0. It should be available on the latest Mac, Win dev, canary.
,
Jan 10
Thank you. It works as expected in 73.0.3666.0. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by woxxom@gmail.com
, Dec 28