Closed Bug 1157817 Opened 5 years ago Closed 4 months ago

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

Categories

(DevTools :: Netmonitor, enhancement, P2)

enhancement

Tracking

(firefox69 affected)

RESOLVED FIXED
Firefox 69
Tracking Status
firefox69 --- affected

People

(Reporter: canuckistani, Assigned: davidwalsh)

References

(Blocks 8 open bugs)

Details

Attachments

(3 files)

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.
Mixed content (from bug 1241548) is another variant of this.
Summary: Show requests blocked by CORS → Show requests blocked by CORS, mixed content, etc.
Duplicate of this bug: 1241548
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?
Flags: needinfo?(odvarko)
Attached image chrome_blocked.png
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).
(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.
Assignee: nobody → vkatsikaros
(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
Flags: needinfo?(odvarko) → needinfo?(vkatsikaros)
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.
Flags: needinfo?(vkatsikaros)
Assignee: vkatsikaros → nobody
Product: Firefox → DevTools
@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
Flags: needinfo?(jduell.mcbugs)
Severity: normal → enhancement
Priority: P3 → P2
Summary: Show requests blocked by CORS, mixed content, etc. → Show requests blocked by security (e.g. CORS), content blocking (e.g. tracking protection), etc
Version: 38 Branch → Trunk
Adding to the questions:

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

Blocked resources should have a stack trace like other resources for debugging.
@michal: please see comment #9 Do you have some input for that?
Or is there anyone else I could/should ask?

Thanks!
Honza
Flags: needinfo?(michal.novotny)
(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?
Flags: needinfo?(michal.novotny) → needinfo?(honzab.moz)
Flags: needinfo?(jduell.mcbugs)
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)
Flags: needinfo?(honzab.moz)
Do the same constrains apply to showing requests that are in progress? If they are close, I would expand the scope of the bug
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!) :)
Flags: needinfo?(honzab.moz)
(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.
Flags: needinfo?(honzab.moz)
> 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).
(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.
(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
Depends on: 1493599
Flags: needinfo?(honzab.moz)
(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
Flags: needinfo?(honzab.moz) → needinfo?(odvarko)
Filed bug 1493786.
Flags: needinfo?(odvarko)
See Also: → 1493786
Duplicate of this bug: 1511942
Duplicate of this bug: 1265691

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.
  1. 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
  1. 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.
  1. 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.
  1. As soon as the backend sends the right blockingReason - the front-end can just handle that and start working without big changes.

Honza

Assignee: nobody → dwalsh

@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

Flags: needinfo?(poirot.alex)

One more thing:

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

Depends on: 1553026
Blocks: 1553026
No longer depends on: 1553026
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b067a5b108dd
Show blocked requests in the Network Monitor r=Honza,Harald

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?

Depends on: 1555057
Blocks: 1555057
No longer depends on: 1555057

(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

Flags: needinfo?(fbraun)
Flags: needinfo?(poirot.alex)

Ah, it's already reported as bug 1555057
Honza

Flags: needinfo?(fbraun)
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69

@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

Status: RESOLVED → REOPENED
Flags: needinfo?(odvarko)
Resolution: FIXED → ---

(sorry - misplaced for different bug :))

Status: REOPENED → RESOLVED
Closed: 4 months ago4 months ago
Resolution: --- → FIXED
Flags: needinfo?(odvarko)
Blocks: 1556451

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?

Flags: needinfo?(victoria)
Attached image 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.

Flags: needinfo?(victoria)

Thanks a lot! Pulled into a new bug 1570819.

You need to log in before you can comment on or make changes to this bug.