1157817 - Show requests blocked by security (e.g. CORS), content blocking (e.g. tracking protection), etc

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P2)

Description

[Jeff Griffiths (:canuckistani) (:⚡︎)]

We added CORS messages to the console in bug 1121824 - yay! I think we should also represent requests that were blocked by CORS or CSP in the netmonitor - as a user maintaining a site that makes requests to other services, I am looking for failed resource loads in the network tool first - this is where 404s and 502s end up, it should also show other sorts of loading failures suchs as CORS or CSP violations.

Comment 1

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

Mixed content (from bug 1241548) is another variant of this.

Comment 3

[Vangelis Katsikaros]

I tried to dig a bit into this bug and I have some questions:

a) In what form would the requests blocked would be presented to the user? In Chrome such requests are shown in red font and the status can be something like "(blocked:mixed-content)"

b) I tried to get information regarding security problems from the `request` object. I understand that `request.securityState` can be "secure" or "insecure", with the later meaning some kind of security problem. 

I used as example target https://googlesamples.github.io/web-fundamentals/fundamentals/security/prevent-mixed-content/active-mixed-content.html and I naively console.log'ed 

> --- a/devtools/client/netmonitor/src/components/request-list-column-domain.js
> +++ b/devtools/client/netmonitor/src/components/request-list-column-domain.js
> @@ -41,7 +41,11 @@ const RequestListColumnDomain = createCl
>      let iconTitle;
>      let title = host + (remoteAddress ?
>        ` (${getFormattedIPAndPort(remoteAddress, remotePort)})` : "");
> -
> +if( item.securityState === "insecure" ) {
> +  console.log("url" + JSON.stringify(item.url, null, 4));
> +  console.log("securityInfo" + JSON.stringify(item.securityInfo, null, 4));
> +  console.log("securityState" + JSON.stringify(item.securityState, null, 4));
> +}
>      if (isLocal) {
>        iconClassList.push("security-state-local");
>        iconTitle = L10N.getStr("netmonitor.security.state.secure");

but I only get

```
url: "http://googlesamples.github.io/web-fundamentals/samples/discovery-and-distribution/avoid-mixed-content/puppy-thumb.jpg"
securityInfo: {
    "state": "insecure"
}
securityState: "insecure"
```

So, I would like to ask how I could grab more useful information to show?

Comment 4

[Vangelis Katsikaros]

Attachment: chrome_blocked.png

Comment 5

[Tim Nguyen :ntim]

It seems like we currently don't show blocked requests in the network monitor, but they do appear in the console though. So I think the first step would be to start showing them.

As for the information, to know whether a request is mixed content, just compare security state of the request with the security state of the document. If the document is HTTPS and if the request is HTTP, then we have mixed content. The main challenge here is to get the blocked requests to show up (the image in comment 3 is mixed content, but isn't actually blocked: only mixed content scripts and sensitive requests are blocked).

Comment 6

[Vangelis Katsikaros]

(In reply to Tim Nguyen :ntim from comment #5)
> It seems like we currently don't show blocked requests in the network
> monitor, but they do appear in the console though. So I think the first step
> would be to start showing them.
> 
> As for the information, to know whether a request is mixed content, just
> compare security state of the request with the security state of the
> document. 

Tim, I was wondering if it makes sense for to implement something to specifically check for mixed content or I should focus on consuming all the kinds of "errors" that the console shows as well.

I will try the later as it's needed anyway and will see how it goes.

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Vangelis Katsikaros from comment #6)
> (In reply to Tim Nguyen :ntim from comment #5)
> > It seems like we currently don't show blocked requests in the network
> > monitor, but they do appear in the console though. So I think the first step
> > would be to start showing them.
> > 
> > As for the information, to know whether a request is mixed content, just
> > compare security state of the request with the security state of the
> > document. 
> 
> Tim, I was wondering if it makes sense for to implement something to
> specifically check for mixed content or I should focus on consuming all the
> kinds of "errors" that the console shows as well.
> 
> I will try the later as it's needed anyway and will see how it goes.

Any progress on this or do we need new API from the necko team?

Honza

Comment 8

[Vangelis Katsikaros]

I got a bit sidettracked with this one and I don't have any progress to show. From looking at the issue, I couldn't spot something I could hook to consume all the kinds of "errors" that the console shows, but I might be wrong. RE the new API is I am not sure how to determine if it's needed or not.

I am happy both to pass the ticket to someone else interested/able to finish this quicker, or continue the work with some guidance on next steps.

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@Jason: the requirement in this bug is to mark blocked requests (due to CORS/CSP/Mixed content) and render them differently from the rest (e.g. using red text color, see the attached screenshot).

It looks like we need new platform API that would allow us to recognize that particular request was actually blocked in the end. Currently they are processed just like any other request at the Necko level.

How difficult it would be to implement such API?
Is necko (e.g. nsIHTTPChannel) the right place to provide such API?
Or are there some API we could use already available?

Note that the platform is already generating errors (displayed in the Console panel atm) informing the user about blocked requests. But there is no back link to the actual request object.

Honza

Comment 10

[:Harald Kirschner :digitarald]

Adding to the questions:

Can we get a reason for blocking (e.g. tracking protection, specific CSP directive, ad-blocking extension id, etc)

Comment 11

[:Harald Kirschner :digitarald]

Also nice to have:

Blocked resources should have a stack trace like other resources for debugging.

Comment 12

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@michal: please see comment #9 Do you have some input for that?
Or is there anyone else I could/should ask?

Thanks!
Honza

Comment 13

[Michal Novotny [:michal]]

(In reply to Jan Honza Odvarko [:Honza] from comment #12)
> @michal: please see comment #9 Do you have some input for that?
> Or is there anyone else I could/should ask?

I don't know much about request blocking. I guess all we do is that we call nsContentSecurityManager::doContentSecurityCheck() in nsHttpChannel::AsyncOpen2() and it either fails or succeeds. We don't get any detailed information so if it's enough nsHttpChannel might be the right place. Otherwise the notification should come probably somewhere from nsContentPolicy. Maybe Honza could give a better answer?

Comment 14

[Honza Bambas (:mayhemer)]

We can't use nsIRequest.status.  Looking at the mixed content blocker, it's just a generic nsIContentPolicy implementation that simply REJECTs requests.  Those are then cancelled with some CSP specific error (I think BAD_URI) that is common for different reasons of rejection.

Looking closed at the nsIContentPolicy interface, apparently it doesn'w work with nsIChannel/nsIRequets interface.  But, nsIContentPolicy takes the load info.


Hence, we need a new API and it should be added on nsILoadInfo, which is specific to each channel (1-1 relation)

Comment 15

[:Harald Kirschner :digitarald]

Do the same constrains apply to showing requests that are in progress? If they are close, I would expand the scope of the bug

Comment 16

[Jason Duell]

Sounds like we may want to split this bug up into a necko part (new API with info) and a devtools part.  

Honza tells me that if comment 11 is correct the necko part shouldn't be more than a few days work (for Honza or someone he delegates to).  Now I'm wondering if comment 12 expands the scope (but don't understand it well enuf to know!) :)

Comment 17

[Honza Bambas (:mayhemer)]

(In reply to Jason Duell [:jduell] (Regression Engineering Owner for 64; needinfo me) from comment #16)
> Sounds like we may want to split this bug up into a necko part (new API with
> info) and a devtools part.  
> 
> Honza tells me that if comment 11 

comment 14? (sorry if I sent a wrong ref)

> is correct the necko part shouldn't be
> more than a few days work (for Honza or someone he delegates to).  Now I'm
> wondering if comment 12

comment 15?

> expands the scope (but don't understand it well enuf
> to know!) :)


(In reply to :Harald Kirschner :digitarald from comment #15)
> Do the same constrains apply to showing requests that are in progress? If
> they are close, I would expand the scope of the bug

can you specify what "constrains" you mean?  Note that most of the info on a request is shown in devtools when the response is done.

Comment 18

[:Harald Kirschner :digitarald]

> Note that most of the info on a request is shown in devtools when the response is done.

The goal would be to show request data as soon as they are known (visually identified as "running") and then fill in the response as it becomes available.

In case of blocked resources, we would show the request when it gets created and visually mark it as "blocked" (hopefully with some meta data on the reason).

Comment 19

[Honza Bambas (:mayhemer)]

(In reply to :Harald Kirschner :digitarald from comment #18)
> > Note that most of the info on a request is shown in devtools when the response is done.
> 
> The goal would be to show request data as soon as they are known (visually
> identified as "running") and then fill in the response as it becomes
> available.
> 
> In case of blocked resources, we would show the request when it gets created
> and visually mark it as "blocked" (hopefully with some meta data on the
> reason).

That's way beyond the scope of this bug and can be done separately.  For the original report (and as the title states) I can see two bugs: necko to expose the reason on nsILoadInfo and a devtools bug to show it.  The rest is for a separate followup.  True is that we were somewhat using the advantage to show stuff after the channel was done, so it might be not a small task to show things in-progres with deep granularity.  But all is definitely doable.

Comment 20

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Honza Bambas (:mayhemer) from comment #19)
> That's way beyond the scope of this bug and can be done separately.
> original report (and as the title states) I can see two bugs: necko to
> expose the reason on nsILoadInfo
I filled bug 1493599 for the Necko part.

> and a devtools bug to show it.
Let's use this bug report for the work that is needed for the Devtools part.

> The rest is for a separate followup. True is that we were somewhat using the advantage
> to show stuff after the channel was done, so it might be not a small task to
> show things in-progres with deep granularity. But all is definitely doable.
@mayhemer: can you please file a bug (or more) as needed for this part?

Thanks!
Honza

Comment 21

[Honza Bambas (:mayhemer)]

(In reply to Jan Honza Odvarko [:Honza] from comment #20)
> (In reply to Honza Bambas (:mayhemer) from comment #19)
> > That's way beyond the scope of this bug and can be done separately.
> > original report (and as the title states) I can see two bugs: necko to
> > expose the reason on nsILoadInfo
> I filled bug 1493599 for the Necko part.

Just took it.

> > The rest is for a separate followup. True is that we were somewhat using the advantage
> > to show stuff after the channel was done, so it might be not a small task to
> > show things in-progres with deep granularity. But all is definitely doable.
> @mayhemer: can you please file a bug (or more) as needed for this part?

I'll rather leave it to Harald or you to file.  You should state your needs and then we can discuss them and eventually, if necessary, split to necko/devtools bugs as needed.

> 
> Thanks!
> Honza

Comment 22

[:Harald Kirschner :digitarald]

Filed bug 1493786.

Comment 25

[Jan Honza Odvarko [:Honza] (always need-info? me)]

David, The platform API are close to landing (the patch in bug 1493599 is already working) and we can start working on the DevTools/Network monitor side.

Some comments & pointers:

1. Another Bug 1151368 introduced recently a new (related) feature allowing users to manually block specific URLs. Those blocked URLs are rendered in red in the UI. The patch is close to the code base we'll be changing in this bug and it's worth to read.

2. Here is the place where we use "blocked" CSS class for manually blocked resources.
   https://searchfox.org/mozilla-central/rev/d143f8ce30d1bcfee7a1227c27bf876a85f8cede/devtools/client/netmonitor/src/components/RequestListItem.js#210

• Manually blocked resources are rendered in red (in the Network panel).
• Resources that are blocked by the platform will use the same CSS styling.

3. Here is the place where we display the 'blocked reason':
   https://searchfox.org/mozilla-central/rev/d143f8ce30d1bcfee7a1227c27bf876a85f8cede/devtools/client/netmonitor/src/components/RequestListColumnTransferredSize.js#47

• The label is rendered in the Transferred column
• The column is represented by RequestListColumnTransferredSize component

4. The existing code base has NetworkObserver._httpResponseExaminer handler registered for "http-on-examine-response", "http-on-examine-cached-response" and "http-on-modify-request" events
   https://searchfox.org/mozilla-central/rev/d143f8ce30d1bcfee7a1227c27bf876a85f8cede/devtools/server/actors/network-monitor/network-observer.js#256

• This handler is responsible for creating (NetworkObserver._createNetworkEvent) and sending network event packet to the client.
• _createNetworkEvent function also sets the blockingReason in the packet. In case of manual blocking the reason is always "DevTools". The blockingReason field is used for both: mark a request as blocked and explain why.

5. Bug 1493599 (the platform API for blocked resources by the platform) introduces a new callback NetworkObserver._httpFailedOpening in devtools/server/actors/network-monitor/network-observer.js

• This callback is registered for new "http-on-failed-opening-request" event
• If this event is fired none of the following events happens: "http-on-examine-response", "http-on-examine-cached-response" and "http-on-modify-request"
• This means that NetworkObserver._httpFailedOpening needs to ensure that network events packet are created and sent to the client (just like it happens for the other events). I think that it should somehow directly call _httpResponseExaminer that calls _createNetworkEvent that sets the blockingReason. Obviously the blocking reason needs to use the right value in case of resources blocked by the platform.
• Perhaps we could use _httpResponseExaminer to directly handle the "http-on-failed-opening-request" event.

6. As soon as the backend sends the right blockingReason - the front-end can just handle that and start working without big changes.

Honza

Comment 26

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@Alex, bug 1493599 introduced a new notification event "http-on-failed-opening-request". You can see an existing (so far empty) _httpFailedOpening handler in DevTools codebase:

https://searchfox.org/mozilla-central/rev/b756e6d00728dda4121f8278a744381d8643317a/devtools/server/actors/network-monitor/network-observer.js#257

The event is fired (and handled) in content process (unlike the other events "http-on-examine-response", "http-on-examine-cached-response" and "http-on-modify-request"). It's because HTTP channel is aborted directly in the content process and it can't even make it to the parent.

The goal for _httpFailedOpening is similar as for _httpResponseExaminer (this one lives in the parent) - create NetworkEventActor and send it to the client (since we want to display blocked requests too).

I believe that we can do something like as follows directly in the content process (in _httpFailedOpening handler).

this._createNetworkEvent(channel, {
  blockedReason: channel.loadInfo.requestBlockingReason + ""
});

There are no other events (after the channel is aborted) and no other data we'd like to collect, so we might event avoid registering the channel in open responses map this.openResponses.set(channel, response);

Does that make sense?
WDYT?

Honza

Comment 27

[Jan Honza Odvarko [:Honza] (always need-info? me)]

One more thing:

• The list of all possible reasons a request may have been blocked is defined here:
  https://searchfox.org/mozilla-central/rev/99a2a5a955960b0e58ceade1db1f7652d9db4ba1/netwerk/base/nsILoadInfo.idl#1058

• We need to create a localized string for every reason that will be displayed in the Net panel UI

• The list of localized strings for Net panel is here:
  https://searchfox.org/mozilla-central/rev/99a2a5a955960b0e58ceade1db1f7652d9db4ba1/devtools/client/locales/en-US/netmonitor.properties#5

Honza

Comment 28

[David Walsh :davidwalsh]

Attachment: Bug 1157817 - Show blocked requests in the Network Monitor r=honza

Displays blocked requests in the Network monitor request listing, providing a reason for why the request was blocked based on response codes provided b nsILoadInfo.idl

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b067a5b108dd
Show blocked requests in the Network Monitor r=Honza,Harald

Comment 30

[Frederik Braun [:freddy] (reo duty for Fx124)]

This is great. Is there a goal to follow-up with showing requests that are blocked by an extension and show which extension blocked the request?

Comment 31

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Frederik Braun [:freddyb] from comment #30)

This is great. Is there a goal to follow-up with showing requests that are blocked by an extension and show which extension blocked the request?

Frederik, please create a new report for this + a test case and we'll look at that, thanks!
Honza

Comment 32

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Ah, it's already reported as bug 1555057
Honza

Comment 33

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/b067a5b108dd

Comment 34

[David Walsh :davidwalsh]

@Honza Reopening as Harald and I noticed tracking URLs and CORS either displaying as a successful request or not displaying at all. Could we talk about this Monday, i.e. the process relationships? i.e. how to get those requests and statuses from the content process? i.e. why we aren't receiving these blocked reasons: https://searchfox.org/mozilla-central/source/netwerk/url-classifier/UrlClassifierFeatureFactory.cpp#265

Comment 35

[Honza Bambas (:mayhemer)]

(sorry - misplaced for different bug :))

Comment 36

[:Harald Kirschner :digitarald]

UX concluded in https://github.com/firefox-devtools/ux/issues/81#issuecomment-516623049 .

Victoria, do you have an existing reference or file for the icon to use?

Comment 37

[Victoria Wang [:victoria]]

Attachment: blocked.svg

Here's the icon (a slightly smaller version of :fvsch's icon). Should be colored Red 60, and left-align with the status text.

Comment 38

[:Harald Kirschner :digitarald]

Thanks a lot! Pulled into a new bug 1570819.