1260234 - Cached CSP within appcache prevents updates of the appcache

Status: RESOLVED INACTIVE

(Core :: DOM: Security, defect, P3)

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

> Summary of email discussion:

Anyway, the other thing that changed recently is that we converted nsOfflineCacheUpdateItem::OpenChannel to use channel->AsyncOpen2() instead of AsyncOpen() [1]. This means that before FF43 no security checks were performed when loading "https://web.whatsapp.com/wa.appcache" but after that change that load became subject to security checks. In a later patch within FF44 we updated the loadingPrincipal [2] for such loads (the loadingPrincipal holds the CSP of the loading Document) and all of a sudden that load became subject to CSP. For that load we are using contentPolicyType 1 (TYPE_OTHER) which maps to default-src. Obviously using default-src 'none' would now block that load.

When visiting https://web.whatsapp.com now I see you are using |default-src 'self'| instead of |default-src 'none'| which should fix the problem in my opinion.

[1] https://hg.mozilla.org/mozilla-central/rev/31332b421bf8#l1.73
[2] https://hg.mozilla.org/integration/mozilla-inbound/rev/cf9e1eb325c8#l10.118

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

> Summarized Email question:
> So, where from do we take the CSP header we base the sec checks on?  I presume it's the top level document that is coming from the appcache.

That is correct.

> If this CSP violation can be easily detected with a certain error code, we could just instruct nsOfflineManifestItem (or whatever better place) to discard the currently active cache and restart the update.

Alternatively we could pass a custom contentPolicyType which will be discarded by CSP.

Comment 3

[Andrew Overholt [:overholt]]

Out of curiosity, is this a regression?

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Andrew Overholt [:overholt] from comment #3)
> Out of curiosity, is this a regression?

Two point of views:
a) After landing Bug 1199295 such loads have become subject to CSP. One could see that as a regression.
b) such loads have not been governed by CSP but should have been prior to landing Bug 1199295. One could see that as a web compatibility issue.

Comment 5

[titob]

Well I just came to report this bug that also affects my website. 

As the appcache is blocked, the site is not updated, and I cannot even update the CSP, because lol, is cached. 

Congrats, my website is broken forever in Firefox unless users use the "Refresh my Firefox".

Comment 6

[Andrew Overholt [:overholt]]

Christoph, given comment 5, is P3 (which I'm assuming mean backlog) appropriate? Not trying to second guess you, just curious about the reasoning :)

Comment 7

[Honza Bambas (:mayhemer)]

(Don't invest resources to appcache unless it's a critical issue, which this IMO is not)

Comment 8

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Andrew Overholt [:overholt] from comment #6)
> Christoph, given comment 5, is P3 (which I'm assuming mean backlog)
> appropriate? Not trying to second guess you, just curious about the
> reasoning :)

When triaging those bugs we decided we shouldn't put any more resources on appcache issues. If you think it's important to get fixed, I am happy to re-evaluate and get this one fixed. Just let me know.

Comment 9

[Andrew Overholt [:overholt]]

(In reply to Honza Bambas (:mayhemer) from comment #7)
> (Don't invest resources to appcache unless it's a critical issue, which this
> IMO is not)

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #8)
> (In reply to Andrew Overholt [:overholt] from comment #6)
> > Christoph, given comment 5, is P3 (which I'm assuming mean backlog)
> > appropriate? Not trying to second guess you, just curious about the
> > reasoning :)
> 
> When triaging those bugs we decided we shouldn't put any more resources on
> appcache issues. If you think it's important to get fixed, I am happy to
> re-evaluate and get this one fixed. Just let me know.

Not putting time into appcache works for me as long as we're not easily letting people get stranded on old cached versions. Thanks!

Comment 10

[Valentin Gosu [:valentin] (he/him)]

Appcache was removed.