1357386 - Gather telemetry for toplevel data: URI loads

Status: RESOLVED FIXED

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

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1357386_telemetry_data_toplevel.patch

I am happy to modify the patch if we can think of any other information we could gather, but I think that's pretty much the best we can do, right?

Comment 2

[François Marier [:francois]]

Comment on attachment 8859505 [details] [diff] [review]
bug_1357386_telemetry_data_toplevel.patch

Review of attachment 8859505 [details] [diff] [review]:
-----------------------------------------------------------------

datareview+

Comment 3

[Daniel Veditz [:dveditz]]

Comment on attachment 8859505 [details] [diff] [review]
bug_1357386_telemetry_data_toplevel.patch

Review of attachment 8859505 [details] [diff] [review]:
-----------------------------------------------------------------

Is this all the information we need? I suspect we're going to find out that compared to the number of documents and frames loaded data: urls are a very small fraction. What then? Are there types of data documents we can carve out to get a better sense of what we might break? My first thought was to count sandboxed data: documents because clearly we can't break those, but since such documents would be essentially sandboxed in Chrome already maybe web devs wouldn't bother to add that attribute.

I'd like to know what fraction of top-level data: documents are from navigations/links vs manual entry or bookmark. Whether or not there's a referrer could be a proxy for that, although Referrer Policy and/or user prefs can mess that up. Is there an easy way to check the "real" referrer from that point in DocShell? If not I could live with the simpler method and just hope Referrer Policy isn't used much to completely blank the referrer.

Would prefer some descriptive #define or const for the buckets rather than raw numbers, though not sure where you'd stick them that would make much sense. If we ever added buckets that we counted elsewhere it would be even more important.

r- mainly for not counting "developer" use of data: vs "web" encounters -- I think that's important to help us judge scale.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1357386_telemetry_data_toplevel.patch

Dan, thanks for your feedback. I flipped the patch around entirely. In fact we are only interested in top level data: URI loads and whether they are navigated or typed into the URL Bar (by a developer or also come from a bookmark).

This patch relies on the triggeringPrincipal (which in most cases is identical to the referrer but also agnostic to the referrer policy). Whenever the a toplevel page is navigated then the triggeringPricnipal is a codebasePrincipal. When typing a data URL into the address bar, the triggeringPrincipal is a systemPrincipal. Similar when clicking a bookmark, then the triggeringPrincipal is a systemPrincipal.

I think that should provide exactly the information we are seeking for: what breakage can we expect if we disallow navigations to data: URIs for toplevel loads.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/59567c6c1e71
Have loadTabs() tests provide the correct triggeringPrincipal. r=gijs,kit
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b92af4575ae
Have loadTabs() provide the correct triggeringPrincipal. r=gijs,smaug

Comment 6

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c928f0602185
Backed out changeset 2b92af4575ae 
https://hg.mozilla.org/integration/mozilla-inbound/rev/c51a2eaa0078
Backed out changeset 59567c6c1e71 for landing with wrong bugnumber

Comment 7

[Daniel Veditz [:dveditz]]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #4)
> In fact we are only interested in top level data: URI loads and whether they are
> navigated or typed into the URL Bar (by a developer or also come from a bookmark).

Why aren't we interested in frames also? I would guess (based on nothing!) that data: in frames is more common than navigating or window.open()ing a data: document. 

The sad thing is that none of these measures tells us what we really want to know: how much breakage would we cause by making data: urls not inherit. We might count tons of data: documents but have them all turn out to be perfectly fine without inheritance because they were written for all the other browsers.

We have a pref that flips the behavior so we could do a shield or pref-flipping experiment -- except we need a way to measure the effects if we do so. Where's a good place for that? Maybe the cross-compartment wrapper access check? If it fails check to see if the target or source compartment is a data: url. By doing it on the failure path we should avoid perf impacts (but of course we should measure that, too, as part of the test). Some access failures might be normal, but we could compare the rates from the test groups vs the global population.

The drawback to that is that a small sample study is likely to miss the specific cases that might break: "Firefox-required" internal apps for particular enterprises. I guess they could just flip the pref as part of their roll-out.

Comment 8

[François Marier [:francois]]

(In reply to Daniel Veditz [:dveditz] from comment #7)
> We have a pref that flips the behavior so we could do a shield or
> pref-flipping experiment -- except we need a way to measure the effects if
> we do so. Where's a good place for that?

Maybe we could start by testing it out in Nightly? Again, it won't tell us about the release population, but it's cheap and it might surface things we haven't thought about.

Comment 9

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Daniel Veditz [:dveditz] from comment #7)
> Why aren't we interested in frames also? I would guess (based on nothing!)
> that data: in frames is more common than navigating or window.open()ing a
> data: document. 

It seems there is some confusion on what this bug is about - let me clarify: For this bug we want to measure breakage if we 'block data: URIs in top level windows' (See Bug 1331351). This measurements have nothing to do with the inheritance change of data: URIs we are doing within Bug	1324406. To this end, I don't think we are interested in (i)frame loads, only in top level loads. Ultimately we would like to distinguish how many developers type 'data:' in the URL bar to test something, we don't want to block those, but we want to figure out how many top level windows get navigated to a data: URI - those we want to measure and ultimately block.

Comment 10

[Daniel Veditz [:dveditz]]

Ah. This bug is blocking both bug 1324406 and bug 1331351 so I thought it was trying to gather telemetry for both. We already have bug 1302399 for that, I guess. Can we remove bug 1324406 from the blocked list here then? Thanks for the clarification.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 8870383 [details] [diff] [review]
bug_1357386_telemetry_data_toplevel.patch

Review of attachment 8870383 [details] [diff] [review]:
-----------------------------------------------------------------

If this telemetry is only for bug 1357386 then this is fine. r=dveditz

Comment 12

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Daniel Veditz [:dveditz] from comment #10)
> Can we remove bug 1324406 from the blocked list here then?
> Thanks for the clarification.

Ah, that caused the confusion -> Reoving Bug 1324406 from the blocklist!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/96875dd613f8
Gather telemetry for toplevel data: URI loads. r=dveditz,francois

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/96875dd613f8