Repository navigation
Add initial patch for CSP:EE style iFrame Inheritance - #29
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Connection Allowlists Bikeshed spec to introduce a CSP: Embedded Enforcement–inspired mechanism for iframe inheritance, allowing an embedder to require (and propagate) a minimum connection allowlist constraint to framed documents via a new iframe attribute and accompanying opt-in/ack headers.
Changes:
- Adds an “Embedded Enforcement” model for iframe navigations, including inheritance (“as strict as”) semantics and blocking behavior when the framed document does not opt in.
- Defines a new
<iframe connectionAllowlist>attribute plusSec-Required-Connection-Allowlist(request) andAllow-Connection-Allowlist-From(response) headers. - Adds strictness comparison and HTML navigation integration steps to enforce the requirement and propagate it to descendants.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mikewest
left a comment
There was a problem hiding this comment.
Thanks! I've left some feedback below, with the overarching request for you to take a look at the approach and details in https://www.w3.org/TR/csp-embedded-enforcement/ and to help me understand either how we'd modify that document or this PR to ensure they're aligned and that we're not defining two distinct enforcement paths for policies. :)
cc @noamr and @domfarolino
noamr
left a comment
There was a problem hiding this comment.
I'm not yet sure how this works with workers, and how we enforce that a document's own required CA needs to intersect with the CA attribute before being passed on for nesting.
A lot of my other comments were already in what @mikewest responded.
Another big open question is whether same-origin contexts with a different required CA are allowed to communicate with each other (e.g. by BroadcastChannel or sharing a SharedWorker/service worekr, or by direct scripting).
Answering this question might require that a "required CA" is part of an agent origin cluster key.
Regardless of the answer, I don't think it's one we need to address in this PR. :) |
Maybe it should be as an ISSUE? |
I think it's distinct enough that you can just add it as a security consideration with an issue we can address in a distinct PR. Or as an ISSUE in the threat model? That might be clearer. |
I meant just add it as an ISSUE somewhere in the spec so that it clear that this is incomplete and there are a few remaining open issues. |
Actually I missed that the integration with "create a policy container from a fetch response" handles workers. So I think this is fine. |
mikewest
left a comment
There was a problem hiding this comment.
I'm swamped at the moment, apologies. Quick feedback that doesn't dig into anything important; will try to take a deeper pass later in the week.
|
|
||
| <div algorithm="requirement is at least as strict as"> | ||
| For the comparison above, a [=string=] (|a|) | ||
| <dfn for=requirement lt="is at least as strict as">is at least as strict as</dfn> a [=string=] (|b|) |
There was a problem hiding this comment.
You don't need the lt here if it's the same text. Also not sure you need for=requirement
Maybe have something in the dfn name that ties it to CA though? it's a pretty generic name
|
I found a few nits and this needs to be rebased. Otherwise I think I'm pretty happy with it but would like another look from @mikewest / @domfarolino |
noamr
left a comment
There was a problem hiding this comment.
Another nit :)
I did another pass on it and it overall looks good.
One thing I suggest having in addition is an explainer that focuses on the use case and the API. It can make it easier to read a very technical spec when there is a more human readable explainer next to it.
|
If Noam's happy, I'm happy. :) Noam, please feel free to land this once the nits are addressed. |
Thanks, nits + fix merge conflicts :) |
Address the four unresolved review nits on PR #29: - Drop the <code> wrapper from the connectionAllowlist attribute dfn so Bikeshed formats it as an element attribute. - Add a note that the 4096 attribute-length limit is an arbitrary bound. - Scope the "is at least as strict as" dfn to the connection allowlist namespace and drop the redundant lt and for=requirement. - Remove the container-policy alternative ISSUE (withdrawn in review). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0cd652e9-05f9-4e15-8450-f1b2e0e5ed60
de0b7f5 to
80bf308
Compare
|
Thanks, @noamr should be updated and rebased now. I'll put up a parallel PR with an explainer to cover this also. |
SHA: a664b3a Reason: push, by noamr Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Adds an initial spec patch to try to describe a CSP:EE inspired approach to handle iFrame inheritance as discussed in #1 .
This adds language around adding the attribute
connectionAllowlistto define a required policy, the UA's requirement to send a notification of aSec-Required-Connection-Allowlistheader, and the ack/opt-in mechanisms for the embedded document. Additional language has been added to describe inheritance/as-strict-as behaviour.