Re: Various comments on the LC draft

On Fri, Apr 25, 2014 at 5:08 PM, Mike Jones <Michael.Jones@microsoft.com>wrote:

>  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> 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>
> .
>

This is no different than the change (back) to enums that we've received
feedback on (from TAG and ex-TAG).

When an implementation wants to support other curves, they'll propose
edits. The same way implementations do for every other web feature they
support or add experimental support for.

I do not support making this change.


>
>
>
>
> 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”.
>

I agree, we need to register. However, I cannot support adding algorithms
like "RS1" or "HS1" to be supported by JWS. We are not a WG equipped to
comment on JWS modifications. Leave that for JOSE.

If you feel that these should be valid algorithms for JWS, I would
encourage you to take it to the JOSE WG for discussion. We don't *use* JWS
in our spec, it's entirely inappropriate to suggest an algorithm would be
appropriate to use for it.

As editor of JWS, I'm sure you'll find it very easy to add :)


>    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.
>

That's not what JWA says - so your disagreement is with a spec you're
editing :)

That is, "crv" is in the section of "Parameters for Elliptic Curve Public
Keys" (in JWA), whereas that table tries to limit itself to those items
listed under "JSON Web Key (JWK) Format" (in JWK) - which lists "kty" and
"alg".


>
>
> 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=25466for 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.
>

Let me rephrase this

The choice of naming it "RSA-OAEP" is unfortunate, and breaks tradition
with all of your other algorithms.

If you were to talk about consistent naming, this would be something like
"ROS1" (comparable to "PS256" or "RS256".

That you've chosen "RSA-OAEP" as a name means that every other user of OAEP
- including when libraries do (inevitably) offer the ability to use OAEP
with SHA-256/mgf1-sha256 - will have to find some other name.

That's why the editors note is there - there is no guidance or pattern from
JOSE on how, if WebCrypto wanted (and can) support OAEP with other
algorithms, it would expose that as the "alg" name. "RSA-OAEP-256"?

In the same way we've chosen to add other algorithms to the table, we, as a
WG, need to decide how to handle JOSE's choice to omit other, valid OAEP
forms - and how to express them for JWK export. Right now, it's unspecified.


>    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
>
>
>

That's great - for JOSE. That still doesn't solve the issue for this WG,
which describes an API that allows supporting PSS with SHA-1 or OAEP with
SHA-256. If/when an implementation supports them (and there are some that
can), we still have to figure out what the relevant JWA alg entries should
be, independent of JOSE.