// The desired length of the authentication tag. May be 0 - 128. [EnforceRange] octet? tagLength; If the tagLength member of normalizedAlgorithm is not present or is null: Let tagLength be 128. If the tagLength member of normalizedAlgorithm is one of 32, 64, 96, 104, 112, 120 or 128: Let tagLength be equal to the tagLength member of normalizedAlgorithm Otherwise: Return an error named DataError.

Comment on attachment 8440260 [details] [diff] [review] 0003-Bug-1025463-AES-GCM-should-accept-a-zero-tagLength-p.patch Review of attachment 8440260 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/crypto/WebCryptoTask.cpp @@ +305,5 @@ > } > > // 32, 64, 96, 104, 112, 120 or 128 > mTagLength = 128; > + if (params.mTagLength.WasPassed() && params.mTagLength.Value() > 0) { The conditional you've added here means that if params.mTagLength == 0, then mTagLength will default to 128. That is not what the spec says. Here's the conditional in the spec that this branch needs to reflect: > If the tagLength member of normalizedAlgorithm is not present or is null: > Let tagLength be 128. > If the tagLength member of normalizedAlgorithm is one of 32, 64, 96, 104, 112, 120 or 128: > Let tagLength be equal to the tagLength member of normalizedAlgorithm > Otherwise: > Return an error named DataError. So if tagLength (params.mTagLength) is zero, then it is present and it's not in the specified set, so we should return DataError. @@ +310,5 @@ > mTagLength = params.mTagLength.Value(); > if ((mTagLength > 128) || > !(mTagLength == 32 || mTagLength == 64 || > (mTagLength >= 96 && mTagLength % 8 == 0))) { > + mEarlyRv = NS_ERROR_DOM_DATA_ERR; This is correct, but shouldn't be in this bug. Could you take a look through to verify that the other error types are aligned with the spec, then file a bug to correct them all?

In other words, the comment is wrong (0-128), and the conditional is correct.

You're absolutely right. Sorry for the extra work...