Closed Bug 1638259 Opened 1 year ago Closed 1 month ago

Style resources blocked by CSP cannot be displayed in the Network Monitor.

Categories

(DevTools :: Netmonitor, enhancement, P2)

78 Branch
enhancement

Tracking

(Fission Milestone:M7a, firefox89 fixed)

RESOLVED FIXED
89 Branch
Fission Milestone M7a
Tracking Status
firefox89 --- fixed

People

(Reporter: nayinain, Assigned: bomsy)

References

(Blocks 1 open bug)

Details

(Whiteboard: dt-fission-m3-mvp)

Attachments

(7 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

  1. Install the uBlock Origin.(https://addons.mozilla.org/en-US/firefox/addon/ublock-origin/)
  2. Add the rule to my filters in uBlock Origin.
    ||bugzilla.mozilla.org^$csp=style-src 'none'; script-src 'none';
  3. Open Network Monitor in https://bugzilla.mozilla.org/

Actual results:

Cannot see any requests about styles in the Network Monitor.

Expected results:

Style resources blocked by CSP should be displayed in the network monitor.

Has STR: --- → yes
See Also: → 1157817

:nayinain, if you think that's a regression, then could you try to find a regression range in using for example mozregression?

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #1)

:nayinain, if you think that's a regression, then could you try to find a regression range in using for example mozregression?

I don't think that's a regression, because in my impression it has never been shown in the network monitor.

removed duplicate comment

Thanks for the report @nayinain
We landed a patch that likely fixes this issue here Bug 1555057
Can you update and re-test to see if it fixes this issue?

Thanks

Flags: needinfo?(nayinain)

(In reply to Hubert Boma Manilla (:bomsy) from comment #5)

Thanks for the report @nayinain
We landed a patch that likely fixes this issue here Bug 1555057
Can you update and re-test to see if it fixes this issue?

Thanks

Thanks, I tried to retest with the latest Nightly(78.0a1 2020-05-15), but the issue still exists.

Flags: needinfo?(nayinain)

nayinain, are you sure? Just tried in Nightly (78.0a1 (2020-05-15) (64-bit)) and the CSP-blocked JS/CSS requests show up as expected in Network panel.

Flags: needinfo?(nayinain)
Attached file testcase.html
Attached image Screenshots3.png

(In reply to :Harald Kirschner :digitarald from comment #7)

nayinain, are you sure? Just tried in Nightly (78.0a1 (2020-05-15) (64-bit)) and the CSP-blocked JS/CSS requests show up as expected in Network panel.

I'm sure it can 100% reproduce under a new profile.

Flags: needinfo?(nayinain)

@nayinain, thanks for the report and test case!

I can easily reproduce it on my machine

Honza

Severity: -- → S3
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2

I created a test case online (based on the one in comment #8):

STR:

  1. Load http://janodvarko.cz/tests/bugzilla/1638259/
  2. Open DevTools and Select the Network panel
  3. The request for style.css is missing (should be displayed as CSP blocked) => BUG

(no extension involved)

Honza

Junior, this feels like a Necko issue. Should I file another bug in Networking component?

Honz

Flags: needinfo?(juhsu)
Attached file dev.child-2.moz_log

I can reproduce, get the log.
But it looks like necko never open the channel for style.css

Looks like css only? I see we block js and show it in devtool

Flags: needinfo?(juhsu)

(In reply to Junior from comment #13)

Created attachment 9151085 [details]
dev.child-2.moz_log

I can reproduce, get the log.
But it looks like necko never open the channel for style.css

Looks like css only? I see we block js and show it in devtool

Yes, this looks like CSS only to me.
Honza

Christoph, looks like Necko never sees the CSS request that is being blocked here; can you help?

Flags: needinfo?(ckerschb)

Nicolas, last time we chatted about a similar problem for mixed content blocker. You mentioned that we are using the nsIScriptError object to dig out information. Is that the case here too?

Currently in the CSP code we use error->InitWithWindowID for logging to the console which I think should do the trick. We also see the error in the web console, but apparently not in the Network Panel.

Happy to help but unsure what to look out for! If you have any pointers, please let me know!

Flags: needinfo?(ckerschb) → needinfo?(nchevobbe)

So yes, we are probably handling a nsIScriptError as we do get the error on the console.
But, the netmonitor isn't listening to error messages, it only cares about requests.
If we'd like to handle that only in DevTools frontend, that would mean the netmonitor would have to watch for error messages, then filter out the one we're interested in (CSP error for CSS files, not JS ones, because they're already shown), and then either transform into a shape the netmonitor can consume, or make the netmonitor being able to consume errors.

That's quite specific, and I wouldn't go that way.
I would expect that if we get those CSP-blocked requests for JS files, it would probably be better to also get them for CSS files?

Flags: needinfo?(nchevobbe)

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #18)

I would expect that if we get those CSP-blocked requests for JS files, it would probably be better to also get them for CSS files?

Yes, that's my expectation too.

Michal, why we are not seeing requests for CSP blocked CSS files?

Honza

Flags: needinfo?(michal.novotny)

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #19)

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #18)

I would expect that if we get those CSP-blocked requests for JS files, it would probably be better to also get them for CSS files?

Yes, that's my expectation too.

Michal, why we are not seeing requests for CSP blocked CSS files?

I don't know.

Flags: needinfo?(michal.novotny)

Against this test page:

    <meta http-equiv="Content-Security-Policy" content="style-src 'none'; script-src 'none';">
    <link href="style.css" rel="stylesheet" type="text/css">
    <script src="script.js"></script>

We get a http-on-failed-opening-request notification for script.js, but not for style.css.

We typically handle this notification in the netmonitor codebase over here.
By default, we only listen for this notification in the content process, see this code. But I tried also listening from the page's process and we do not get additional notification there.

This notification is typically emitted from here

Using NSPR_LOG_MODULES=nsHttp:5 ./mach run, I can see that script.js is being processed by necko:

[Child 57286: Main Thread]: V/nsHttp HttpChannelChild::AsyncOpen [this=0x7fd4fec44000 uri=http://janodvarko.cz/tests/bugzilla/1638259/script.js]
[Child 57286: Main Thread]: V/nsHttp HttpChannelChild::AsyncOpen failed [this=0x7fd4fec44000 rv=0x805e0006 blocking-reason=4006]
[Child 57286: Main Thread]: D/nsHttp nsHttpHandler::NotifyObservers [chan=0x7fd4fec44070 event="http-on-failed-opening-request"]

But there is no occurence of style.css. So I imagine that the request is blocked before Necko has a chance to event create a Channel?

Then, I'm a bit lost. May be something is done from HTMLinkElement?

I tracked down the web console error message to this CSP code which seems to be mostly exposed via NS_CheckContentLoadPolicy
And there is this code in style loader, which may be the one prevent even doing a request.

It isn't clear what is the best option from here. If we are not creating a Channel, this is probably for a reason. I imagine performance?
So we would have to deal with that and have the netmonitor to report a request without a nsIChannel object.
Having said that, I don't think we should be listening to console message and should probably listen to a more specific platform event.
And eventually create one if necessary.

Could it be that the CSP modules emits event on blocked resources? Does this one call to NS_CheckContentLoadPolicy from layout/style/Loader.cpp emits an event that netmonitor could listen to?

@ckerschbaumer Any idea about that?

Flags: needinfo?(ckerschb)

(In reply to Alexandre Poirot [:ochameau] from comment #21)

But there is no occurence of style.css. So I imagine that the request is blocked before Necko has a chance to event create a Channel?

So, you are right, in the case of stylesheets the platform has not created an nsIChannel at the time we perform that security check. In the case of stylesheets that security check is happening within Loader::CheckContentPolicy.

Please note that there are more content security checks were we don't have a channel yet. So the problem might not limited to stylesheets but to more content types. Searching for SEC_ONLY_FOR_EXPLICIT_CONTENTSEC_CHECK reveals all those places.

In the specific case of CSP I guess devtools networking could listen to the CSP SecurityPolicyViolationEvent but that is specific to CSP and I guess we would have the same problem for mixed content and other security checks, so that's also not an option.

How do we handle inline styles? I guess those don't show up in the Network in the first place. However, we log CSP errors to the console, could we somehow observe those and perform some kind of matching?

Flags: needinfo?(ckerschb)

Valentin, I am not sure if that is feasible or a good idea, but asking anyway. So it seems at the time our security machninery blocks CSS loads within Loader::CheckContentPolicy, we have not created a channel yet. In turn that causes the blocked request not to show up in the Network Panel, because the Network Panel listens for 'http-on-failed-opening-request'.

Would it be an option to add something like the following code to CheckContentPolicy:

if (NS_FAILED(rv) || NS_CP_REJECTED(shouldLoad)) {
nsCOMPtr<nsIChannel> devtoolsChannel;
NS_NewChannelInternal(getter_AddRefs(devtoolsChannel), aTargetURI, secCheckLoadInfo);
RefPtr<nsHttpHandler> handler = nsHttpHandler::GetInstance();
handler->OnFailedOpeningRequest(devtoolsChannel);
return NS_ERROR_CONTENT_BLOCKED;
}

In other words, if we are going to block the load, we create a 'fake' channel just to immediately call OnFailedOpeningRequest? Or are we entering the devils kitchen by doing that?

Flags: needinfo?(valentin.gosu)

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #23)

In other words, if we are going to block the load, we create a 'fake' channel just to immediately call OnFailedOpeningRequest? Or are we entering the devils kitchen by doing that?

I think that could work. We use this trick in other places too (creating fake channel but not opening it)
My only concern is with devtools code handling a channel that hasn't been opened.
For example here it tries to look at a bunch of attributes on the channel - some of these will throw.

Flags: needinfo?(valentin.gosu)

Thanks Christoph for the analysis!

I really like the idea to open a temporary blocked channel in order to trigger standard HTTP observers (the other option, parsing existing CSP errors would be too hacky).

(In reply to Valentin Gosu [:valentin] (he/him) from comment #24)

My only concern is with devtools code handling a channel that hasn't been opened.
For example here it tries to look at a bunch of attributes on the channel - some of these will throw.

We can make the code more safe/ready for some missing attributes on the channel. When there is a test build (with Christoph's patch applied) I can test it with DevTools and see what changes would be needed and/or if there are some unexpected issues.

Honza

Blocks: 1682153
No longer blocks: 1682153
See Also: → 1682153

Not sure if related, but we have a failing test on the console that is checking if we et CSP warning in the console: Bug 1615515

I am going to try to work on this based on the suggested solutions provided above
So to summarize the discussion into some high level steps

  1. When the stylesheet is blocked by the security policy we create a fake channel (which won't be opened)
  2. The OnFailedOpeningRequest is called with the fake channel
  3. In the NetworkObserver (Network Panel) we listen for the http-on-failed-opening-request,
    so we make sure we can handle channels that have not been opened.
Assignee: nobody → hmanilla
Status: NEW → ASSIGNED
Whiteboard: dt-fission-m3-mvp

Tracking dt-fission-m3-mvp bugs for Fission M7 (blocking Beta experiment).

Fission Milestone: --- → M7
Depends on: 1689644

Tracking for Fission M8 milestone (Release experiment).

Fission Milestone: M7 → M8
Pushed by hmanilla@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c2604353843b
Notify devtools when style resources blocked on CSP r=necko-reviewers,valentin,emilio
https://hg.mozilla.org/integration/autoland/rev/2ba855b8d0e9
[devtools] Add test for CSP blocked styles r=nchevobbe
Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 89 Branch
Fission Milestone: M8 → M7a
You need to log in before you can comment on or make changes to this bug.