Re: Various comments on the LC draft

On Thu, Apr 24, 2014 at 11:42 PM, Vijay Bharadwaj <
Vijay.Bharadwaj@microsoft.com> wrote:

>  A few of us at Microsoft read through the LC draft at
> http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/ and came up with the
> following comments. They are mostly editorial in nature, so I’m collecting
> them here in one place. I will go through bugzilla and file them over the
> next few days, but do feel free to let me know if any of these is
> controversial. Thanks!
>
>
>
> (The following includes contributions by: Mike Jones, Brian LaMacchia,
> Anthony Nadalin, Israel Hilerio, John Simmons, Kilroy Hughes, and Karen
> Easterbrook.)
>
>
>
> 14.2. Data Types<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#subtlecrypto-interface-datatypes>
>
> This says “jwk” “The key is represented as JSON according to the JSON Web
> Key format.”  This will need to be reworded as part of Bug 24963<https://www.w3.org/Bugs/Public/show_bug.cgi?id=24963>to clarify the type of the returned object (whether ArrayBuffer or JWK).
>

Agreed


>
>
> 14.3. Methods and Parameters<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#subtlecrypto-interface-methods>
>
> In 14.3.1 through 14.3.12, we have statements like “If the name<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#dfn-Algorithm-name>member of
> normalizedAlgorithm does not identify a registered algorithm<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#algorithms>that supports the encrypt operation, then return
> an error<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#concept-return-an-error>named
> NotSupportedError<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#dfn-NotSupportedError>”.
> It seems possible that limited devices (such as embedded devices or
> implementations using hardware crypto modules) will not support the entire
> range of parameters or operations. Even with software implementations, not
> all parameter values will be supported – for example every RSA
> implementation in practice only supports a fixed set of key sizes.
> Therefore we believe these statements should be augmented in each
> occurrence to say something like: “If the name<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#dfn-Algorithm-name>member of
> normalizedAlgorithm does not identify a registered algorithm<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#algorithms>that supports the encrypt operation, or if the implementation does not
> support the encrypt operation with this algorithm using the specified
> parameters, then return an error<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#concept-return-an-error>named
> NotSupportedError<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#dfn-NotSupportedError>
> ”.
>
>
>
I'm not a fan of this change as proposed, though I am supportive of the
reasoning.

A couple issues with it:
1) For implementation consistency, the check about whether nor not the
specified parameters are valid needs to be deferred to the per-algorithm
sections (eg: 18.4.5), since that's where they are normalized into the
parameter type specific for the algorithm.
2) Implementations - including software implementations - may not fully be
aware of what is supported or not. For example, a given algorithm may be
known by the UA, and be known by the cryptographic library or
implementation that the UA uses, but be disabled through means that the UA
doesn't know about (eg: a group policy to disable a legacy algorithm). From
a UA/implementer perspective, it's simply "the operation failed"

Support for "operation failed" consistently across all operations was
something Eric Roman already raised as
https://www.w3.org/Bugs/Public/show_bug.cgi?id=25422 and
https://www.w3.org/Bugs/Public/show_bug.cgi?id=25420

As such, I think the current wording - which allows "performing the
operation results in an error" to raise an OperationError - provides the
ability for implementations that don't support all parameters to return an
error.

As an implementor, I don't think we can reasonably distinguish between all
the possible NotSupportedErrors, and thus I would prefer that the spec not
mandate that specific error.



>  18.2. Recommended algorithms<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#recommended-algorithms>
>
> Please add these algorithms:
>
> ·         RSASSA-PKCS1-v1_5 using SHA-256 (This is in the Recommended set
> for JOSE, and more secure than the SHA-1 variant in the spec.)
>
> ·         RSA-OAEP using SHA-1 and MGF1 with SHA-1 (This is the RFC 3447
> default and the mostly widely deployed OAEP variant. It is also the variant
> used by JOSE)
>

I'm fairly certain this section is going to continue to cause nothing but
trouble, for both reviewers (as in the CFRG WG) and for implementors.

I know we (as a WG) continue to waffle on this, but I can't help but feel
it's better off removed entirely.


>
>
> 18.8.7. ECDSA Operations<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#ecdsa-operations>
>
> Section 18.8.4 specifies the valid values of NamedCurve, but
> curve-specific instructions are scattered through the Import Key and Export
> Key sections of Section 18.8.7. It seems better to collect curve-specific
> items in a table in Section 18.8.4, listing the DOMString value, the curve
> specification and the curve OID, and then to make the text describing the
> operations generic. This would allow for easy extensibility of the spec in
> the future and make the current version easier to read. For example, step
> 11 in the SPKI subsection of Import Key in Section 18.8.7 could say, “If
> params is equivalent to one of the object identifiers listed in Section
> 18.8.4, set the namedCurve attribute of algorithm to the corresponding
> DOMString value.” Similar problems exist in 18.9.4. ECDH Operations<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#ecdh-operations>
> .
>

So I understand the motivation for this, but at the same time, the EC cases
follow the same pattern as the other algorithms - which may have more
complexity (eg: the PSS and OAEP cases).

We've discussed normative tables before, but that is not really followed by
any other web specifications. The tables are provided for informative
reference, but the normative algorithms are explicitly stated.


>
>
> 18.9.4. ECDH Operations<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#ecdh-operations>
>
> As I’ve mentioned before, SP800-56A Section 5.7 prohibits exposing the DH
> / ECDH shared secret directly and instead requires that it be run through a
> KDF before being used. Many FIPS-validated implementations follow this
> restriction. Allowing the Derive Bits operation with DH or ECDH violates
> this prohibition.  We should recognize that at least some implementations
> will only expose deriveKey and not deriveBits with these algorithms, and
> make the latter optional.
>

I don't think we can get consensus here. I understand your point, but at
the same time, Microsoft's own APIs do not follow this limitation,
precisely because it's recognized as a valid use case.

Given how much considerable 'optionality' is already present in the spec, I
cannot feel we'd be doing even more harm to developers to suggest
yet-another-inconsistent-between-browsers feature. If anything, it gets
removed entirely, which would be extremely unfortunate for the valid use
cases discussed previously.


>
>
> 20.1. Generate a signing key pair, sign some data<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#examples-signing>
>
> Delete the comma from this example text:
>
> hash: {
>
>     name: "SHA-256",
>
>   }
>
>
>

Agreed


>  21.1. JSON Web Signature and Encryption Algorithms Registration<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#iana-section-jws-jwa>
>
> The “HS1” algorithm could legally be used in a JWS object.  Therefore,
> change the “HS1” registration to read:
>
>    - Algorithm Name: "HS1"
>    - Algorithm Description: HMAC using SHA-1
>    - Algorithm Usage Location(s): "alg"
>    - JOSE Implementation Requirements: Deprecated
>    - Change Controller: W3C Web Cryptography Working Group
>    - Specification Document(s): [[ This Document ]]
>
>
>
> The document failed to register the “RS1” algorithm identifier, which
> could legally be used in a JWS object.  Therefore, please add this
> registration:
>
>    - Algorithm Name: "RS1"
>    - Algorithm Description: RSASSA-PKCS-v1_5 using SHA-1
>    - Algorithm Usage Location(s): "alg"
>    - JOSE Implementation Requirements: Deprecated
>    - Change Controller: W3C Web Cryptography Working Group
>    - Specification Document(s): [[ This Document ]]
>
>
>

I strongly oppose both of these. If you want to add algorithms usable for
JOSE, discuss with the JOSE WG. We shouldn't be using Web Crypto to
circumvent their process. That the registry exists and allows for non-JOSE
algorithms is great, but we should at least respect the domain of JOSE, who
has made specific security choices on this.


>  21.2. JSON Web Key Parameters Registry<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#iana-section-jwk>
>
> Change word “Registry” to “Registration” in section title.
>
>
>
> The registration is missing the required “Parameter Description” clause.
> Please add this clause to the registration of “ext”:
>
>    - Parameter Description: Extractable
>
>
>
Agreed


>  23.1. Normative References<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#normative-references>
>
> There are references throughout the spec to “ECMA262” and yet the
> reference in the References section is labelled “ECMAScript”.  These names
> should be made consistent.
>
>
>
> A.1. Algorithm mappings<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#jwk-mapping-alg>
>
> Add the curve names to the “EC” mappings.  For instance, this:
>
> { kty: "EC",
>
>   alg: "ES256" }
>
> should become this:
>
> { kty: "EC",
>
>   crv: "P-256",
>
>   alg: "ES256" }
>
> (and do the same for P-384 and P-521).
>

This was not an accidental omission. The absence of parameters for other
cases - such as RSA - was equally intentional. "crv" is a property of the
key itself (much like "n" or "e").

JWA makes it clear what the crv should be if the "alg" is present, which is
what that mapping describes (informatively).

Now, it might be argued that there is a case to be made so that
{ kty: "EC", crv: "P-256" }

is recognized as { name: "ECDSA", namedCurve: "P-256", hash: { name:
"SHA-256" } }, but that would be a change to 18.8.7

I've filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=25465 to clarify
this, and https://www.w3.org/Bugs/Public/show_bug.cgi?id=25466 for related.


>
>
> Add this new mapping entry:
>
> { kty: "RSA",
>
>   alg: "RSA-OAEP" }
>
> to:
>
> { name: "RSA-OAEP",
>
>   hash: {name: "SHA-1"}
>
> }
>
>
>

Yes, although we'll need to be careful to track JOSE. As has been noted in
the JOSE group, the choice of OAEP with SHA-1 is "unfortunate" - and
inconsistent with how JOSE has treated other parameterized algorithms.


>  Add this new mapping entry:
>
> { kty: "RSA",
>
>   alg: "RS1" }
>
> to:
>
> { name: " RSASSA-PKCS1-v1_5",
>
>   hash: {name: "SHA-1"}
>
> }
>
>
>
> We would not suggest adding entries for RSA-PSS with SHA-1, ECDSA with
> SHA-1 or ECDSA where the curve is not aligned with the hash.
>
>
>

Any reasoning behind that? Having RSA-OAEP use SHA-1/mgf1-with-SHA1 but not
having RSA-PSS do the same seems somewhat inconsistent, given the rationale
JOSE as put forth.