AES-CBC decrypt: Unspecified error type for ciphertext of odd length

[BenWiederhake]

Context: https://w3c.github.io/webcrypto/#aes-cbc-operations

Some observations:

• The decrypt operation does not explain how to handle errors in Step 2 ("… performing the CBC Decryption operation …"), seemingly assuming that no error can occur
• The decrypt operation goes on to explain step-by-step the details of padding removal, specifying that inconsistent padding should result in an OperationError, but remains silent about other error types.
• Most other situations that similarly call into a third-party library in order to perform some high-level action explicitly state the error type, e.g. ECDSA generateKey: "If performing the key generation operation results in an error, then throw an OperationError."
• The description of SubtleCrypto.importKey contains nothing about this specific detail. (As expected)

Suggestion: Insert between Step 1 and Step 2: "If ciphertext does not have a length that is a multiple of 16 bytes, then throw an OperationError." (Or some other Error type.)

Minimal-ish reproducer:

var keyData = new Uint8Array([
    0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f]);
var ivData = new Uint8Array([
    0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f]);
var ciphertextOdd = new Uint8Array([0x04, 0x05, 0x06, 0x07, 0x08]);
var aesAlgorithmKeyGen = { name: "AES-CBC", length: 128 };
var aesAlgorithmDecrypt = { name: "AES-CBC", iv: ivData };
var aesKey = await window.crypto.subtle.importKey("raw", keyData, aesAlgorithmKeyGen, false, ["decrypt"]);
await window.crypto.subtle.decrypt(aesAlgorithmDecrypt, aesKey, ciphertextOdd)
.then((res) => { console.log("success?!"); }, (err) => { console.log("failed with " + err.name); } );

I can't find a WPT test that covers this edge case. Note that bigPadChar, inconsistentPadChars, and zeroPadChar are different edge cases.

It seems that Firefox and Chrome agree that this should result in an OperationError. However, I'm not entirely confident that all the other implementors agree with that. Given that there is no WPT test for this, I don't want to assume that this as a given, even though OperationError is the obvious choice here.

EDIT: That's why I think this would be a substantive change, because it clears up an […] under-specified part of the specification in such a way that […] an agent whose conformance was once unclear becomes clearly either conforming or non-conforming, even though it's only a hypothetical such agent. For example, that hypothetical agent might raise a DataError.

I tried to sign up using the link you provided to me, @twiss, but it seems I'm not allowed to do so without a special affiliation. (I do not work for a W3C Member organization, and strongly prefer to not pull my employer into this. Therefore my only option is to "participate as an Invited Expert", which sounds bureaucratically and legally intense.)

Therefore, I'm unable to write a PR.

[twiss]

Thanks for the report! I also agree that throwing OperationError is the obvious outcome; unfortunately it seems Safari's current implementation doesn't agree and returns an empty ArrayBuffer for your test case (cc @nmahendru).

So, I agree it's technically a substantive change, though a fairly minor and hopefully non-controversial one. And clearly we should have a WPT test for this.

And, makes sense! I'll make a PR.