Add telemetry for the enablePrivilege pref

RESOLVED FIXED in Firefox 17

Status

()

Core
Security: CAPS
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mccr8, Assigned: froydnj)

Tracking

unspecified
mozilla18
Points:
---
Bug Flags:
sec-review +

Firefox Tracking Flags

(firefox17+ fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
I don't know if this is a good idea or not securitywise (quantifying how many people are exposed...) but froydnj says it would be easy to do if we think it is a good idea.
(Assignee)

Comment 1

5 years ago
Created attachment 660043 [details] [diff] [review]
patch

Here's the patch to add telemetry on whether we've called enablePrivilege at all.
I don't know whether it would be worth trying to find out how many times it had been called; it sounds like we only care about whether enablePrivilege has been called.

This would be fairly trivial to uplift if we wanted some idea of how big of an effect enablePrivilege changes would have.
Attachment #660043 - Flags: review?(bobbyholley+bmo)
Comment on attachment 660043 [details] [diff] [review]
patch

This looks right, though we need someone from security to make the call on whether we want to quantify this information. Flagging dveditz as a catch-all, but feel free to re-assign as appropriate.

Is there any way to make telemetry data private?

For this to be useful, we'll want to backport it to 17.
Attachment #660043 - Flags: review?(dveditz)
Attachment #660043 - Flags: review?(bobbyholley+bmo)
Attachment #660043 - Flags: review+
Comment on attachment 660043 [details] [diff] [review]
patch

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

I'm not sure the patch matches the original request.

mccr8 seems to be wanting to know "How many users have made themselves Pwnable"

the patch is measuring "How often are users running content that required them to turn on the pref", which is a subset of the question mccr8 was asking. This subset corresponds roughly to "How many will get made when we remove this entirely" so it is a useful number, but it's not quite the original "how unsafe are our users after bug 757046?"

There's a third question that would be interesting to know: "How much web content /attempts/ to use enablePrivilege whether or not users are vulnerable?".

Aside from questions of correctness--and that I find all 3 questions interesting to know, but especially the two you aren't answering ;-) -- I have no security or privacy concerns with this patch. Bug 757046 on the other hand...
> corresponds roughly to "How many will get made ..."

"How many will get MAD ..."
I think you wanted more of a sec-review+ than a r+ from me based on comment 2 so I'll set that instead and clear the patch r?
Flags: sec-review+
Attachment #660043 - Flags: review?(dveditz)
tracking-firefox17: --- → +
If something like the current patch landed on beta/aurora prior to the CAPS-ectomy then the accumulate in netscape_security_enablePrivilege would be measuring question 3: how many users encounter web content that calls enablePrivilege.

The only way to answer that question after bholley's changes would be to always hook up the netscape.security.enablePrivilege method and then check the pref each time it's called before doing something with it. That's probably not worth it, especially if we can get a rough answer to 3 by uplifting the probe to Aurora and maybe Beta ASAP.

For the uplift probe it'd be great to get a second flag accumulator after the  securityManager->EnableCapability(cap.ptr()); call if it succeeded. That way we'll know how much web content calls it and how many users agree to it. The first number is how much trouble users will be in if they flip the dire pref, and the second number might hint at how many users are likely to do so.
(In reply to Daniel Veditz [:dveditz] from comment #6)
> If something like the current patch landed on beta/aurora prior to the
> CAPS-ectomy then the accumulate in netscape_security_enablePrivilege would
> be measuring question 3: how many users encounter web content that calls
> enablePrivilege.
> 
> The only way to answer that question after bholley's changes would be to
> always hook up the netscape.security.enablePrivilege method and then check
> the pref each time it's called before doing something with it. That's
> probably not worth it, especially if we can get a rough answer to 3 by
> uplifting the probe to Aurora and maybe Beta ASAP.

It'd need to be beta, because the CAPS-ectomy is already on aurora.
(Assignee)

Comment 8

5 years ago
(In reply to Daniel Veditz [:dveditz] from comment #6)
> For the uplift probe it'd be great to get a second flag accumulator after
> the  securityManager->EnableCapability(cap.ptr()); call if it succeeded.
> That way we'll know how much web content calls it and how many users agree
> to it. The first number is how much trouble users will be in if they flip
> the dire pref, and the second number might hint at how many users are likely
> to do so.

Given comment 7, is this still applicable?
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/703fb179c834
Assignee: nobody → nfroyd
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/703fb179c834
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
(Assignee)

Comment 11

5 years ago
Comment on attachment 660043 [details] [diff] [review]
patch

We'd like to uplift this to Aurora to get more information via Telemetry on how users are affected by the enablePrivilege removal.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): None
User impact if declined: None
Testing completed (on m-c, etc.): On m-c for a few days.
Risk to taking this patch (and alternatives if risky): Minimal risk.
String or UUID changes made by this patch: None.
Attachment #660043 - Flags: approval-mozilla-aurora?

Updated

5 years ago
Attachment #660043 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 12

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/c51952868cb5
(Assignee)

Comment 13

5 years ago
I'm not doing a version of this patch for beta unless/until I hear something addressing comment 6 and/or comment 7.

Updated

5 years ago
status-firefox17: --- → fixed
You need to log in before you can comment on or make changes to this bug.