Nested context required document policy never appears to be set?

[domfarolino]

Create a required policy for a browsing context appears to be the algorithm where we acatually create the Document Policy that contains a union of all document policies (for duplicates, we take the strictest) from the following two sources:

• The policy attribute of the iframe loading the document we're attempting to initialize
• The iframe's nested context required document policy

Only nested context required document policy never appears to be set anywhere, so I can't imagine the "Create a required policy for a browsing context" ever does the correct thing, right?

[domfarolino]

OK I dug into this a bit more today and I have some more critiques and thoughts. It is a bit confusing to track how Document Polices combine/coalesce throughout various levels of the subframe hierarchy, for a few reasons:

• The spec appears to have lots of bugs
• The spec has some duplicate members that appear both on document and browsing context and it is not clear what all of their uses should be

Bugs

In terms of bugs I've spotted in the spec, there are at least a few:

• The OP of this issue; the fact that the nested context required document policy is simply never set
• A browsing context's required document policy is said to be null whenever the browsing context's opener is null. This is the case for almost every child frame on the web, so I think this is worded wrong. I think the spec is trying to express the impact that a parent/child relationship has on a bc's required document policy (i.e., a bc's required document policy is only set for nested bc's, as evident by this algorithm) but is mistakenly doing so via opener/openee rhetoric
• The fact that we never actually parse the Required-Document-Policy response header, like we do for the Document-Policy header here. This is particularly bad, because it means that we never actually store the value of a document's Required-Document-Policy anywhere in any member, which means we never actually delegate it down to subframes for future enforcement, which is certainly the intention of the spec 😞
• It's really not clear if a Document object has a document policy member or not. The spec mentions a concrete declared document policy member, but unfortunately it is only ever read and never written to. I guess a loose reading of the spec could indicate that it is written to by algorithms like process response policy which "returns a declared document policy", but it is not clear what returning a declared document policy even means, since a declared document policy is a specific member of a Document object/instance, which we never explicitly set. The spec has several mentions of "document’s document policy" but that doesn't really mean anything. For example, see step 7 of this monkeypatched algorithm. This doesn't really work, because there is not a document policy member on the Document object -- do we instead mean declared document policy? reported document policy? a new member altogether? My read is that the spec always means declared document policy, in which case I'd prefer renaming this to document policy since that's what we're using in more cases than not.

Confusing/duplicate members

My read of the spec is the following (and please read the -> syntax as if it were C++ pointer syntax):

• document->document_policy (was declared document policy): A document policy that is only influenced by Document's Document-Policy response header. This policy only influences the Document that this is a member on.
  • This header is parsed correctly in the spec today, but we need to explicitly set document->document_policy better
• document->required_document_policy: I propose that we introduce this member, which will be a direct copy of the Document's parsed Required-Document-Policy response header value. It has no influence over the Document that this document policy is a member on. It is solely used to produce a minimum document policy that all subframes (nested under this Document) must adhere to (via their Document-Policy response header)
  • The spec current does not parse or store this anywhere, so I propose we do that, performing its processing almost identically with the above member.

Now we get to the members stored on the browsing context concept

• document->browsing_context->nested_context_required_document_policy: Unfortunately this is never set anywhere in the spec. Regardless, it is a document policy that never ever impacts the document of the browsing context that this member sits on; it is only used to influence the document policy required for subframes nested underneath this browsing context. [EDIT]: I'm almost positive this is meant to be the current_bc->require_document_policy + current_doc->required_document_policy, as this is exactly what will be enforced in all subframes under this document, but not for the current document.
• document->browsing_context->required_document_policy: This member is only ever set for nested browsing contexts, as evident by the create a required policy for a browsing context algorithm. It is currently the policy on a given browsing context that we use check against the Document-Policy response header on the document that loads in said browsing context, to make sure the Document-Policy sent with the document is indeed strict enough. That means by the time a nested browsing context navigates, it must already have this member set on it, so that when the nav finishes, we can compare the response header against it. That means it must be the strictest union of the following three policies:
  1. this_browsing_context->container_element->policy_attribute
  2. this_browsing_context->parent_bc->required_document_policy (this aims to pick up all of the strictest document policies that at least the parent document had to respect)
  3. this_browsing_context->parent_bc->document->required_document_policy (this aims to pick up any additional strict policies that the parent's document's Required-Document-Policy response header might have sent)

It is (2) + (3) that I think the spec's concept of the nested context required document policy meant to represent. In other words, when a document loads we know it had to at least meet the bar set by its browsing context's required document policy, but if that document was sent with additional restrictions in the form of the Required-Document-Policy response header that should begin to take effect for subframes rooted at this document, then we need to make sure the nested browsing context of any subframes we have, has a required document policy of at least our bc's required document policy plus the restrictions in the form of our R-D-P header! We should fix this!

//cc @domenic

[clelland]

Thanks for the very thorough review, @domfarolino! The spec is in pretty sad shape, but I think I have some time to take care of the most egregious issues now.

To the specific points:

1. Yes, nested context required document policy is not set anywhere. That's this issue.
2. Opener vs top-level is relevant to sandboxing; there was a plan (see https://github.com/WICG/document-policy/issues/22#issuecomment-562612153) is to propagate document policies to sandboxed frames. In that case. only top-level contexts with no opener are guaranteed to start with a blank policy. The statement there is probably not needed, and all of the logic for determining the initial state should go into the algorithms.
3. Require-Document-Policy does need to be read, at the same time as Document-Policy.
4. The document should have the policy; The 'declared policy' is intended to be the policy which was declared in the Document-Policy response header, and that is necessarily the policy in effect (If there were stricter requirements, then the document would not have loaded). The HTML integration section should actually set that somewhere.

[clelland]

I've updated the spec to fix most of these issues, I think. Take a look and see what's still outstanding:

• A Document's 'declared document policy' is just 'document policy' now.
• The 'nested context required document policy' lives on the Document, and is essentially the document->required_document_policy that you proposed. It is constructed from the response when the document is created.
• Some of the terminology issues have been fixed up; we're just passing and returning document policies, which are defined as a data structure.
• Parsing algorithms have been cleaned up a bit, now separating the structured-header parsing from the interpretation of that dictionary as a policy declaration.