Closed Bug 1369309 Opened 7 years ago Closed 7 years ago

Neutralize the threat of fingerprinting of media statistics when 'privacy.resistFingerprinting' is true

Categories

(Core :: Security, enhancement, P1)

Product:
Core
Component:
Security
Type:
enhancement

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: timhuang, Assigned: timhuang)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fingerprinting][tor][fp:m2])

Attachments

(5 files)

Bug 1369309 - Part 1: Spoofing media statistics to 0 when 'privacy.resistFingerprinting' is true.
59 bytes, text/x-review-board-request
arthur
: review+
jwwang
: review+
Details
Bug 1369309 - Part 2: Add a test case to check whether media statistics has been spoofed correctly when 'privacy.resistFingerprinting' is true.
59 bytes, text/x-review-board-request
arthur
: review+
jwwang
: review+
Details
Bug 1369309 - Part 3: Making the media statistics reports a spoofed value when fingerprinting resistance is enabled.
59 bytes, text/x-review-board-request
arthur
: review+
cpearce
: review+
Details
Bug 1369309 - Part 4: Modify the test case 'test_video_stats_resistfingerprinting.html'.
59 bytes, text/x-review-board-request
arthur
: review+
cpearce
: review+
Details
CNN_-_Breaking_News__Latest_News_and_Videos.jpg
243.26 KB, image/jpeg
Details 
For fingerprinting resistance, we want to disable media statistics when 'privacy.resistfingerprinting' is true. 

We want to provide the same behavior as 'media.video_stats.enabled' = false.
Priority: -- → P1
Rank: 15
Component: Audio/Video → Audio/Video: Playback
Whiteboard: [fingerprinting][tor][fp:m1] → [fingerprinting][tor][fp:m2]
I think a good way to do this is not to disable this API, but to cancel the possibility of fingerprinting when 'privacy.resistFingerprinting' is true. And what 'media.video_stats.enabled' = false do is exactly the way that it makes all statistics report 0.

So what I am going to do is making all statistics report 0 when 'privacy.resistFingerprinting' is true.

Chris, do you have any thoughts regarding this?
Flags: needinfo?(cpearce)
Summary: Disable media statistics when 'privacy.resistFingerprinting' is true → Neutralize the threat of fingerprinting of media statistics when 'privacy.resistFingerprinting' is true
Passing the needinfo to JW.
Flags: needinfo?(cpearce) → needinfo?(jwwang)
SGTM.
Flags: needinfo?(jwwang)
Comment on attachment 8878294 [details]
Bug 1369309 - Part 1: Spoofing media statistics to 0 when 'privacy.resistFingerprinting' is true.

https://reviewboard.mozilla.org/r/149620/#review154246

::: dom/html/HTMLVideoElement.cpp:153
(Diff revision 1)
>  }
>  
>  uint32_t HTMLVideoElement::MozParsedFrames() const
>  {
>    MOZ_ASSERT(NS_IsMainThread(), "Should be on main thread.");
> -  if (!sVideoStatsEnabled) {
> +  if (!sVideoStatsEnabled || nsContentUtils::ShouldResistFingerprinting()) {

Extract !sVideoStatsEnabled || nsContentUtils::ShouldResistFingerprinting()) to a function so we have only one place to change the policy.

::: dom/html/HTMLVideoElement.cpp:190
(Diff revision 1)
>  }
>  
>  double HTMLVideoElement::MozFrameDelay()
>  {
>    MOZ_ASSERT(NS_IsMainThread(), "Should be on main thread.");
> +  if (nsContentUtils::ShouldResistFingerprinting()) {

This should be !sVideoStatsEnabled || nsContentUtils::ShouldResistFingerprinting(), right?
Attachment #8878294 - Flags: review?(jwwang) → review-
Comment on attachment 8878295 [details]
Bug 1369309 - Part 2: Add a test case to check whether media statistics has been spoofed correctly when 'privacy.resistFingerprinting' is true.

https://reviewboard.mozilla.org/r/149622/#review154282

::: browser/components/resistfingerprinting/test/mochitest/test_media_stats.html:13
(Diff revision 1)
> +-->
> +<head>
> +  <meta charset="utf-8">
> +  <title>Test for Bug 1369309</title>
> +  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
> +  <script type="application/javascript" src="/tests/dom/media/test/manifest.js"></script>

I would prefer just put this test under dom/media/test.

::: browser/components/resistfingerprinting/test/mochitest/test_media_stats.html:31
(Diff revision 1)
> +  SimpleTest.waitForExplicitFinish();
> +  var manager = new MediaTestManager;
> +  var testCases = [
> +    { name:"320x240.ogv", type:"video/ogg", width:320, height:240, duration:0.266 },
> +    { name:"seek-short.webm", type:"video/webm", width:320, height:240, duration:0.23 },
> +    { name:"bogus.duh", type:"bogus/duh" }

Why do we want to test a bogus file which has no video to play at all?

::: browser/components/resistfingerprinting/test/mochitest/test_media_stats.html:74
(Diff revision 1)
> +    });
> +  }
> +
> +  function ontimeupdate_RFPDisabled(event) {
> +    var v = event.target;
> +    v.removeEventListener('timeupdate', ontimeupdate_RFPDisabled);

We already have test_VideoPlaybackQuality.html to include the case where privacy.resistFingerprinting=false. We should just set privacy.resistFingerprinting to true and test video stats. So we don't have prefs changing in the middle of test and rely on PARALLEL_TESTS=1 to work.

::: browser/components/resistfingerprinting/test/mochitest/test_media_stats.html:91
(Diff revision 1)
> +    v.loop = true;
> +    manager.started(token);
> +    SpecialPowers.pushPrefEnv({"set": [["privacy.resistFingerprinting", true]]},
> +      function() {
> +        v.play();
> +        v.addEventListener("timeupdate", ontimeupdate_RFPEnabled);

Listening to "timeupdate" is tricky and unreliable. You should listen to the 'ended' event like test_VideoPlaybackQuality.html does.
Attachment #8878295 - Flags: review?(jwwang) → review-
Comment on attachment 8878294 [details]
Bug 1369309 - Part 1: Spoofing media statistics to 0 when 'privacy.resistFingerprinting' is true.

https://reviewboard.mozilla.org/r/149620/#review154374
Attachment #8878294 - Flags: review?(jwwang) → review+
Comment on attachment 8878295 [details]
Bug 1369309 - Part 2: Add a test case to check whether media statistics has been spoofed correctly when 'privacy.resistFingerprinting' is true.

https://reviewboard.mozilla.org/r/149622/#review154378

::: dom/media/test/test_video_stats_resistfingerprinting.html:13
(Diff revision 2)
> +-->
> +<head>
> +  <meta charset="utf-8">
> +  <title>Test for Bug 1369309</title>
> +  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
> +  <script type="application/javascript" src="/tests/dom/media/test/manifest.js"></script>

Just say src="manifest.js".

::: dom/media/test/test_video_stats_resistfingerprinting.html:52
(Diff revision 2)
> +    is(playbackQuality.droppedVideoFrames, 0,
> +        "VideoPlaybackQuality.droppedVideoFrames should be 0 if fingerprinting resistance is enabled");
> +    is(playbackQuality.corruptedVideoFrames, 0,
> +        "VideoPlaybackQuality.corruptedVideoFrames should be 0 if fingerprinting resistance is enabled");
> +
> +    removeNodeAndSource(v);

Move #31, #52 and #53 out of this funciton for they are doing things unrelated to this function.

::: dom/media/test/test_video_stats_resistfingerprinting.html:65
(Diff revision 2)
> +    manager.started(token);
> +    v.addEventListener("ended", checkStats);
> +    v.play();
> +  }
> +
> +  SpecialPowers.pushPrefEnv({"set": [["privacy.resistFingerprinting", true]]},

http://searchfox.org/mozilla-central/rev/20d16dadd336e0c6b97e3e19dc4ff907744b5734/dom/media/test/manifest.js#1594

You can modify gTestPrefs in this test so you don't need SimpleTest.waitForExplicitFinish(); above.
Attachment #8878295 - Flags: review?(jwwang) → review+
Thanks for your review, JW.
Comment on attachment 8878295 [details]
Bug 1369309 - Part 2: Add a test case to check whether media statistics has been spoofed correctly when 'privacy.resistFingerprinting' is true.

https://reviewboard.mozilla.org/r/149622/#review154396

::: dom/media/test/test_video_stats_resistfingerprinting.html:62
(Diff revisions 2 - 3)
>      v.token = token;
>      v.src = test.name;
>      manager.started(token);
> -    v.addEventListener("ended", checkStats);
> +    v.addEventListener("ended", event => {
> +      let v = event.target;
> +      v.removeEventListener("ended", checkStats);

You remove the wrong handler. In addition, you can use the 'once' handler so you don't need to remove it.
Comment on attachment 8878295 [details]
Bug 1369309 - Part 2: Add a test case to check whether media statistics has been spoofed correctly when 'privacy.resistFingerprinting' is true.

https://reviewboard.mozilla.org/r/149622/#review154396

> You remove the wrong handler. In addition, you can use the 'once' handler so you don't need to remove it.

Thanks for pointing this out, and your advice is really helpful.
Comment on attachment 8878295 [details]
Bug 1369309 - Part 2: Add a test case to check whether media statistics has been spoofed correctly when 'privacy.resistFingerprinting' is true.

https://reviewboard.mozilla.org/r/149622/#review156474
Attachment #8878295 - Flags: review?(arthuredelstein) → review+
Comment on attachment 8878294 [details]
Bug 1369309 - Part 1: Spoofing media statistics to 0 when 'privacy.resistFingerprinting' is true.

https://reviewboard.mozilla.org/r/149620/#review156476
Attachment #8878294 - Flags: review?(arthuredelstein) → review+
Sorry I'm late to the party on this...

(In reply to Tim Huang[:timhuang] from comment #1)
> I think a good way to do this is not to disable this API, but to cancel the
> possibility of fingerprinting when 'privacy.resistFingerprinting' is true.
> And what 'media.video_stats.enabled' = false do is exactly the way that it
> makes all statistics report 0.
> 
> So what I am going to do is making all statistics report 0 when
> 'privacy.resistFingerprinting' is true.
> 
> Chris, do you have any thoughts regarding this?

The moz*Frames attributes never made it into an official spec, and never got cross browser support, and were made obsolete by the introduction of the VideoPlaybackQuality API. So we should just remove the moz*Frames attributes.

We will need to send an "Intent to unship" email to mozilla.dev.platform for this change, and update the documentation at https://developer.mozilla.org/en-US/docs/Web/API/HTMLVideoElement , and make sure it's in our release notes.

Similar attributes are exposed in the VideoPlaybackQuality object, so there's a finger printing risk there.

However if we do not report the number of dropped frames, sites like YouTube won't be able to adjust the bitrate of the media they're streaming down, and that would degrade playback experience on lower end machines as they'd be being asked to play media their machines can't handle. So I think you'd be better to either round off VideoPlaybackQuality.droppedVideoFrames or add some kind of noise.

Just reporting 0 for VideoPlaybackQuality.droppedVideoFrames is not a good idea; it will make the UX of video on the web worse.
Depends on: 1379050
I have opened Bug 1379050 for removing the moz*Frames, and I will tackle the issue of VideoPlaybackQuality that Chris mentioned in comment 19.
Because there are some addons are still using moz*Frames API and we can find some usage of this API on the field [1]. So, we might need to keep this moz*Frames API around and find a suitable rounded value instead of 0 for fingerprinting resistance. Does this sound reasonable for you, Chris?

[1] https://github.com/search?utf8=%E2%9C%93&q=mozParsedFrames+extension%3Ajs&type=Code&ref=advsearch&l=&l=
Flags: needinfo?(cpearce)
Or we should leave the work of removing moz*Frames API in Bug 1379050 and focus on neutralizing the fingerprinting issue in this bug.
I think rounding the moz*Frames APIs when fingerprinting resistance is enabled for now is fine. Long term I would still like to remove these attributes.
Flags: needinfo?(cpearce)
I proposed that we should report a fake value which is based on the current play time on media statistics for fingerprinting resistance. That is to say, we report a value this is based on a fixed frames per second, like 30 f/s, and give a fixed dropped frames ratio, 20 percent or something. Let's say a video which has been played for 5 seconds, it should report its media statistics as following values
  * mozParsedFrames    = 150
  * mozDecodedFrames   = 150
  * mozPresentedFrames = 120
  * mozPaintedFrames   = 120
  * mozFrameDelay      = 0  
  * VideoPlaybackQuality.totalVideoFrames     = 150
  * VideoPlaybackQuality.droppedVideoFrames   = 30
  * VideoPlaybackQuality.corruptedVideoFrames = 0

Chris, what do you think about this and what's the reasonable frames per seconds and the dropped ratio should be?

Arthur, does this make sense for fingerprinting resistance?
Flags: needinfo?(cpearce)
Flags: needinfo?(arthuredelstein)
No longer depends on: 1379050
(In reply to Tim Huang[:timhuang] from comment #24)

> Arthur, does this make sense for fingerprinting resistance?

Hi Tim -- it definitely makes sense to me to spoof these values or otherwise report the same results to every content page. However, I worry that a site may have a target (set point) of, say, 5% dropped frames. So if you return a fixed dropped frames ratio of droppedVideoFrames/totalVideoFrames (such as 20%), then the page may respond by continuously lowering its video quality (bitrate) until it reaches zero (or some minimum value).
Flags: needinfo?(arthuredelstein)
(In reply to Arthur Edelstein [:arthuredelstein] from comment #25)
> (In reply to Tim Huang[:timhuang] from comment #24)
> 
> > Arthur, does this make sense for fingerprinting resistance?
> 
> Hi Tim -- it definitely makes sense to me to spoof these values or otherwise
> report the same results to every content page. However, I worry that a site
> may have a target (set point) of, say, 5% dropped frames. So if you return a
> fixed dropped frames ratio of droppedVideoFrames/totalVideoFrames (such as
> 20%), then the page may respond by continuously lowering its video quality
> (bitrate) until it reaches zero (or some minimum value).

Yea, ditto this, 20% seems quite high. 

I think we might want to make this constants pref values.  Not because we actually want to change them or encourage users to change them, but because it's the most expedient way to perform real-world testing of different values (in -central, and when it makes it to release) until we can settle on something that works well and doesn't cause downgrades.
Comment on attachment 8887854 [details]
Bug 1369309 - Part 3: Making the media statistics reports a spoofed value when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/158780/#review167076

Sorry for the delay in reviewing this patch. I'm still nervous about spoofing media statistics because I worry it is complex and will have unintended effects. At least it would be interesting to investigate if we would have more breakage by just returning the value 0 for all of these statistics, or throwing an error when content attempts to access the API. Or is there a more graceful way to refuse to provide the statistics?

::: toolkit/components/resistfingerprinting/nsRFPService.cpp:122
(Diff revision 1)
> +nsRFPService::GetSpoofedDroppedFrames(double aTime)
> +{
> +  // Bound the dropped ratio from 0 to 100.
> +  uint32_t boundedDroppedRatio = min(sVideoDroppedRatio, 100u);
> +
> +  return NSToIntFloor(aTime * sVideoFramesPerSec * (boundedDroppedRatio / 100.0));

Are we leaking a clock at higher resolution than the 100 ms rounding we applied elsewhere in fingerprinting resistance?

::: toolkit/components/resistfingerprinting/nsRFPService.cpp:132
(Diff revision 1)
> +nsRFPService::GetSpoofedPresentedFrames(double aTime)
> +{
> +  // Bound the dropped ratio from 0 to 100.
> +  uint32_t boundedDroppedRatio = min(sVideoDroppedRatio, 100u);
> +
> +  return NSToIntFloor(aTime * sVideoFramesPerSec * ((100 - boundedDroppedRatio) / 100.0));

Here also we should possibly round to nearest 100 ms.
Attachment #8887854 - Flags: review?(arthuredelstein)
Comment on attachment 8887855 [details]
Bug 1369309 - Part 4: Modify the test case 'test_video_stats_resistfingerprinting.html'.

https://reviewboard.mozilla.org/r/158782/#review167078
Attachment #8887855 - Flags: review?(arthuredelstein) → review+
(In reply to Tim Huang[:timhuang] from comment #24)
> I proposed that we should report a fake value which is based on the current
> play time on media statistics for fingerprinting resistance. That is to say,
> we report a value this is based on a fixed frames per second, like 30 f/s,
> and give a fixed dropped frames ratio, 20 percent or something. Let's say a
> video which has been played for 5 seconds, it should report its media
> statistics as following values
>   * mozParsedFrames    = 150
>   * mozDecodedFrames   = 150
>   * mozPresentedFrames = 120
>   * mozPaintedFrames   = 120
>   * mozFrameDelay      = 0  
>   * VideoPlaybackQuality.totalVideoFrames     = 150
>   * VideoPlaybackQuality.droppedVideoFrames   = 30
>   * VideoPlaybackQuality.corruptedVideoFrames = 0
> 
> Chris, what do you think about this and what's the reasonable frames per
> seconds and the dropped ratio should be?

I think that the consequence of reporting a consistent 20% frame drop in VideoPlaybackQuality would be that sites that adapt on the values reported by VideoPlaybackQuality will think that the browser can't keep up, and downgrade their quality to a lower bitrate. The site will then still see frames dropped, and so will downgrade quality again, and so on, until they're on their lowest possible quality.

So we'd end up making the YouTube experience terrible when finger print resistance is enabled with this algorithm.

Is that what you want? i.e. is that an acceptable compromise?

The previous approach of always reporting 0 results in (potentially) too high video quality on low end computers, whereas I think this approach results in too low video quality on high end computers.

Perhaps a better compromise is to assume that anyone savy enough to enable finger print resistance has a machine powerful enough to handle either 720p or 480p video. Most desktop machines < 3 years old can handle decoding this resolution in software. You can check the video resolution with HTMLVideoElement.videoWidth/videoHeight. For videos with resolution > MAX_RESOLUTION you report 10% frame drop. For resolution less than that, you report 0 dropped frames. The exact drop rate I suggest you determine by testing different rates on YouTube. I expect this algorithm would result in videos staying consistently at your MAX_RESOLUTION, i.e. we'd reach an acceptable quality compromise.
Flags: needinfo?(cpearce) → needinfo?(tihuang)
(In reply to Chris Pearce (:cpearce) from comment #33)
> (In reply to Tim Huang[:timhuang] from comment #24)
> > I proposed that we should report a fake value which is based on the current
> > play time on media statistics for fingerprinting resistance. That is to say,
> > we report a value this is based on a fixed frames per second, like 30 f/s,
> > and give a fixed dropped frames ratio, 20 percent or something. Let's say a
> > video which has been played for 5 seconds, it should report its media
> > statistics as following values
> >   * mozParsedFrames    = 150
> >   * mozDecodedFrames   = 150
> >   * mozPresentedFrames = 120
> >   * mozPaintedFrames   = 120
> >   * mozFrameDelay      = 0  
> >   * VideoPlaybackQuality.totalVideoFrames     = 150
> >   * VideoPlaybackQuality.droppedVideoFrames   = 30
> >   * VideoPlaybackQuality.corruptedVideoFrames = 0
> > 
> > Chris, what do you think about this and what's the reasonable frames per
> > seconds and the dropped ratio should be?
> 
> I think that the consequence of reporting a consistent 20% frame drop in
> VideoPlaybackQuality would be that sites that adapt on the values reported
> by VideoPlaybackQuality will think that the browser can't keep up, and
> downgrade their quality to a lower bitrate. The site will then still see
> frames dropped, and so will downgrade quality again, and so on, until
> they're on their lowest possible quality.
> 
> So we'd end up making the YouTube experience terrible when finger print
> resistance is enabled with this algorithm.
> 
> Is that what you want? i.e. is that an acceptable compromise?
> 
> The previous approach of always reporting 0 results in (potentially) too
> high video quality on low end computers, whereas I think this approach
> results in too low video quality on high end computers.
> 
> Perhaps a better compromise is to assume that anyone savy enough to enable
> finger print resistance has a machine powerful enough to handle either 720p
> or 480p video. Most desktop machines < 3 years old can handle decoding this
> resolution in software. You can check the video resolution with
> HTMLVideoElement.videoWidth/videoHeight. For videos with resolution >
> MAX_RESOLUTION you report 10% frame drop. For resolution less than that, you
> report 0 dropped frames. The exact drop rate I suggest you determine by
> testing different rates on YouTube. I expect this algorithm would result in
> videos staying consistently at your MAX_RESOLUTION, i.e. we'd reach an
> acceptable quality compromise.

I think this is a good idea that adjusting the dropped rate according to the video resoultion. And I will take this approach and figure out a reasonable dropped rate. Thanks, Chris.
Flags: needinfo?(tihuang)
Comment on attachment 8887854 [details]
Bug 1369309 - Part 3: Making the media statistics reports a spoofed value when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/158780/#review167132

Clearing the review request for now, as per discussion in bugzilla/irc.

::: dom/html/HTMLVideoElement.cpp:270
(Diff revision 1)
>    DOMHighResTimeStamp creationTime = 0;
>    uint32_t totalFrames = 0;
>    uint32_t droppedFrames = 0;
>    uint32_t corruptedFrames = 0;
>  
> -  if (IsVideoStatsEnabled()) {
> +  if (sVideoStatsEnabled) {

I'd prefer it if you left the accessor function there as it makes it easier to modify the behaviour of this in future, and protects against accidental state changes by writing to the boolean field.
Attachment #8887854 - Flags: review?(cpearce)
Comment on attachment 8887855 [details]
Bug 1369309 - Part 4: Modify the test case 'test_video_stats_resistfingerprinting.html'.

https://reviewboard.mozilla.org/r/158782/#review167608

Clearing review request for now.
Attachment #8887855 - Flags: review?(cpearce)
I have tried several dropped rates for youtube to find a proper rate. Youtube indeed collects the playback quality through HTMLVideoElement.getVideoPlaybackQuality(), however, it seems that Youtube doesn't rely on this to adjust the video quality. I have tried different dropped rates including 100 percent dropped rate, it doesn't change its video quality at all. And according to [1], Youtube adjusts its video quality depending on the player size, the quality of the original source and the network latency, but not the playback quality. 

And I also tried Netflix, the result shows that Netflix will adjust its video quality according to the playback quality. The dropped rate threshold of decreasing video quality is about 53 percent, which is much bigger than I expected. And it doesn't restore its video quality back to the high quality after Firefox reports a zero dropped rate with the lower quality video. Maybe Netflix remembered that the high quality won't work on this machine, so it chose a viable video quality with a low dropped rate instead of changing it back. But, I cannot tell this.

According to aforementioned result, maybe 20 percent dropped rate can work. However, IMO, I think it is still too big for general cases. Therefore, a smaller dropped rate would be more appropriate. I will use a 5 percent dropped rate for now, and I will add a pref for allowing us to change this and test if this rate is a proper rate in the future.
 

[1] https://support.google.com/youtube/answer/91449?hl=en
Chrome has a buggy implementation of frame drop count, so YouTube's devs are of the opinion that it's impossible to rate adjust based on the frame drop count. Google are wrong, as demonstrated by Netflix rate adjusting successfully based on frame drop count.
Comment on attachment 8887854 [details]
Bug 1369309 - Part 3: Making the media statistics reports a spoofed value when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/158780/#review170080

This is OK, but I'll point out that not all videos are 30fps. YouTube has a substantial amount of 60fps content. Unfortunately Gecko's media stack does not expose whether the content is 24/30/60 fps to HTMLMediaElement. You could probably figure out whether the source is 24/30/60 fps by looking at the frame stats, and then round the fps observed in the frame stats to the nearest of 24/30/60 fps, and use that instead of the spoofed 30fps value.

If you don't do this, there's potential for some sites to get confused as the frame counts won't match up with what they expect. I think it's unlikely it will cause a problem today based on my understanding of how frame stats are used by players today, but that could change in future as 60fps content becomes more common.

::: toolkit/components/resistfingerprinting/nsRFPService.cpp:124
(Diff revision 2)
> +
> +/* static */
> +uint32_t
> +nsRFPService::GetSpoofedDroppedFrames(double aTime, uint32_t aWidth, uint32_t aHeight)
> +{
> +  uint32_t targetRes = sTargetVideoRes * NSToIntCeil(sTargetVideoRes * 16 / 9.0);

Maybe you want to factor out the video resolution check into a single helper function, rather than repeating it twice.
Attachment #8887854 - Flags: review?(cpearce) → review+
twitch.tv is also a good site to test; they'll have a bunch of variable length content, including 48 fps.
Comment on attachment 8887855 [details]
Bug 1369309 - Part 4: Modify the test case 'test_video_stats_resistfingerprinting.html'.

https://reviewboard.mozilla.org/r/158782/#review170082
Attachment #8887855 - Flags: review?(cpearce) → review+
Comment on attachment 8887854 [details]
Bug 1369309 - Part 3: Making the media statistics reports a spoofed value when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/158780/#review171342

::: toolkit/components/resistfingerprinting/nsRFPService.cpp:232
(Diff revisions 1 - 2)
> +      // PR_SetEnv() needs the input string been leaked intentionally, so
> +      // we copy it here.
> +      tz = ToNewCString(tzValue);
> +      if (tz) {
> +        PR_SetEnv(tz);
> +      }

This part of the patch seems to be unrelated to this ticket?
Attachment #8887854 - Flags: review?(arthuredelstein) → review+
Comment on attachment 8887854 [details]
Bug 1369309 - Part 3: Making the media statistics reports a spoofed value when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/158780/#review171342

> This part of the patch seems to be unrelated to this ticket?

This part of code is not a part of my patch. It is here because I rebased the codebase. Other patches did some modify here, so their changes been displayed in the inter-diff.
(In reply to Chris Pearce (:cpearce) from comment #43)
> Comment on attachment 8887854 [details]
> Bug 1369309 - Part 3: Making the media statistics reports a spoofed value
> when fingerprinting resistance is enabled.
> 
> https://reviewboard.mozilla.org/r/158780/#review170080
> 
> This is OK, but I'll point out that not all videos are 30fps. YouTube has a
> substantial amount of 60fps content. Unfortunately Gecko's media stack does
> not expose whether the content is 24/30/60 fps to HTMLMediaElement. You
> could probably figure out whether the source is 24/30/60 fps by looking at
> the frame stats, and then round the fps observed in the frame stats to the
> nearest of 24/30/60 fps, and use that instead of the spoofed 30fps value.
> 

I will file a follow-up bug for this, thanks.

> If you don't do this, there's potential for some sites to get confused as
> the frame counts won't match up with what they expect. I think it's unlikely
> it will cause a problem today based on my understanding of how frame stats
> are used by players today, but that could change in future as 60fps content
> becomes more common.
>
Flags: needinfo?(tihuang)
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
Rebase m-c, and try looks good.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b09c43969b90
Flags: needinfo?(tihuang)
Keywords: checkin-needed
Flags: needinfo?(tihuang)
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/81f871657e18
Part 1: Spoofing media statistics to 0 when 'privacy.resistFingerprinting' is true. r=jwwang, r=arthuredelstein
https://hg.mozilla.org/integration/autoland/rev/23efa91adf8a
Part 2: Add a test case to check whether media statistics has been spoofed correctly when 'privacy.resistFingerprinting' is true. r=jwwang, r=arthuredelstein
https://hg.mozilla.org/integration/autoland/rev/9afd71d7438c
Part 3: Making the media statistics reports a spoofed value when fingerprinting resistance is enabled. r=cpearce, r=arthuredelstein
https://hg.mozilla.org/integration/autoland/rev/45ec9d0af14b
Part 4: Modify the test case 'test_video_stats_resistfingerprinting.html'. r=cpearce, r=arthuredelstein
Keywords: checkin-needed
Glad to see this was landed. :D
Tim, thank you for the great work!
Arthur, Chris, and JW, thanks for your review!
Verified on Mac OS 10.12.6 with Nightly 58.0a1 (2017-10-25) (64-bit)

Verification steps:
1. Set parameter privacy.resistFingerprinting to true
2. Open new tab and navigate to cnn.com
3. Find an id of any video element on the page
4. Open Console, and execute this command: document.getElementById('id')

Expected results:
Media statistics should be set to zero.

Actual results:
Media statistics is *not* set to zero:
mozAudioCaptured: false
mozAutoplayEnabled: true
mozDecodedFrames: 603
mozFragmentEnd: 4.17959
mozFrameDelay: 0
mozHasAudio: true
mozPaintedFrames: 603
mozParsedFrames: 603
mozPresentedFrames: 603
mozPreservesPitch: true
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
See data returned after turning on anti-fingerprinting
Also this additional verification step:
* In Console, type: document.getElementById('id').getVideoPlaybackQuality()

It returns this:
VideoPlaybackQuality
corruptedVideoFrames: 0
creationTime: 614300
droppedVideoFrames: 0
totalVideoFrames: 17775

It should return all zeros when anti-fingerprinting is enabled.
The protection here is not to make all media statistics into zero, but making the media statistics to be only related to the play time of the media. The protection will give a fixed frames per second, 30 fps, and a zero drop rate when the video source is less than or equal to 480p. For 720p or greater resolution, we will still use the 30 fps but with a 5 percent drop rate. See comment 33, comment 34 and comment 37 for more details.
So this is in fact an expected behavior.

Tom, maybe we can adjust the corresponding test cases in the test plan for this as well? Thanks.

(In reply to Tim Huang[:timhuang] from comment #67)
> The protection here is not to make all media statistics into zero, but
> making the media statistics to be only related to the play time of the
> media. The protection will give a fixed frames per second, 30 fps, and a
> zero drop rate when the video source is less than or equal to 480p. For 720p
> or greater resolution, we will still use the 30 fps but with a 5 percent
> drop rate. See comment 33, comment 34 and comment 37 for more details.
Flags: needinfo?(tgrabowski)
Let's move to Security component. :-)
Component: Audio/Video: Playback → Security
Thanks for pointing out these comments Tim.  As Wesley suggested, I updated our test cases for this in the TestRail. Next time we ever execute this test, we will know what to expect.
Flags: needinfo?(tgrabowski)
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Flags: needinfo?(tihuang)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: