1285476 - AppCache can be used to bypass CORS

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Anne (:annevk)]

See https://github.com/whatwg/html/issues/1427 for details and a potential solution (double caching).

This is a security issue, but it's made public already.

Comment 1

[Honza Bambas (:mayhemer)]

The test case confirms we suffer from the issue.  On reload of the evil site (from [1]) we load the anonymous xhr request from appcache.  Appcache doesn't recognize the 'anonymous' flag.  It means we serve non-anonymously loaded content to anonymous requests.  That is bad.

However, there used to be an issue from times we were honoring the anonymity flag - bug 687758.  Honoring it will break some sites again, but I personally don't care at all.  Appcache is a dead technology.

[1] https://github.com/jakearchibald/appcache-credentials

Comment 2

[Honza Bambas (:mayhemer)]

Attachment: v1 (hard-forbid serving from appcache to anon requests)

Comment 3

[Honza Bambas (:mayhemer)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a3f1c1fd561

Comment 4

[Anne (:annevk)]

Note that an alternative that should not break sites is double requests and double caching for each entry in AppCache. Not ideal obviously.

Comment 5

[Honza Bambas (:mayhemer)]

(In reply to Anne (:annevk) from comment #4)
> Note that an alternative that should not break sites is double requests and
> double caching for each entry in AppCache. Not ideal obviously.

I know.  But I don't want to allocate resources for that right now.  OTOH, I'm spending recently quite a lot of time on appcache already, so that I may fix this along...

Comment 6

[Honza Bambas (:mayhemer)]

Attachment: v2 (double caching)

Mutually exclusive with the v1 patch.  This is a roughly manually tested stuff.

Comment 7

[Honza Bambas (:mayhemer)]

Comment on attachment 8775149 [details] [diff] [review]
v1 (hard-forbid serving from appcache to anon requests)

Doesn't make sense to deal with this patch.

Comment 8

[Honza Bambas (:mayhemer)]

Comment on attachment 8775295 [details] [diff] [review]
v2 (double caching)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7f78406a117

Michal, feel free to pass to Jason, he has some experience with the uriloader parts.

Comment 9

[Michal Novotny [:michal]]

Comment on attachment 8775295 [details] [diff] [review]
v2 (double caching)

Jason will be better reviewer for this.

Comment 10

[Jason Duell]

Comment on attachment 8775295 [details] [diff] [review]
v2 (double caching)

Review of attachment 8775295 [details] [diff] [review]:
-----------------------------------------------------------------

> This is a roughly manually tested stuff.

Patch makes sense to me.  Can we test it any better manually before we ship it?

Comment 11

[Honza Bambas (:mayhemer)]

(In reply to Jason Duell [:jduell] (needinfo me) from comment #10)
> Comment on attachment 8775295 [details] [diff] [review]
> v2 (double caching)
> 
> Review of attachment 8775295 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> > This is a roughly manually tested stuff.
> 
> Patch makes sense to me.  Can we test it any better manually before we ship
> it?

Do you want me to block on this before do more testing here?  I did of course a check this does what's expected and anon resources load and also that the test case from https://github.com/jakearchibald/appcache-credentials no longer show the symptoms.

IMHO, this enough to go.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e56fc8ee868
Let appcache double-cache resources that are cross origin, r=michal

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6e56fc8ee868

Comment 14

[Jason Duell]

> IMHO, this enough to go.

Yes, that's fine