EME: PersistentState should be disabled in private browsing mode

RESOLVED FIXED in Firefox 55

Status

()

defect
P3
normal
RESOLVED FIXED
2 years ago
10 months ago

People

(Reporter: karl.gallagher, Assigned: cpearce)

Tracking

({privacy})

51 Branch
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [fingerprinting])

Attachments

(4 attachments)

Reporter

Description

2 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/601.7.8 (KHTML, like Gecko) Version/9.1.3 Safari/601.7.8

Steps to reproduce:

Initiated playback of Widevine protected content and setting persistentState option when requesting MediaKeySystemAcess


Actual results:

The DRM server sends a token (provider_client_token) along with the content license. This is persisted by the browser CDM implementation.  If I open a new private browsing tab I notice that cookies/history ...etc is cleared however if I playback the same protected media content the server logs show that the provider_client_token has persisted.




Expected results:

Since the provider_client_token is typically used to track devices, it should not be persisted in private browsing mode.   In essence, support for persistentState should be disabled in this mode.
CC'ing jdm and ehsan for private browsing's sake.

:cpearce, is this still your ballpark?
Group: firefox-core-security → core-security
Component: Untriaged → Audio/Video: GMP
Flags: needinfo?(cpearce)
Product: Firefox → Core
Tanvi: do we also need to segregate this persistent id (or prevent it) by origin attribute?
Group: core-security → core-security-release
Flags: needinfo?(tanvi)
Keywords: privacy, sec-low
Whiteboard: [fingerprinting]
When in private browsing mode, all persistent state is persisted in memory instead of on disk, and we clear the state when we exit private browsing mode.

Karl: do you see the client token being persisted across private-browsing being exited?
Flags: needinfo?(cpearce)
(In reply to Daniel Veditz [:dveditz] from comment #2)
> Tanvi: do we also need to segregate this persistent id (or prevent it) by
> origin attribute?

This already happens.
Flags: needinfo?(tanvi)
In fact, we should be clearing the temporary storage cache when all same-origin connections to the CDM instance have shutdown. 

So provided you close all same-origin tabs using the CDM before you start up the new tab using the CDM, we should clear the CDM data.
Reporter

Comment 6

2 years ago
(In reply to Chris Pearce (:cpearce) from comment #3)
> When in private browsing mode, all persistent state is persisted in memory
> instead of on disk, and we clear the state when we exit private browsing
> mode.
> 
> Karl: do you see the client token being persisted across private-browsing
> being exited?


Hi Chris.

I retested and I can confirm that the persistent data does not 'bleed' in or out of private browsing mode and also that when the connection to the origin is shutdown (first tab closed), it is cleared.

However, You explanation does raise some other issues for me.

 - using volatile storage to implement EME persistentState seems anachronistic, as stated I think it would be better to disable this feature in private browsing mode. This seems to be in keeping with the recommendations in Section 11.7 of the specification (and with the Chrome implementation)

- temporarily storing data that service providers expect to be persistent (or pseudo-persistent), can cause issues with backend implementations  - e.g increased device 'churn'
Reporter

Comment 7

2 years ago
Hi,

Is this something that you guys are considering changing?

Also, if no longer a security concern can this be made public?  We have customers (service operators), who would like to have a public explanation of the current behaviour to reference.

Thanks
Karl.
Ni for comment 6 / comment 7. :-)
Flags: needinfo?(cpearce)
(In reply to karl.gallagher from comment #6)
> (In reply to Chris Pearce (:cpearce) from comment #3)
> > When in private browsing mode, all persistent state is persisted in memory
> > instead of on disk, and we clear the state when we exit private browsing
> > mode.
> > 
> > Karl: do you see the client token being persisted across private-browsing
> > being exited?
> 
> 
> Hi Chris.
> 
> I retested and I can confirm that the persistent data does not 'bleed' in or
> out of private browsing mode and also that when the connection to the origin
> is shutdown (first tab closed), it is cleared.
> 
> However, You explanation does raise some other issues for me.
> 
>  - using volatile storage to implement EME persistentState seems
> anachronistic, as stated I think it would be better to disable this feature
> in private browsing mode. This seems to be in keeping with the
> recommendations in Section 11.7 of the specification (and with the Chrome
> implementation)


In general, my understanding of the intent of our Private Browsing mode was to try to make it as hard as possible for a site to figure out that the user is in Private Browsing mode. It is however possible; I know of at least one way to detect Private Browsing mode. So rejecting persistent state explicitly while the user is in Private Browsing mode would run counter to our intent with Private Browsing mode.

> - temporarily storing data that service providers expect to be persistent
> (or pseudo-persistent), can cause issues with backend implementations  - e.g
> increased device 'churn'

I have found limits on numbers of devices to be a particularly unpleasant feature of DRM, so I am sympathetic to this point.

I will follow up with our Private Browsing mode people to see whether we wish to do something here.
Flags: needinfo?(cpearce)

Comment 10

2 years ago
I think it's OK to change our behavior here from a PB standpoint.
I'll change our behaviour so that we reject navigator.requestMediaKeySystemAccess requests for persistent storage while we're in private browsing mode.

I don't think this bug needs to be a security bug.
Group: core-security-release
Keywords: sec-low
Assignee

Updated

2 years ago
Assignee: nobody → cpearce
Priority: -- → P3
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 16

2 years ago
mozreview-review
Comment on attachment 8872206 [details]
Bug 1334111 - Reject MediaKeys requests for persistent storage when in PB mode.

https://reviewboard.mozilla.org/r/143668/#review147396
Attachment #8872206 - Flags: review?(gsquelart) → review+

Comment 17

2 years ago
mozreview-review
Comment on attachment 8872207 [details]
Bug 1334111 - Move MediaKeySystemConfig to-string helper to MediaKeySystemAccess.

https://reviewboard.mozilla.org/r/143670/#review147398
Attachment #8872207 - Flags: review?(gsquelart) → review+

Comment 18

2 years ago
mozreview-review
Comment on attachment 8872208 [details]
Bug 1334111 - Log MediaKeySystemConfiguration on creation of MediaKeySystemAccess.

https://reviewboard.mozilla.org/r/143672/#review147400
Attachment #8872208 - Flags: review?(gsquelart) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 23

2 years ago
mozreview-review
Comment on attachment 8872209 [details]
Bug 1334111 - Test for EME persistentState being rejected in Private Browsing mode.

https://reviewboard.mozilla.org/r/143674/#review147548

::: browser/base/content/test/plugins/browser_private_browsing_eme_persistent_state.js:18
(Diff revision 2)
> +  let tab = win.gBrowser.selectedTab = win.gBrowser.addTab(TEST_URL);
> +  await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
> +  await promiseWaitForFocus(win);
> +
> +  let browser = gBrowser.getBrowserForTab(tab);

I think you can replace this with:

```js
let tab = await BrowserTestUtils.openNewForegroundTab(win.gBrowser, TEST_URL);
```

and then pass `tab.linkedBrowser` to the ContentTask.spawn code.

::: browser/base/content/test/plugins/browser_private_browsing_eme_persistent_state.js:43
(Diff revision 2)
> +  let result = await isEmePersistentStateSupported(true);
> +  is(result, false, "EME persistentState should *NOT* be supported in private browsing window.");
> +
> +  result = await isEmePersistentStateSupported(false);
> +  is(result, true, "EME persistentState *SHOULD* be supported in non private browsing window.");

This reads a bit odd, you're checking that isFooSupported(true) == false, and then the reverse.

How about passing the `{private: <bool>}` object as the argument? That will make this easier to read.

You can also omit the intermediate `result` var, ending up with:

```js
is(await isEmePersistentStateSupported({private: true}), false,
   "EME ... ");
```
Attachment #8872209 - Flags: review?(gijskruitbosch+bugs) → review+
Comment hidden (mozreview-request)

Comment 25

2 years ago
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/411fa6abd4f3
Reject MediaKeys requests for persistent storage when in PB mode. r=gerald
https://hg.mozilla.org/integration/autoland/rev/a5f9fa64adc6
Move MediaKeySystemConfig to-string helper to MediaKeySystemAccess. r=gerald
https://hg.mozilla.org/integration/autoland/rev/8f1b6e2e4883
Log MediaKeySystemConfiguration on creation of MediaKeySystemAccess. r=gerald
https://hg.mozilla.org/integration/autoland/rev/99fcc9cc3d16
Test for EME persistentState being rejected in Private Browsing mode. r=Gijs

Updated

10 months ago
See Also: → 1447085

Updated

10 months ago
See Also: → 653826
You need to log in before you can comment on or make changes to this bug.