Closed Bug 1334111 Opened 7 years ago Closed 7 years ago

EME: PersistentState should be disabled in private browsing mode

Categories

(Core :: Audio/Video: GMP, defect, P3)

Product:
Core
Component:
Audio/Video: GMP
Version:
51 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: karl.gallagher, Assigned: cpearce)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

(Keywords: privacy, Whiteboard: [fingerprinting])

Attachments

(4 files)

Bug 1334111 - Reject MediaKeys requests for persistent storage when in PB mode.
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
Bug 1334111 - Move MediaKeySystemConfig to-string helper to MediaKeySystemAccess.
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
Bug 1334111 - Log MediaKeySystemConfiguration on creation of MediaKeySystemAccess.
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
Bug 1334111 - Test for EME persistentState being rejected in Private Browsing mode.
59 bytes, text/x-review-board-request
Gijs
: review+
Details 
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.
(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'
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)
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: nobody → cpearce
Priority: -- → P3
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 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 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 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+
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
See Also: → 1447085
See Also: → 653826
Blocks: 1706121
Regressions: 1775019
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: