search.query() includes an Origin header when called with OpenSearch provider that uses POST
Categories
(WebExtensions :: Request Handling, defect)
Tracking
(Not tracked)
People
(Reporter: dotproto, Unassigned, NeedInfo)
References
Details
This bug was originally reported on Discourse.
Calling browser.search.query() in an extension unexpectedly includes the extension's Origin header in the HTTP request sent to the web server if the search provider's OpenSearch definition uses POST as the request method.
Reproduction steps
- Navigate to
https://html.duckduckgo.com/html/ - Right click the address bar and select
Add "DuckDuckGo (HTML)" - Open
about:preferences#searchand change your Default Search Engine to DuckDuckGo HTML. - Install Vimium (https://addons.mozilla.org/en-GB/firefox/addon/vimium-ff/)
- Navigate to
https://example.com - Press
oand type a search term
Observed behavior
The address bar will show https://html.duckduckgo.com/html/ and the page body will say forbidden.
Expected behavior
The address bar should say https://html.duckduckgo.com/html/ and the page body should contain search results for the search term.
Additional notes
It seems that when the extension calls browser.search.query(), the resulting HTTP request includes the extension's origin in the Origin header, causing DDG's server to reject the request with a 403. A similar issue was reported in the past (bug 1615901).
Original post
I use the HTML variant of DuckDuckGo available at
https://html.duckduckgo.com/html/. I added the search engine by going to the aforementioned URL and right-clicking in the address bar and adding the search engine. As of the recent version 2.2.0 Vimium seems to be using search.query() to perform a search using the browser’s default search engine.Interestingly, this broke the search functionality for me. I looked into it a bit and reported the issue on the Vimium issue tracker, where the kind GitHub user philg-dev took the time to investigate.
They came to the conclusion that what’s probably going on is that as the DuckDuckGo HTML and Lite variants specify POST as the request method in their OpenSearch description the “POST requests through Vimium result in failure due to the Content Security Policy, since Vimium’s Vomnibar sends an HTTP POST request with
Origin: moz-extension://964d1f78-8013-445f-b3d0-4cace47a2eb4, which obviously doesn’t align with anything that is defined in DDG’s CSP.”https://github.com/philc/vimium/issues/4669
As I’ve mentioned I am no developer, so I’m very unsure, but I feel like perhaps this isn’t how it’s supposed to be? My impression is that search.query() is supposed to allow to perform a search just like the browser would. Of course, I might be very wrong!
Is there nothing that may be improved on the side of the browser? Is this just how it is and should be and the DuckDuckGo HTML and Lite variants should use GET requests if they want this use case to work?
Anyway, I’m very curious to hear any thoughts on this matter.
Comment 1•11 months ago
|
||
The Origin header is likely set because it is a POST request, set here: https://searchfox.org/mozilla-central/rev/29184ec2b107c8b9dd8c9a594711c27545dfb2c7/netwerk/protocol/http/nsHttpChannel.cpp#10250-10363
This matches the regular behavior on the web, e.g. if you visit example.com and trigger a POST, then the destionation sees example.com in Origin. Here is a snippet to add a POST form via the devtools:
document.body.innerHTML=`<form action=https://httpbingo.org/headers method=post target=_blank><input type=submit value=POST></form>`
The browser.search.query and browser.tabs.create APIs are supposed to simulate user interactions with the browser, so I think that at least in case of navigation requests with isAddonRequest=true, we should not include Origin.
Tom, WDYT?
Comment 2•11 months ago
|
||
What would speak against using a system principal as triggering for the search? I assume that would more closely match how a search triggered by the user e.g. via the address bar would behave.
I am a bit hesitant to touch the Origin header code again because it is quite prone to causing issues.
Comment 3•11 months ago
|
||
(In reply to Tom Schuster (MoCo) from comment #2)
What would speak against using a system principal as triggering for the search? I assume that would more closely match how a search triggered by the user e.g. via the address bar would behave.
A potential downside is that it prevents internal consumers from detecting that the tab was created by an extension. This difference can also be observed externally, e.g. by extensions using the webRequest API (checking the originUrl field in webRequest.onBeforeRequest for example).
In bug 1615901, the fix was to mint a null principal with the same origin attributes as the requester. Since extension contexts can have origin attributes incompatible with the target window (e.g. privateBrowsingId), I don't think that we should pretend to know the desired origin attributes here.
The search URL cannot be set to an arbitrary URL by the extension, so I think that from the security POV is should be fine to unconditionally use the system principal in search API requests.
Mark, do you see objections to unconditionally using a system principal at https://searchfox.org/mozilla-central/rev/29184ec2b107c8b9dd8c9a594711c27545dfb2c7/browser/components/search/SearchUIUtils.sys.mjs#543 ?
Comment 4•11 months ago
|
||
(In reply to Rob Wu [:robwu] from comment #3)
In bug 1615901, the fix was to mint a null principal with the same origin attributes as the requester. Since extension contexts can have origin attributes incompatible with the target window (e.g.
privateBrowsingId), I don't think that we should pretend to know the desired origin attributes here.
Would a null principal without origin attributes work here? We've used that in some other places when loading items.
The search URL cannot be set to an arbitrary URL by the extension, so I think that from the security POV is should be fine to unconditionally use the system principal in
searchAPI requests.
I think, in theory an extension could set a search URL in its manifest, and then get the user to use it for calling the search URL, so if we can use the null principal, I think it would feel better.
Comment 5•11 months ago
|
||
(In reply to Mark Banner (:standard8) from comment #4)
(In reply to Rob Wu [:robwu] from comment #3)
In bug 1615901, the fix was to mint a null principal with the same origin attributes as the requester. Since extension contexts can have origin attributes incompatible with the target window (e.g.
privateBrowsingId), I don't think that we should pretend to know the desired origin attributes here.Would a null principal without origin attributes work here? We've used that in some other places when loading items.
If it works for bug 1615901, then it may work here. It will probably still include "Origin: null" in requests, but if that addresses the reported issue, then it may be good enough.
The search URL cannot be set to an arbitrary URL by the extension, so I think that from the security POV is should be fine to unconditionally use the system principal in
searchAPI requests.I think, in theory an extension could set a search URL in its manifest, and then get the user to use it for calling the search URL, so if we can use the null principal, I think it would feel better.
An extension can only set it to a https:-URL (and localhost/IP addresses. That is fairly restricted. And users would end up navigating there anyway if they enter any search terms, with the system principal.
Comment 6•11 months ago
|
||
I think the Origin problems in extensions were mostly related to fetch(), so maybe changing it for top-level wouldn't be too bad. Or we could make sure to correctly tag the nsILoadInfo for the loads as searches and then don't send the Origin header? We have this triggeringSearchEngine parameter, but it doesn't look like it ends up in nsILoadInfo.
Comment 7•11 months ago
|
||
Is there any situation where we want an origin header to have a full webextension URI? I was under the impression we randomized the IDs and didn't want those to be easily discoverable by web content for privacy reasons. If so, perhaps we need some assertions or similar to ensure we never actually send the webextension URL as an origin header?
Description
•