Open
Bug 912202
Opened 11 years ago
Updated 2 years ago
Unify site-specific and third party permission across all forms of local storage
Categories
(Core :: Storage: IndexedDB, defect, P3)
Tracking
()
NEW
People
(Reporter: akligman, Unassigned)
References
Details
(Keywords: privacy)
Attachments
(1 file, 4 obsolete files)
The current policy of blocking all access to IndexedDB from third-party iframes puts excessive limits on composability of web apps. Denying access by default is fine, as long as there is a way for the user to add exceptions for third-party domains that they trust.
Related: #595307
Depends on: 595307
Comment 1•11 years ago
|
||
Sanboxed iframes with allow-same-origin, specifically, should be allowed or an alternative standard mechanism proposed. the disadvantage of the later is that the former is per spec and implemented elsewhere
Comment 2•11 years ago
|
||
This restriction is also causing my team (Orion) problems as we use iframes to let third parties contribute functionality back to our browser-based IDE. In particular this prevents our plugin authors from providing some basic offline support on FF only.
We're using sandboxed iframes and could also do something on the iframe side if some sort of trust handshake was needed.
Reporter | ||
Comment 3•11 years ago
|
||
Having an option in about:permissions to control this would be nice.
Reporter | ||
Updated•11 years ago
|
Flags: needinfo?(mmc)
Comment 4•11 years ago
|
||
I would welcome unification of site-specific permissions and third-party permissions across all forms of local storage. The current site-specific permissions are a mess, and it's very confusing for developers to have cookie and indexdb requests treated differently.
Jonas, what do you think?
Flags: needinfo?(mmc) → needinfo?(jonas)
Agreed! Would you be able to work on this, or know of anyone who would? If so we could get together an come up with a comprehensive design.
Flags: needinfo?(jonas)
Comment 6•11 years ago
|
||
Ok, good :) I wouldn't be able to work on this anytime soon, but we should come up with a plan anyway -- I may be able to recruit someone who has capacity.
Updated•11 years ago
|
Summary: Allow IndexedDB in third-party iframes → Unify site-specific and third party permission across all forms of local storage
Reporter | ||
Comment 7•11 years ago
|
||
In the short-term, can we enable indexeddb in third-party iframes when third-party cookies are enabled?
I don't actually. Ben Turner does these days.
But yes, I agree that we should enable IndexedDB in cross-origin iframes when third-party cookies are enabled.
Flags: needinfo?(jonas) → needinfo?(bent.mozilla)
We're all in basic agreement that all forms of temporary storage should behave the same as cookies. ack is going to take a look here.
Flags: needinfo?(bent.mozilla)
Reporter | ||
Comment 12•10 years ago
|
||
To clarify, we want allow IndexedDB for a fist-party domain even when cookies are disabled?
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(jonas)
Reporter | ||
Comment 13•10 years ago
|
||
Added a check for cookie permissions when testing third-partyness. Check is essentially the same as the one for third-party cookies.
(Ignore the whitespace changes.)
Attachment #8472295 -
Flags: feedback?(jonas)
Attachment #8472295 -
Flags: feedback?(bent.mozilla)
Reporter | ||
Comment 14•10 years ago
|
||
The changes to CookieServiceChild are trivial, but the patch seems to work. Not sure what needs to be done there (WIP).
Comment on attachment 8472295 [details] [diff] [review]
idb-third-party-iframes.patch
Review of attachment 8472295 [details] [diff] [review]:
-----------------------------------------------------------------
::: netwerk/cookie/CookieServiceChild.cpp
@@ +213,5 @@
> +CookieServiceChild::CheckCookiesAllowed(nsIURI *aHostURI,
> + nsIChannel *aChannel,
> + bool *result)
> +{
> + return NS_OK;
This doesn't seem right. Note that FirefoxOS uses child processes a lot and we want to make sure that this bug is fixed there too.
::: netwerk/cookie/nsCookieService.cpp
@@ +1638,5 @@
>
> NS_IMETHODIMP
> +nsCookieService::CheckCookiesAllowed(nsIURI *aHostURI,
> + nsIChannel *aChannel,
> + bool *result)
Someone other than me needs to look at this.
Attachment #8472295 -
Flags: feedback?(jonas) → feedback+
I think that if cookies are completely disabled, we should also disable IndexedDB. Eventually we should enable IndexedDB in the persistent storage area even when cookies are disabled, but let's not worry about that for now.
Flags: needinfo?(jonas)
Comment on attachment 8472295 [details] [diff] [review]
idb-third-party-iframes.patch
Review of attachment 8472295 [details] [diff] [review]:
-----------------------------------------------------------------
This looks generally fine, but I agree with sicking that the child process impl has to work too.
Also, your editor has made this patch hard to follow with all the unrelated whitespace changes. You'll want to revert those before getting a full review.
::: dom/base/nsGlobalWindow.cpp
@@ +10639,5 @@
> bool isThirdParty;
> aError = thirdPartyUtil->IsThirdPartyWindow(this, nullptr,
> &isThirdParty);
> +
> + if (isThirdParty) {
You're not checking aError.Failed() here, 'isThirdParty' could be uninitialized.
@@ +10644,5 @@
> + if (!codebaseURI) {
> + // Document's principal is not a codebase (may be system), so
> + // can't set cookies
> +
> + aError.Throw(NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
Previously we didn't throw but just made the return value null. I'd keep that approach.
@@ +10648,5 @@
> + aError.Throw(NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
> + return nullptr;
> + }
> +
> + // not having a cookie service isn't an error
Really? In what cases would we not have a cookie service?
@@ +10650,5 @@
> + }
> +
> + // not having a cookie service isn't an error
> + nsCOMPtr<nsICookieService> service = do_GetService(NS_COOKIESERVICE_CONTRACTID);
> + nsIURI* documentURI = GetDocument()->GetDocumentURI();
What happens if documentURI is null?
::: netwerk/cookie/nsCookieService.cpp
@@ +1638,5 @@
>
> NS_IMETHODIMP
> +nsCookieService::CheckCookiesAllowed(nsIURI *aHostURI,
> + nsIChannel *aChannel,
> + bool *result)
I'd suggest someone in network, jduell or jdm perhaps
Attachment #8472295 -
Flags: feedback?(bent.mozilla) → feedback+
Reporter | ||
Comment 18•10 years ago
|
||
Removed whitespace changes from patch.
Assignee: nobody → akligman
Attachment #8472295 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Reporter | ||
Comment 19•10 years ago
|
||
Attachment #8479317 -
Attachment is obsolete: true
Reporter | ||
Comment 20•10 years ago
|
||
Attachment #8485369 -
Attachment is obsolete: true
Reporter | ||
Comment 21•10 years ago
|
||
Attachment #8501961 -
Attachment is obsolete: true
Pretty sure Alan has moved on.
Assignee: akligman → nobody
Status: ASSIGNED → NEW
Comment 23•9 years ago
|
||
Still interested in seeing this fixed, but I no longer have time to do the work.
Updated•7 years ago
|
Priority: -- → P3
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•