1233917 - offline cache update service needs to be user context aware

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, defect)

Description

[Dave Huseby [:huseby]]

In the file uriloader/prefetch/nsOfflineCacheUpdateService.cpp there are calls to createCodebasePrincipal that need to be made aware of user context isolation in origin attributes.  Here's the offending code:

> uriloader/prefetch/nsOfflineCacheUpdateService.cpp
>   line 722
>     NS_IMETHODIMP
>     nsOfflineCacheUpdateService::OfflineAppAllowedForURI(nsIURI *aURI,
>                                                          nsIPrefBranch *aPrefBranch,
>                                                          bool *aAllowed)
>     {
>         OriginAttributes attrs;
>         nsCOMPtr<nsIPrincipal> principal =
>             BasePrincipal::CreateCodebasePrincipal(aURI, attrs);
>         return OfflineAppPermForPrincipal(principal, aPrefBranch, false, aAllowed);
>     }
> 
>     nsresult
>     nsOfflineCacheUpdateService::OfflineAppPinnedForURI(nsIURI *aDocumentURI,
>                                                         nsIPrefBranch *aPrefBranch,
>                                                         bool *aPinned)
>     {
>         OriginAttributes attrs;
>         nsCOMPtr<nsIPrincipal> principal =
>             BasePrincipal::CreateCodebasePrincipal(aDocumentURI, attrs);
>         return OfflineAppPermForPrincipal(principal, aPrefBranch, true, aPinned);
>     }

Comment 1

[Dave Huseby [:huseby]]

Jonas, do you know the right way to go on this?  I figured that any caching should be user context isolation, but the offline part makes me think I need to ask around for confirmation.

Comment 2

[Dave Huseby [:huseby]]

Jonas,

So after looking at this bug more, I'm pretty sure the answer is one of two possibilities:
1) have the nsOfflineCacheUpdateService use the nsContentUtils::OfflineAppAllowed() function that takes a principal.  Right now it's calling the one that takes an nsIURI.  I think if we pass in the mLoadingPrincipal instead of the mDocumentURI, we'll get what we want.  But that assumes the mLoadingPrincipal->URI == mDocumentURI.
2) change/add and nsContentUtils::OfflineAppAllowed() function that takes an nsIURI and a PrincipalOriginAttributes object.  That way we can pass in the origin attributes from the mLoadingPrincipal so that the user context ID propagates.

What do you think?

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Please make the code take a principal. In all cases that I can think of, if you want to do security/privacy/capability checks, you should operate on principals, not URIs.

Another question is if this code is still used. Years ago we created the concept of offline capable websites based on localStorage and appcache. But since then we've realized that appcache sucks, and localStorage sync-ness makes it bad for storing large amounts of data. So I don't know if we still have UI which hooks into this code.

Might be worth looking in to.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

It looks like this code is only used by appcache these days. Which is about to be retired. So I'd just leave this as-is to be honest. Seems like at it's just checking things in the permission database anyway, and those aren't segmented by user context.

Most likely appcache will die before this becomes an issue.

Comment 5

[Dave Huseby [:huseby]]

Won't fix per comment 4.

Comment 6

[Tanvi Vyas[:tanvi]]

appcache is b2g specific, right?

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Tanvi Vyas [:tanvi] from comment #6)
> appcache is b2g specific, right?

No, it's a general web platform feature: https://developer.mozilla.org/en-US/docs/Web/HTML/Using_the_application_cache

As the warnings on the page describes, it's been deprecated in favor of Service Workers, and there are plans to eventually remove it (bug 1237782)

Comment 8

[Tanvi Vyas[:tanvi]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #7)
> (In reply to Tanvi Vyas [:tanvi] from comment #6)
> > appcache is b2g specific, right?
> 
> No, it's a general web platform feature:
> https://developer.mozilla.org/en-US/docs/Web/HTML/Using_the_application_cache
> 
> As the warnings on the page describes, it's been deprecated in favor of
> Service Workers, and there are plans to eventually remove it (bug 1237782)

Then does that mean that sites could track users using appcache across user contexts?  Jonas says in comment 4:

> Seems like at it's
> just checking things in the permission database anyway, and those aren't
> segmented by user context.

But I'm not sure what that means.

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Tanvi Vyas [:tanvi] from comment #8)
> (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #7)
> > (In reply to Tanvi Vyas [:tanvi] from comment #6)
> > > appcache is b2g specific, right?
> > 
> > No, it's a general web platform feature:
> > https://developer.mozilla.org/en-US/docs/Web/HTML/Using_the_application_cache
> > 
> > As the warnings on the page describes, it's been deprecated in favor of
> > Service Workers, and there are plans to eventually remove it (bug 1237782)
> 
> Then does that mean that sites could track users using appcache across user
> contexts?  Jonas says in comment 4:
> 
> > Seems like at it's
> > just checking things in the permission database anyway, and those aren't
> > segmented by user context.
> 
> But I'm not sure what that means.

Maybe Jonas can help clarify.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'm going off of recollection here, so if anyone really cares, please do make sure to check my assertions.

As I recall, there's two features involved here.

A) The standard appcache feature, implemented by gecko and all other major browser engines. If we don't make this OriginAttributes-aware, then a website might indeed be able to track the user across userContextIds.

B) The gecko-specific feature to grant certain origin "offline capabilities". Back when this feature was written I believe that we gave the origin the ability to store additional data in localStorage, as well as the ability to use the appcache feature without a prompt.

This features uses the permission database to track which origins have "offline capabilities".

Since then we've changed the localStorage code to no longer grant additional storage for origins which have "offline capabilities". We have also changed appcache to not prompt by default.

However it still seems like the appcache code has check for "offline capabilities", though I have no idea how it changes behavior depending on if the origin has "offline capabilities" or not.

Since we don't separate the permission database based on userContextId, that means that "offline capabilities" is granted in all user contexts. However that does not mean that the website can track the user across user contexts.


Obviously all this is a bit messy. But given that appcache is about to be retired I don't think anyone is really interested in fixing this up.

I don't know if the appcache storage has been made OriginAttributes aware or not. I.e. I don't know A above is a problem. I suggest talking to Honza about this if you really care.

Obviously fixing B would require making permissions be separated by user context. But keep in mind that tracking is not possible even with this unfixed. For further questions I suggest talking to Huseby.