RE: Various comments on the LC draft

Thanks for your responses, Ryan.  A few additional comments are included below, prefixed by “Mike>”.

From: Ryan Sleevi [mailto:sleevi@google.com]
Sent: Friday, April 25, 2014 3:08 PM
To: Vijay Bharadwaj
Cc: public-webcrypto@w3.org
Subject: Re: Various comments on the LC draft



On Thu, Apr 24, 2014 at 11:42 PM, Vijay Bharadwaj <Vijay.Bharadwaj@microsoft.com<mailto: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.

Mike> The gist of this one is that the wording currently makes it seem like you can only return “NotSupportedError” in a particular case, whereas there are lots of other cases in which implementations might reasonably return it.  The spec should be worded to allow this.

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.

Mike> Unless it’s removed entirely, can you please add these, as they are what JOSE implementations use?

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.

Mike> The real problem is that, as written, the “Otherwise” clause of Import Key operation requires that an error be returned if the curve is not one of secp256r1, secp384r1, or secp521r1.  This MUST be generalized to allow implementations to support other curves, even if those curves aren’t specified in the initial version of this specification.  This problem must also be corrected in other locations where it occurs, such as 18.9.4. ECDH Operations<http://www.w3.org/TR/2014/WD-WebCryptoAPI-20140325/#ecdh-operations>.


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.

Mike> JOSE defined those registries *exactly* so that third parties could define algorithms usable in JOSE implementations.  That *is* a process defined by JOSE.  Adding these wouldn’t be circumventing the process – it would be using it.

Also, the WebCrypto document should be consistent.  You’re defining the “RS1” alg value, therefore you need to register it, like you’re already doing for “HS1”.
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").

Mike> I disagree – “crv” is a property of the algorithm.  I don’t feel super-strong about this one, but it would seem more helpful to implementers to include the “crv” parameter in the table than to omit it.  It’s non-normative, so it’s not like including it would confuse anyone, and it would help some.

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.

Mike> The choice isn’t unfortunate – it’s deliberate, because that’s what’s actually deployed.  This was extensively discussed in JOSE and most participants believed that using the default parameters was the right call – especially since many implementations don’t provide any way to change to using a different hash function.
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.

Mike> In the implementation survey conducted in 2012, there was already widespread support for RSASSA-PKCS1-v1_5 with SHA-256, so there was no problem specifying it for JWS.  However, there was not widespread support for OAEP with SHA-256, so that wasn’t specified.  I personally agree with you that we needn’t have specified any PSS identifiers at all, since it’s also not widely deployed, but given that it wasn’t, we weren’t breaking anything by specifying PSS with SHA-2, since mostly it isn’t deployed at all in any form.

                                                                -- Mike