ECDSA import does not specify the hash algorithm

[jimsch]

Both of the RSA algorithms and HMAC set the hash algorithm at the time of import of the key. This is not done for ECDSA as the hash algorithm is specified at the time of the sign/verify operation.

The hash algorithm should be specified in EcdsaGenParams and EcdsaImportParams (new structures) and removed from EcdsaParams which becomes None.

I looked and did not see this in Bugzilla but would not be surprised if I missed it.

[mwatson2]

Comments on this proposed change ?

[mwatson2]

7/11 call suggests implementing this change.

[mwatson2]

PR #120

[mwatson2]

Please note this is a breaking change: applications that presently provide the hash parameter at sign / verify time will need modification to provide it instead at generate / import time.

[engelke]

After sleeping on it, I'm no longer in favor of the change. Given how mature a couple of implementations are now, and the existence of quite a few tools built on them, I don't want to break them without a very strong need. I agree this change makes sense from a consistency point of view, but that's not enough for a breaking change.

[mwatson2]

With some additional specification complexity, we could support both modes: hash attached to the key and hash supplied with the operation. Providing the hash in both or neither place would be an error. We could even deprecate creating keys without a hash attached, whilst leaving it in the specification.

[jimsch]

How much does not making this change affect client code? If they want to be algorithm agile, do they need to have different code for the RSA and ECDSA to perform signatures and verifications or can the code still overlap to a large degree?

[mwatson2]

If an author has general-purpose code which accepts the algorithm parameters for key generation/import and sign/verify as separate inputs, then having this difference between algorithms would be ok.

If the general-purpose code just accepts a single input from which it derives the value to be used for the generation/import step and the sign/verify step, then they would need to modify that code to switch between these approaches.

[engelke]

As an app developer, doing this would not be a problem. But it seems to add complexity to the spec for browser developers, and would make all current implementation non-compliant.

I don't know all the issues, but (again as an app developer) this change does not appear worth making.

[ericroman920]

I am not in favor of this change.

I can't speak authoritatively to the cryptographic implications of not binding the hash at creation time.

...But from a compatibility perspective I am very concerned about the implications:

Chrome for years has supported the current version of the API which takes a hash in the sign/verify parameters rather than at key creation time.

Changing this now would cause problems, and would need a carefully thought out migration plan.

The obvious breakage is of course that code that used to work may now reject the promise, and require runtime tests to inspect which version of the API is being used. This is not great, but at least it is something web developers could account for. (There are also possibility of introducing bugs because sign/verify may or may not respect the hash passed in).

But my bigger concern is around CryptoKeys that have already been persisted to long-term storage (i.e. IndexedDB).

Because CryptoKey supports structured clone, the browser may have serialized it. What happens now upon de-serialization when inflating values from IndexedDB?

• Does de-serialization succeed/fail?
• Can these inflated keys be used with sign/verify? If so how: do we make the hash optional to support this use-case?

Lastly, CryptoKey's persisted via structured clone may be un-extractable. So a webdeveloper dealing with this migration wouldn't necessarily be able4 to do something like de-serialize, then exportKey, then importKey with hash to work around this.

If we proceed down this route, we need to be very thoughtful in how we allow for migration, since I don't think breaking the de-serialization of existing CryptoKey is an acceptable outcome.

Unless there is a clear security problem that needs to be fixed ([1] reaches a different conclusion), I don't think we should change it.

And if we do change it, I would probably advocate deprecating "ECDSA" and choosing a new algorithm name, instead of shuffling around the parameters (I think this makes it easier to code against and reason about given existing deployments).

[1] https://lists.w3.org/Archives/Public/public-webcrypto/2015Jan/0057.html

[mwatson2]

We have two comments against this change, so I propose to close as "won't fix".

@jimsch Do you agree ?

[ericroman920]

Won't Fix sounds good to me.

[jimsch]

@ericroman920 Does the same serialization problem exist with RSA keys?

[ericroman920]

The serialization issues applies to all key format : adding a new (required) parameter to any of the existing key types introduces compatibility questions surrounding what to do when de-serializing existing keys (whose provenance is generally IndexedDB)

As for RSA, all the keys that WebCrypto currently supports already have a mandatory hash parameter. (Especially after the removal of RSA-ES).

When we removed RSA-ES support in Chrome it did mean that de-serialization of such keys would fail, so there was the same sort of compatibility concern (but it was still early days and behind an experimental flag).

[jimsch]

I just wanted to make sure that RSA keys were serialized with the correct set of items.

I would rather that things be the same, I don't believe that an optional hash at creation would be reasonable however. I don't like it but it is probably to best course to won't fix it.

[ericroman920]

I just wanted to make sure that RSA keys were serialized with the correct set of items.

Correct. I believe this is required to conform the spec so we are good. Certainly Chrome serializes the RSA key's hash and enforces it when de-serialized via structured clone.