Re: CSP 1.1 DOM design

Inline.


On Mon, Nov 5, 2012 at 9:27 AM, Mike West <mkwst@google.com> wrote:

> On Sun, Nov 4, 2012 at 9:58 PM, Alex Russell <slightlyoff@google.com>wrote:
>
>> Looking at Section 3.4 of the CSP 1.1 draft [1], I'm noticing that the
>> IDL specified feels very, very strange to use from the JS perspective.
>>
>
> Thanks for taking a look! This is great feedback.
>
>
>>  For instance, the name "document.SecurityPolicy" would indicate to a
>> mere JS hacker like me that the SecurityPolicy is a class from which
>> instances will be created. Instead, it's an instance of the SecurityPolicy
>> interface. A more idiomatic name might be "document.policy",
>> "document.csp", or "document.securityPolicy" as leading-caps tend to be
>> reserved for classes, not instances.
>>
>
> Adam, do you remember why we ran with 'SecurityPolicy' rather than
> 'securityPolicy'? I know we discussed it, but I can only find the comment
> resulting from that discussion (
> https://bugs.webkit.org/show_bug.cgi?id=91707#c5).
>
>
>> Similarly, it's not possible (AFAICT) to new-up an instance of
>> SecurityPolicy and no API provided for parsing a policy to understand how
>> it would react.
>>
>
> That's an interesting suggestion. What's the use-case you see for
> providing a mechanism for parsing/examining a policy?
>

I'm hitting this in real time as I'm trying to write an extension that
merges/displays policies. Long story short, I've got a "default user
policy" that I'd like to apply to each page until/unless that page gives me
a more locked-down policy. I'd also like to know what has been blocked in
the page. The current API is balls for all of this. Luckily CSP parsing is
easy. On the downside, matching and getting the rule application right
isn't.

There's already a parser/matcher in the implementation. Not exposing it is,
on a more philosophical basis, simply bad design.


> The only thing I can come up with off the top of my head is the tool we
> briefly chatted about that would help developers understand the impact of a
> policy. :)
>

That was going to be my other use-case: I want to build a tool that shows
people how to construct policies well. But it's a minor point; the larger
issue is that it's bad layering to say "the browser does this but no, you
can't have access to that parser/matching logic". The default here has to
be for API designers to show why they MUST NOT expose something like this
when they're providing an API, not on developers to show that they MUST
have the feature. Don't worry, though, most of the DOM gets this wrong. But
we don't have to here! Hooray!


>
>
>> Lastly, there's no serialization method provided. A toString()
>> implementation might work well.
>>
>
> What would the string representation of the object look like? Just the
> original policy?
>

Yep.


> One complication is that the page's active policy might be created by the
> union of several policies (one sent per HTTP, one in a meta tag, etc).
> Would we want to retain that representation in a string version?
>

Yes. Isn't that the applied policy?

Also, speaking as someone writing the union logic himself in JS at the
moment, I'd love for union/intersection methods to be made available. They
should take SecurityPolicy instances/strings (var-args style) as arguments
and return a new SecurityPolicy instance (not locked down).


>
>
>>     readonly attribute DOMString[] reportURIs;
>>
>
> We decided at TPAC to remove the reportURIs getter unless someone has a
> really good use-case for it.
>

First, that's just totally backwards. If it's in the serialization, it
needs to be in your API unless there's a compelling reason to remove it. As
I've been working with this stuff, I also think the per-policy
domain/setting flags should be exposed on SecurityPolicy instances as well.
reportUR[I|L]s should just be part of that list.

Next, not only should reportURLs be there, but there should be an event you
can catch for violations with the JSON payload you'd send to the server
delivered to the DOM as well.


>
>
>> One open issue: I'm not sure If allowsEval, allowsInlineScript, and
>> allowsInlineStyle should just be boolean getters or if they should stay
>> methods.
>>
>
> I like the idea of converting these `allowEval()`-style calls to read-only
> booleans. Perhaps 'isActive' as well.
>
>
>> Also, it's unclear if the current document's policy should simply be a
>> locked-down instance of a SecurityPolicy class that has accessors for each
>> of the policy items (script-src, object-src, style-src, img-src,
>> media-src, frame-src, font-src, connect-src).
>>
>
> I think that's more or less what the current interface does. (e.g.
> `document.SecurityPolicy.allowsFontFrom('xxx')` is an accessor for the
> effective permissions granted via the 'font-src' directive). Would you
> prefer more direct access to the policy? We'd shied away from that on the
> assumption that this interface required less knowledge of CSP in order to
> usefully include on a page. Should we revisit that question?
>

Yes  = )

I think it's good to have the test methods. I also think it's good to have
a full-faith representation to the JS of how the policy is parsed,
represented, and serialized.

Ask the question this way: if I was implementing the matching/enforcement
logic in JS, what would I need access to? *THAT THING IS YOUR API.*

Regards