Skip to content

Commit

Permalink
DRM: Create new persistent MediaKeySession when load fails
Browse files Browse the repository at this point in the history
We noticed of an issue which led to the impossibility of playing an
encrypted content in a "persistent-license" mode if its license had
already been persisted in the past but it could not be retrieved by
the browser through the `MediaKeySession.load` API.

This problematic situation only arises if the RxPlayer's persistent
session storage, generally stored in `localStorage` by the application,
is de-synchronized with the browser/CDM internal one, with the former
having more items than the latter.
Arguably that situation is not frequent:

  - we reproduced it at a large scale when updating the browser of some
    devices on which the RxPlayer runs.

    That update was not a small step (the two versions were released
    years apart), leading us to believe that that browser internal
    representation of persisted `MediaKeySession` had changed, thus it
    being not able to re-load old persistent `MediaKeySession`.

    Because the application kept however the same store of old persisted
    `MediaKeySession`, such de-synchronization was created.

    (To note that the RxPlayer currently has a policy of being
    retro-compatible by default - unless opted-out for performance
    reasons - when it comes to old persisted `MediaKeySession`, so
    updating the RxPlayer should normally not invalidate old persisted
    `MediaKeySession` the way it did for the browser here.)

  - In any case, we suppose that a browser or CDM may decide at any time
    to evict old persisted `MediaKeySession`.

    The RxPlayer also has an eviction mechanism in its own store but as
    both logics are completely un-synchronized, differences may appear
    in any time.

---

The previous logic of the RxPlayer when failing to load an old
persistent `MediaKeySession`'s information on a newly created one (as
that's how the EME API works), was to just abort trying to load it
and continue doing the license request on the same `MediaKeySession`.

However, by re-reading the specification, it seems that this does not
work. This is not indicated clearly however, but can be deduced by
following the `unitialized` flag the EME specification brings up in some
API's documentation.

Reading the different algorithms proposed for the `load` and
`generateRequest` methods, it appears that neither can be called after
the other was on the same `MediaKeySession`, because this `unitialized`
flag is set to `false`, having previously been asserted to be `true`
right before the actual implementation logic is done.

Even more implicit, it seems that it is not possible to `close` a
`MediaKeySession` whose `load` call did not retrieve anything, by
following this time an EME flag called `callable`.

---

The (simple) fix for that situation, which remain to be tested, is to
just throw away, without closing it, a `MediaKeySession` whose `load`
call resolved with false, and to create a new one so that
`generateRequest` can be called on it.

Also, to prevent any issues (for now unseen) with that `callable` flag
whose context appear more unclear, I chose to check if a `sessionId` is
set on a persistent `MediaKeySession` whose closing operation failed,
to ignore the failure if it wasn't and just continue the usual logic.
I did that check in such a way because there seem to be a relation
between a set (i.e. different than the empty string) `sessionId` and the
`callable` flag though this is never clearly stated.
  • Loading branch information
peaBerberian committed Aug 1, 2022
1 parent 3a689f8 commit 7cb1ce0
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 8 deletions.
11 changes: 5 additions & 6 deletions src/compat/eme/load_session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,14 @@ const EME_WAITING_DELAY_LOADED_SESSION_EMPTY_KEYSTATUSES = 100;
* Load a persistent session, based on its `sessionId`, on the given
* MediaKeySession.
*
* Returns an Observable which emits:
* - true if the persistent MediaKeySession was found and loaded
* - false if no persistent MediaKeySession was found with that `sessionId`.
* Then completes.
* Returns a Promise which resolves with:
* - `true` if the persistent MediaKeySession was found and loaded
* - `false` if no persistent MediaKeySession was found with that `sessionId`.
*
* The Observable throws if anything goes wrong in the process.
* The Promise rejects if anything goes wrong in the process.
* @param {MediaKeySession} session
* @param {string} sessionId
* @returns {Observable}
* @returns {Promise.<boolean>}
*/
export default async function loadSession(
session : MediaKeySession | ICustomMediaKeySession,
Expand Down
30 changes: 28 additions & 2 deletions src/core/decrypt/create_session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,18 @@ async function createAndTryToRetrievePersistentSession(
if (!hasLoadedSession) {
log.warn("DRM: No data stored for the loaded session");
persistentSessionsStore.delete(storedEntry.sessionId);

// The EME specification is kind of implicit about it but it seems from my
// understanding (Paul B.) that a MediaKeySession on wich a `load` attempt
// did not succeed due to the loaded session not being found by the
// browser/CDM, should neither be used anymore nor closed.
// Thus, we're creating another `"persistent-license"` `MediaKeySession`
// in that specific case.
loadedSessionsStore.removeSessionWithoutClosingIt(entry.mediaKeySession);
const newEntry = loadedSessionsStore.createSession(initData,
"persistent-license");
return { type: MediaKeySessionLoadingType.Created,
value: entry };
value: newEntry };
}

if (hasLoadedSession && isSessionUsable(entry.mediaKeySession)) {
Expand Down Expand Up @@ -153,7 +163,23 @@ async function createAndTryToRetrievePersistentSession(
persistentSessionsStore.delete(persistentEntry.sessionId);
}

await loadedSessionsStore.closeSession(entry.mediaKeySession);
try {
await loadedSessionsStore.closeSession(entry.mediaKeySession);
} catch (err) {
// From reading the EME specification in details, it seems that a
// `MediaKeySession`'s ability to be closed is tightly linked to its
// possession of a "sanitized session ID" set as `sessionId`.
// This is never clearly stated however and I'm (Paul B.) always afraid of
// breaking compatibility when it comes to EME code.
// So we still try to close the `MediaKeySession` in any case, only, if it
// fails and it didn't had any `sessionId` set, we just ignore the error.
// Note that trying to close the `MediaKeySession` might incur some delays
// in those rare cases.
if (entry.mediaKeySession.sessionId !== "") {
throw err;
}
loadedSessionsStore.removeSessionWithoutClosingIt(entry.mediaKeySession);
}
if (cancelSignal.cancellationError !== null) {
throw cancelSignal.cancellationError;
}
Expand Down
29 changes: 29 additions & 0 deletions src/core/decrypt/utils/loaded_sessions_store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
loadSession,
} from "../../../compat";
import log from "../../../log";
import assert from "../../../utils/assert";
import isNullOrUndefined from "../../../utils/is_null_or_undefined";
import { IProcessedProtectionData } from "../types";
import KeySessionRecord from "./key_session_record";
Expand Down Expand Up @@ -310,6 +311,34 @@ export default class LoadedSessionsStore {
await Promise.all(closingProms);
}

/**
* Find the given `MediaKeySession` in the `LoadedSessionsStore` and removes
* any reference to it without actually closing it.
*
* Returns `true` if the given `mediaKeySession` has been found and removed,
* `false` otherwise.
*
* Note that this may create a `MediaKeySession` leakage in the wrong
* conditions, cases where this method should be called should be very
* carefully evaluated.
* @param {MediaKeySession} mediaKeySession
* @returns {boolean}
*/
public removeSessionWithoutClosingIt(
mediaKeySession : MediaKeySession | ICustomMediaKeySession
) : boolean {
assert(mediaKeySession.sessionId === "",
"Unitialized `MediaKeySession` should always be properly closed");
for (let i = this._storage.length - 1; i >= 0; i--) {
const stored = this._storage[i];
if (stored.mediaKeySession === mediaKeySession) {
this._storage.splice(i, 1);
return true;
}
}
return false;
}

/**
* Get the index of a stored MediaKeySession entry based on its
* `KeySessionRecord`.
Expand Down

0 comments on commit 7cb1ce0

Please sign in to comment.