Re: [SRI] Comments on Subresource Integrity spec

On Mon, May 11, 2015 at 11:34 PM Gervase Markham <gerv@mozilla.org> wrote:

> On 08/05/15 22:15, Joel Weinberger wrote:
> > In
> >
> https://w3c.github.io/webappsec/specs/subresourceintegrity/#the-integrity-attribute-1
> ,
> > in particular the production for hash-algo, which references the CSP
> > spec and its definition of hash algorithms, so I believe it implicitly
> > does.
>
> Well, OK, but that seems a bit obscure. I think there would be no harm
> in being explicit about it - either with a list, or by saying yuo are
> deferring to the CSP spec.
>
As Dan mentioned, I think it's been pretty clear on the list. I don't
particularly feel like the wording needs to be changed to make this any
more explicit, but if any of the other editors (or anyone else) wants to
make a pull request to add in wording to clarify, I'd certainly merge it.

>
> >     * The spec does not say how to form the identifying tag for a hash
> >     function - e.g. converting "SHA-256" into "sha256". This may seem
> >     simple, but the possible variants of SHA-3 are still not defined
> (AIUI),
> >     and so there may be ambiguity here. The CSP document punts on this
> too,
> >     simply defining 3 values - "sha256", "sha384" and "sha512". Is this
> >     their problem, or something to be addressed by the spec?
> >
> > I don't think I understand the question. In the CSP spec,
> > https://w3c.github.io/webappsec/specs/CSP2/#source-list-valid-hashes
> > defines a conversion of the hash algorithms in the syntax into explicit
> > hash functions.
>
> Yes, you don't understand the question :-) Let me try again. Next year,
> NIST standardises the SHA3 algorithm, in 256 (a.k.a. subtype A), 384
> (a.k.a. subtype B) and 512-bit (a.k.a. subtype C) variants. Chrome
> implements this as:
>
> integrity="sha3_256-09F9..."
>
> Firefox, however, checks in a patch on the same day, recognising:
>
> integrity="sha3-09F9..."
>
> with the variant chosen by the length of the hash given. And, by amazing
> coincidence, Safari also publishes an implementation on the same day
> going for:
>
> integrity="sha3A-09F9..."
>
> Who is right? In other words, who gets to mint new hash identifiers? The
> first browser to publish? Or does Subresource Integrity have to wait for
> the CSP people to publish a new version of their spec? Is that wise?
>
> >     * The spec does not give guidance as to whether, if integrity
> metadata
> >     is loaded in a privileged context, and is applied to an unprivileged
> >     document load, the user agent should consider the resulting context
> >     still to be privileged ("secure"). I can see the case for both
> answers.
> >     There is no security loss in loading jQuery from a CDN using
> >     integrity-protected HTTP. There may be a small privacy loss (e.g. the
> >
> > There is no privacy in the transport, so there is a major security loss.
> > I believe
> >
> https://w3c.github.io/webappsec/specs/subresourceintegrity/#unprivileged-contexts-remain-unprivileged-1
> > does address this, making it explicit that unpriviliged contexts remain
> > unprivileged.
>
> I don't believe that section says what you claim it says. It's point is
> "authors SHOULD deliver integrity metadata only to a privileged
> document." That says nothing about whether a browser should show a
> mixed-content warning when integrity metadata is delivered to a
> privileged context, as suggested (loaded over HTTPS, a page starts as a
> privileged context, and remains one until the first HTTP load, after
> all), but the resource is then loaded over HTTP. Does it remain
> privileged? If the suggestion is no, the spec should say so.

You're right, my mistake. I'll upload a CL to make this more explicit.

> > The idea is that if the hash function is broken, then it doesn't provide
> > any security, so checking the integrity is completely unnecessary (since
> > an attacker could just inject their own matching hash).
>
> I think this view has an unnecessarily binary understanding of "broken".
> I am not recommending continued use of known problematic algorithms, but
> if we are in a state where checking Hash Algorithm A is proof against
> everything including governments, and checking Hash Algorithm B is
> suspected to be good against everyone except the NSA and GCHQ, there is
> still value in checking Hash Algorithm B, if the alternative is simply
> loading the content anyway. The attacker is not always a nation-state :-)
>
> Checking even an MD5 would defeat _me_, if I was trying to attack you...
> There's some value in that, surely? :-)
>
I still disagree. We could also support ROT13, which would stop some number
of attackers as well, but it wouldn't really fit a definition of "secure."
It's important to keep a clear set of security guarantees of what an
integrity check gets you. I actually do believe that it is as binary as I
initially presented. I don't buy the "Gov't can break it but no one else
can" theory. If the Govn't can break it... then it's broken, and we should
assume that a large criminal org can also break it.

Additionally, the discretion of what hash functions are used is still up to
the user agent. For example, we explicitly allow the user agent to define
its own algorithm for "strong" hash functions (see
https://w3c.github.io/webappsec/specs/subresourceintegrity/#get-the-strongest-metadata-from-set.x).
So perhaps our disagreement is all academic, because I think this section
ought to satisfy your concerns. If a user agent decides that a hash
function is "strong enough," it can keep it, but just put it low in the
priority list.

Also, to Dan's point that we shouldn't "fail closed," I agree, and I
believe the spec is clear on that (
https://w3c.github.io/webappsec/specs/subresourceintegrity/#agility-1). If
the hash function is unsupported, the user agent must ignore the integrity
value. Chrome will, of course, also give a console message, and I suspect
other implementations will do similarly.

>
> >     * The ABNF for the integrity attribute allows option names without
> >     values, but it seems that it requires the = sign even when there's no
> >     value. That seems like a bug? It also doesn't seem to allow multiple
> >     options for a hash - the option-expression is not allowed multiple
> >     times, and no option separator is defined. In fact, & is an
> >     option-value-char so it could not be used as things stand. Perhaps
> >     semicolon? In general, it would be good to have a non-normative
> >     fictional example of option use, even if none are defined in the
> spec.
> >
> > I believe the production does allow for multiple options, using "?" as a
> > separator.
>
> Yes, now I look again, you are right. I don't like this, because it
> seems like you are using URL parameter style syntax ("?" and "=") but
> not actually using it, which is a footgun. I think you would be better
> off supporting:
>
> sha256-09F911029D74E35BD84156C5635688C0?foo=bar&baz=quux
>
> but using semicolon as the separator is also URL-like.
>
> > I don't think it's a bug to leave in the '=' sign; we wanted
> > it to be explicit that the value should be empty.
>
> I think you are going to hit exactly the same issue they had with HTML
> and non-valued attributes ("checked='checked', anyone?); I predict you
> will end up supporting the bare attribute name anyway. :-)
>
> Gerv
>