Open Bug 1353685 Opened 8 years ago Updated 2 years ago

Should ServiceWorker call SetBlockedRequest

Categories

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

enhancement

Tracking

()

Tracking Status
firefox55 --- affected

People

(Reporter: allstars.chh, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [domsecurity-backlog1])

In Bug 1267075 we have a patch to prevent calling SetBlockedRequest from ServiceWorker. In this bug we could re-think how ServiceWorker should handle image blocking. see https://bugzilla.mozilla.org/show_bug.cgi?id=1267075#c38 from bz "Maybe what we should do is simply set a boolean member while calling into imagelib to indicate that we're in that call. When that boolean is not set, ignore calls to SetBlockedRequest. And when that boolean _is_ set, ignore or throw for calls to LoadImage, since it means some extension is being evil and reentering us. Then file a followup for how we want to handle async stuff like service workers."
I think this is a perf thing?
(In reply to Ben Kelly [not reviewing due to deadline][:bkelly] from comment #1) > I think this is a perf thing? not exactly, ServiceWorker will call CSP check in http://searchfox.org/mozilla-central/source/dom/workers/ServiceWorkerEvents.cpp#254 Now in Bug 1267075 we will call SetBlockedRequest if CSP check failed, that means now SetBlockedRequest could be triggered by ServiceWorker, which might have problems if we didn't think it carefully. For example I met some problem related to this in https://bugzilla.mozilla.org/show_bug.cgi?id=1267075#c10.
I guess I have no idea what SetBlockedRequest is or what it means. Without that description and the impact on end user UI/script its hard for me to answer this. Can you summarize what the impact is from bug 1267075? What will be changing for service worker?
Flags: needinfo?(allstars.chh)
See comment 1 for the context. In Bug 1267075 we've decided to call SetBlockedRequest if NS_CheckContentLoadPolicy failed.(Part 1 patch), however SetBlockedRequest is originally designed to be called synchrously from image loader code, now it could be called from anywhere calling Content Polciy check, for example from ServiceWorker. And in Bug 1267075 Part 4 I added a work-around to prevent SetBlockedRequest is called by SW, so in this bug we need to think about how to make calling SetBlockedRequest correctly from ServiceWorker.
Flags: needinfo?(allstars.chh)
So what happens from the users perspective if an image is blocked by NS_CheckContentLoadPolicy() via a service worker that is different from if its blocked in the image loader? I get that we're not calling this method, but I don't understand what it does in terms of user observable behavior.
Flags: needinfo?(allstars.chh)
SetBlockedRequest sets image state that is then used to implement the :-moz-suppressed, :-moz-broken, :-moz-user-disabled, etc pseudo-classes. That is, it communicates to the image _why_ it was blocked. We then render it differently based on the reason it was blocked (not rendering at all, rendering with a little placeholder icon, etc).
So in a sane world, this sort of information would be stored in the error Response and then imagelib could surface it to the DOM image code or something. Unfortunately, Necko has no concept of Response right now, and our content policy checks are a slightly unholy mix of sync and async checks, with the sync case definitely not having a useful concept of Response because we just fail out from AsyncOpen2...
If SetBlockedRequest or the information representing it was set on the nsIChannel, then we could propagate it through the InternalResponse properly.
Flags: needinfo?(allstars.chh)
Priority: -- → P3
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.