Clarify steps for destruction of keys / licenses

[mwatson2]

Discussion on #85, (see, in particular, this comment), suggests the specification could describe the steps for destruction of keys / licenses more clearly.

I suggest we:
(1) extract the steps for destruction of keys / licenses from the Session close algorithm into a new Destroy keys / licenses algorithm.
(2) refer to this algorithm from the persistent-usage-record session type definition (instead of the current text which duplicates the requirements in prose).

These changes would not add or remove any requirements, but would make the existing requirements easier to understand.

[paulbrucecotton]

@ddorwin, @jdsmith3000: Can one of you please review and merge in Mark's proposed changes so that we can close this issue?

[ddorwin]

For those not following the discussion in the PR, I don't think we should include key destruction in a user agent algorithm invoked by the CDM. See this comment).

[mwatson2]

I will look at this more closely tomorrow, but it was not my intention that
the algorithm be invoked by the CDM, but rather that the User Agent invokes
it when the MediaKeySession is destroyed and tells the CDM (asynchronously
if the user agent implementation wishes) to destroy the keys.

As an aside, I don't think our specification can, does or should address
the relationship between UA and CDM in terms of execution model (I.e.
threading). We define only the observable behavior and anything else is
implementation-specific. Typically we say that the user agent 'uses the
CDM' (usually asynchronously to the JS thread) to perform some task, but we
do not say what that involves, implementation-wise.

As an example, if some task or event is triggered by the CDM (for example a
license renewal message), we do not say whether this is implemented as a
callback from the CDM, the CDM posting a task to some kind of queue, the UA
polling the CDM or any other implementation approach.

[mwatson2]

Further to my comments on the PR, I think we have the same problem with the existing Session Close method. It says it is run "when the CDM closes the session", but it is also explicitly invoked in the close() method. In the former case, it is not necessary for the UA to "destroy the keys and licenses" since the CDM has presumably already done this. In any case, the UA needs to use the CDM to perform that task.

So, I suggest we also split this into the steps that are run when the UA initiates session closure (from the close method) and the steps that are run after the CDM has closed the session on its own initiative.

Also, perhaps the CDM should never just "destroy keys and licenses" on its own initiative: it should close the session.

[jdsmith3000]

I think that it is a reasonable clarification as well to include key destruction in processes that run when a related MediaKeySession is destroyed. I don't see the change proposed here as controversial, and as one that clarifies a topic that came up in recent TAG discussions. The topic warrants clarification in the spec.

[mwatson2]

The original PR is an improvement, but not perfect as @ddorwin points out.

I created an alternative PR #179 which is more explicit about the initiation of the close and destroy algorithms. Specifically, either the UA or the CDM can initiate closure of a session. Only the UA can initiate key destruction outside the context of these two closure methods.

There is a little further refactoring which could be done to handle destruction of licenses for persistent sessions (triggered by remove()) in the same method.

Please comment on whether you prefer this to the previous PR (#172)

[mwatson2]

There are two more problems with the existing specification, unrelated to secure release, which could also be encompassed by this issue:

The close method "uses the CDM" to "process the close request" and then calls the Session Close algorithm.

The Session Close algorithm then "Destroys the licenses and keys". We've discussed above that this action needs to be wrapped in 'use the CDM to' language. The new issues are that:
(1) presumably "processing the close request" will result in destruction of the licenses and keys, so we do not need to do it again
(2) the Close All Session algorithm only calls Session Close and does not ask the CDM to "process the close request" - if there is anything that "processing the close request" does that "destroy the licenses and keys" does not do, then we will be missing this for the Close All Sessions case compared to the close() case.

My suggestion is that we define only one kind of action that the UA can request for the CDM: "close the session" and we define that closing the session in the CDM destroys any non-stored licenses and keys.

Comments ?

[ddorwin]

@mwatson2, this exploration exposes some specific issues with the existing algorithms, especially those related to closing sessions. I believe we can quickly reach consensus on these, and I think it will be most effective to address them in individual small PRs. I filed #181 to track these issues.

That leaves this issue to focus on potential changes to the specification of key destruction and related issues that have wider implications, including related to #85.

I propose that we address #181 to improve the spec then circle back to this issue when we can more easily reason about the meaning and correctness of the changes proposed in this issue and PRs.

[mwatson2]

I have closed the "alternative" PR and made minimal changes to the original PR ( #172 ) to address the comments on execution model.

Per @ddorwin's comment, though, we have identified a few wider issues while discussing this one and so I agree we should address those under #181. Many of the changes proposed here (#172) will be useful there.

[mwatson2]

Just for the record, though, whilst I think it would be better to address all the issues in #181, if we do not have time for that it is still worth addressing the specific clarification which arose from the discussion with the TAG and I believe the PR #172 is sufficient for that. So, whilst this issue is blocked on #181 now, we should remove that block if we cannot make progress in #181.

[mwatson2]

The session close refactor was merged under #181 and the other aspects of #181 are independent of the issue addressed here.

I've updated the PR (#172) and I believe this issue should no longer be blocked on the rest of #181.

[mwatson2]

Reviewed by @ddorwin @mwatson2 @jdsmith3000.

@mwatson2 to move the requirement to run this algorithm to the MediaKeySession section and to refer to this section from the Issue note for Issue 85.

[mwatson2]

I have updated the PR (#172) for this issue, please review.

[ddorwin]

In the editors meeting last week, the editors agreed to land PR #172 because it clarifies the expected behavior for the "persistent-usage-record" and thus may be a useful in the discussion in issue #85.

For the record, this does not imply agreement that the behavior defined in #172 fits the (current) web platform. We will continue to discuss the entire feature in #85.