1292655 - Limit BatteryManager chargingTime/dischargingTime precision

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Tom S [:evilpie]]

These properties seem to be exact to a second and are potential tracking vectors (https://www.theguardian.com/technology/2016/aug/02/battery-status-indicators-tracking-online). We should reduce the precision of those properties similar to what we have done for level. (Bug 1191918)

I think something in the range of 15-30 minutes should still be enough precision to do something useful with that information.

Comment 1

[Tom S [:evilpie]]

However,  we  emphasize  that  the  dischargeTime  levels  can  be  subject  to  frequent  changes,
in  response  to  change  in  the  users'  computer  use  patterns.  This  means  that,  in  practice,  the
risk of long-term tracking with this information may be negligible
- https://eprint.iacr.org/2015/616.pdf

So we might not actually need to this? I still think exposing sub-minute information is probably too much. I prepared a patch that rounds to 15 minutes.

Comment 2

[Tom S [:evilpie]]

Attachment: Round the remainingTime in the BatteryManager to 15 minutes

Comment 3

[Boris Zbarsky [:bzbarsky]]

This could really use some more wide-ranging discussion and review from whoever owns this feature (probably not me).

There's a big difference between "15 minutes remaining" and "1 minute remaining", imo, so maybe it makes sense to round to nearest 15 unless smaller than 15 and then round to nearest 1 or 5 or something.  But it really depends on the intended use cases and attack scenarios....  We really need someone in this discussion who is at least aware of the former.  :(

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8778789 [details] [diff] [review]
Round the remainingTime in the BatteryManager to 15 minutes

Going to punt this to Andrea, who may know something about the goals here....

Comment 5

[Andrea Marchesini [:baku]]

Comment on attachment 8778789 [details] [diff] [review]
Round the remainingTime in the BatteryManager to 15 minutes

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

T

::: dom/battery/BatteryManager.cpp
@@ +151,5 @@
>        mRemainingTime = mCharging ? kDefaultRemainingTime : kUnknownRemainingTime;
> +    } else if (mRemainingTime != kUnknownRemainingTime) {
> +      // Round the remaining time to a multiple of 15 minutes
> +      const double MINUTES_15 = 15.0 * 60.0;
> +      mRemainingTime = lround(mRemainingTime / MINUTES_15) * MINUTES_15;

The only thing I don't like here is that if mRemainingTime < 15.0*60.0 we return 0.
I would like to see a 15.0*60.0 in this case.

Comment 6

[Tom S [:evilpie]]

Attachment: v2 - Round the remainingTime in the BatteryManager to 15 minutes

Thanks for reviewing. Are you sure about 15 minutes?

Comment 7

[Andrea Marchesini [:baku]]

Comment on attachment 8781275 [details] [diff] [review]
v2 - Round the remainingTime in the BatteryManager to 15 minutes

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

Ok. This looks good to me. About 15 minutes, maybe it's a bit too much, but this depends of the quality of the battery and so on.
I'm OK with this value.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0a05115cfe8
Limit BatteryManager chargingTime/dischargingTime precision. r=baku

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 10

[Jean-Yves Perrier [:teoli]]

Updated:
https://developer.mozilla.org/en-US/Firefox/Releases/51#Others
and
https://developer.mozilla.org/en-US/docs/Web/API/BatteryManager/chargingTime
https://developer.mozilla.org/en-US/docs/Web/API/BatteryManager/dischargingTime