Re: Comments on Subresource Integrity

(Resending from the correct address...)

Thank you! Responses inline.

On Sat, Jan 11, 2014 at 4:06 PM, Anne van Kesteren <annevk@annevk.nl> wrote:

> I looked at yesterday's draft of
> http://w3c.github.io/webappsec/specs/subresourceintegrity/
>
> The introduction starts off a little weird. Maybe swap the first two
> sentences?
>

Hrm. I'll poke at this, I guess.


> ni URL scheme. Is this expected to be parsed and handled by browsers?
>

Only insofar as the three components need to be extracted.


> Because then we need a definition layered on top of
> http://url.spec.whatwg.org/ I think.
>

What would you suggest?


> The eligibility check in 3.3.2 seems to check for CORS twice. A
> resource is "CORS same-origin" if a CORS check for it passes.
> (Terminology here is still a bit in flux due to the rewrite in Fetch.)
>

https://github.com/w3c/webappsec/commit/774c2f61708af325e168b780dc8da48875e1816d


> Step 4 of that algorithm could use some explanation as to why it is
> done. It seems somewhat dodgy.
>

Which part of step 4 (now step 2)? The "Access-Control-Allow-Credentials"
bit? I added that as a resource fetched via a basic fetch would fail a CORS
check even if it contained a reasonable 'Access-Control-Allow-Origin'
header due to steps 4 and 5 of the resource sharing check.

Since most of these requests will be basic fetches, bypassing those steps
seems reasonable.

If that's the bit you meant, I'll add a note to that effect:
https://github.com/w3c/webappsec/commit/7ff8c4720b4241d6c89a1cebb5ab5e2bc5cd4288


> In 3.3.3 you want to talk about resource's URL's scheme, not just its
> scheme.
>

https://github.com/w3c/webappsec/commit/70d78d5ff4055941a9754ce22f868ce091f1e9a0


> Why do we make integrity a global attribute while other fetch-related
> attributes are always local? That does not make much sense to me.
>

If I'm honest, this was laziness. I think you're probably right that we
should add it to the individual interfaces.


> For the <iframe> element I don't understand why we'd delay rendering
> if the policy is not block.
>

Good point:
https://github.com/w3c/webappsec/commit/f3919c3e1e3d125cab1fac41a0676efd4a425c46


> For CSS I think we want something like integrity-url(), but maybe CSS
> should have a more generic mechanism as I suspect we want to be able
> to control more there in the long term. E.g. CORS, whether Referer is
> emitted, whether cookies are included, etc. So maybe we should have
> url() and fetch() where fetch() allows for metadata.
>

You should talk with Tab. I'll spin this off into a new thread.


> For XMLHttpRequest should we not put if statements around dispatching
> progress events and such if the policy is block? Seems somewhat weird
> for that API to be different from the others.
>

The current spec basically says "Hey author, don't be silly. Listen only
for `load` and `error` if you care about integrity."

But it might make sense to protect authors from themselves. How does
https://github.com/w3c/webappsec/commit/ff3149f29eefe60c226439f7eefb5f14e7354999
 look?


> You argue that the MIME type "SHOULD be provided" but then in the
> examples in 1.2 you never do that.


https://github.com/w3c/webappsec/commit/4a204c7e788d8aaeebc6deb37f0947fe4a907317


> Also, given that you only check
> whether the MIME type specified by the integrity attribute matches
> that specified by the server, you are not actually protecting the
> numerous contexts where we end up sniffing the resource. E.g. <img>
> only looks for image/svg+xml, <script> never looks at the MIME type,
> etc. If you want this MIME type thing to be meaningful you got to dive
> into the http://mimesniff.spec.whatwg.org/ rabbit hole I think.
>

I was assuming nosniff-style behavior here. If that option is set, I know
that Blink, at least, does reject scripts that are served with
inappropriate headers. I guess I need to read that spec to see how I can
define the kind of behavior I'd like to see here.


> Finally, I'm having a hard time writing integrity, isn't there some
> easier-to-spell word we can use? hash is still free I think.
>

"hash", "digest", "checksum", "whatever". If folks have strong feeling
about the names, I'm happy to change them. "integrity" made the most sense
to me, as that's the purpose of the attribute, but I'm totally sympathetic
to spelling. :)

Thanks for taking the time to read the spec and provide feedback. I
appreciate it.