Define the ECDH length limit for supports("deriveBits")

[thibmeu]

SubtleCrypto.supports("deriveBits", algorithm, length) needs an clearer ECDH length rule.

For P-521, WebCrypto ECDH returns the x-coordinate. That is 66 bytes (ceil(521 / 8)), so deriveBits(..., 528) succeeds and returns a 66-byte buffer. 529 should fail.

The spec should say whether supports("deriveBits", { name: "ECDH", public }, length) uses that same byte-rounded maximum. Without that, implementations may treat P-521 as capped at 521 bits, even though deriveBits(..., 528) is valid.

This PR proposes text, but happy to convert to an issue for discussion

---

Preview | Diff

[kkoyung]

I think this length check is already covered by Step 8 of the check support for an algorithm algorithm, and we don't need an extra step for it, assuming my interpretation of specification is correct. (That said, I find the description of Step 8 is a little unclear, so please feel free to correct me if I'm misunderstanding it.)

My interpretation of Step 8 is that the implementation executes the target operation in a kind of "dry-run" mode, where it:

• Runs the target operation.
• Skips the step relies on an unavailable parameter, such as key, plaintext or ciphertext. (These cover all the computationally expensive cryptographic operations.)
• Returns false if the operation throws an error. Otherwise, returns true.

In the other words, the implementation would still perform all the validation on length, usages and parameters in normalizedAlgorithm, specified by the target operation.

Combining with the Step 8 of the ECDH deriveBits operation (https://w3c.github.io/webcrypto/#ecdh-operations-derive-bits),

8. If length is null:
       Return secret.
   Otherwise:
       If the length in bits of secret is less than length:
           throw an OperationError.
       Otherwise:
           Return a byte sequence containing the first length bits of secret.

the implementation would already check the length here.

[thibmeu]

The unclear part for me is that Step 8 says the implementation skips a step that relies on an unavailable parameter, and the ECDH deriveBits length check relies on secret.

For ECDH, though, the length in bits of secret can be determined from the available parameters, so this should still return false when length is too large.

Maybe Step 8 should say that a step is skipped only when the parameter or value it relies on cannot be determined during the dry run.

[kkoyung]

I see your point. The length of secret depends on secret itself, which relies on the unavailable key. Therefore, the length of secret may itself be considered as unavailable. However, the length of secret can determined by the available parameters without computing secret. It is unclear how this case should be handled.

That said, the whole "dry-run" interpretation is just my own guess. Indeed, it doesn't seems to be fully compatible with the current wording of step 8.

If the step relies on an unavailable parameter, such as key, plaintext or ciphertext:
    Return true. 

For example, since step 1 of the ECDH deriveBits operation relies on the unavailable parameter key, the above statement could be interpreted as returning true immediately, without executing any of the remaining steps of the operation. Under this interpretation, most of the operations are simply returning true.

It would probably be best to wait a response from the editors explaining the original intention behind step 8.

[twiss]

Hi 👋 Thanks for flagging this, and sorry for the delayed response.

The original intention behind Step 8 was to run the initial validation steps in the operations, and then stop. This is particularly relevant for the operations that check some field(s) of the algorithm struct, such as the PBKDF2 derive bits operation. This is a bit hacky but allows us not to copy those steps into this spec.

However, if the operation does some checks at the end, it indeed doesn't work.

Perhaps, rather than adding some ECDH-specific text to supports here, we could add a check to the beginning of the ECDH derive bits operation in w3c/webcrypto? That way, the implementation also doesn't have to perform the whole operation before finding out that it won't work.

Because the text there is quite general, we could similarly write something quite general and e.g. say something like:

If length is greater than the smallest multiple of 8 greater than or equal to field size of the curve specified by the namedCurve property of the [[algorithm]] internal slot of key, then throw an OperationError.

That should not be observably different (other than performance) from the current spec text, I think.

If that seems reasonable I can also make a PR due to the IPR issue, otherwise if someone else wants to do so go for it :)

[panva]

if someone else wants to do so go for it

w3c/webcrypto#558

[thibmeu]

the change in w3c/webcrypto#558 would address the issue indeed. happy to close this PR. thanks