Re: Hashes.

Thanks for your feedback. I'm incorporating the requested changes in
the attached patch.

On Thu, Dec 12, 2013 at 1:52 PM, Joel Weinberger <jww@chromium.org> wrote:
>
>
>
> On Thu, Dec 12, 2013 at 10:56 AM, Garrett Robinson <grobinson@mozilla.com>
> wrote:
>>
>> On 12/11/2013 05:57 PM, Dionysis Zindros wrote:
>>
>> > The change of "nonce-value" to "base64-value" is undesired for the
>> > following reasons:
>> >
>> > (a) When using a nonce in scripts or styles, the nonce-value is not
>> > the base64 encoding of anything. It's just a random string securely
>> > generated by the server independently for each request. The spec
>> > grammar should use a name that reflects this. "nonce-value" is
>> > appropriate, but "base64-value" is not.
>>
>> The prior set of allowed characters for nonce-value was identical to
>> base64-value (I only clarified that ='s should only be at the end). The
>> original spec patch separated the two into "nonce-value" and
>> "hash-value", but since they were the same ABNF rule it seemed
>> unnecessary and confusing to keep them separated.
>>
>> baes64 is just an encoding, and one that is commonly used to avoid
>> issues in textual formats like HTML. It would not be wise to stick a
>> bunch of random bytes in the nonce attribute of an HTML element - but a
>> bunch of base64-encoded random bytes is not problematic.
>
> I second Garrett's point. The choice of base64 characters for nonce-value
> originally was not an accident. Given that it's a standard encoding that's
> generally not problematic, why not be explicit about it?
>>
>>
>> > (b) The grammar key "nonce-value" is referenced 3 times in the rest of
>> > the spec. This change makes these references dangling. Furthermore,
>> > changing these references to "base64-value" to match the grammar is
>> > not a good idea, as they talk about the semantics of nonce-value in
>> > particular. The references are in sections 3.2.2.3 and 3.2.2 and the
>> > text talks particularly about the nonce, not the hash.
>> >
>> > I recommend we go back to "nonce-value" as a separate grammar record
>> > for the nonce value, and use an identical grammar record with the same
>> > right-hand-side called "hash-value" for the hash value.
>>
>> We have base64-value because nonce-value == hash-value. I could perhaps
>> see the value (for clarity in reading the spec) if we re-defined them as
>> follows:
>>
>> base64=value = ...
>> nonce-value  = base64-value
>> hash-value   = base64-value
>>
>> But, again, I am not sure if this is more or less confusing, and it is
>> certainly more verbose.
>>
>> >>
>> >> [2]: https://dvcs.w3.org/hg/content-security-policy/rev/053e1cf7c388
>> >
>> > The NIST standard illustrates hash results as hex. We want to make it
>> > clear that the binary result of the hashing functions is subsequently
>> > fed to base64. Let's add a clarification there.
>> > Recommend that we change the following:
>> >
>> > If the <a
>> > href="#dfn-digest-of-an-elements-contents"><var>algorithm</var>
>> > digest of <var>element</var>'s contents</a> is a case-insensitive match
>> > for
>> > <var>expected</var>, return true and abort these steps.
>> >
>> > To the following:
>> >
>> > If the <a href="http://tools.ietf.org/html/rfc4648#section-4">base64
>> > encoding</a> of
>> > the binary <a
>> > href="#dfn-digest-of-an-elements-contents"><var>algorithm</var>
>> > digest
>> > of <var>element</var>'s contents</a> is a case-insensitive match for
>> > <var>expected</var>, return true and abort these steps.
>> >
>> > Even better, it would be great to define a separate "actual" in
>> > addition to "expected".
>>
>> Additionally, that should be a *case-sensitive* match, since base64
>> includes a-z and A-Z.
>>
>> > Recommend that we also add something along the lines of the following,
>> > to ease adoption and implementation for web developers:
>> >
>> > If the user agent fails to match hash-value, the user agent SHOULD
>> > report a warning message in the developer console containing the
>> > actual hash value.
>> >
>> > These modifications are reflected in the attached patch
>> > (csp-specification.dev.html.patch).
>> >
>> > Thanks!
>>
>> That's a nice idea!
>>
>> -Garrett
>
>