Restrict authentication tag length for GCM by default

[Starkteetje]

What is the problem this feature will solve?

When using the node crypto module to decrypt data with Galois Counter Mode (GCM), the expected authentication tag length is not set by default in the Decipher class. As a consequence, code that is is safe for other modes such as CCM or algorithms like ChaChaPoly is not safe.

As an example

function decrypt(key, iv, ciphertext, tag) {
    decipher = createDecipheriv("aes-256-gcm", key, iv)
    decipher.setAuthTag(tag)
    return Buffer.concat([decipher.update(ciphertext), decipher.final()])
}

is insecure, because the correct 4-byte prefix of the actual tag will be accepted, whereas

function decrypt(key, iv, ciphertext, tag) {
    decipher = createDecipheriv("chacha20-poly1305", key, iv)
    decipher.setAuthTag(tag)
    return Buffer.concat([decipher.update(ciphertext), decipher.final()])
}

is safe, because for ChachaPoly, the expected authentication tag length defaults to 16. For CCM, the code will throw, because it requires passing an option:

function decrypt(key, iv, ciphertext, tag) {
    decipher = createDecipheriv("aes-256-ccm", key, iv, {authTagLength:16})
    decipher.setAuthTag(tag)
    return Buffer.concat([decipher.update(ciphertext), decipher.final()])
}

which is supported, but optional for GCM.

As mentioned above, crypto accepts multiple tag length values for GCM, in particular 4 bytes. 4 bytes is brute-forceable, though practical exploitability depends on the context, e.g. if user interaction is required, it won't be practical.

This isn't a security vulnerability with Node as it is documented behaviour and was explicitly decided in #17523. However, this leads to many implementations in the wild being insecure. I did stumble upon this during a code review and did a small survey of open source projects and among 41 projects on GitHub with >100 stars that used the node module for actual decryption (not benchmarking, no CTFs, not test-only), only 7 implementations did fully protect against short authentication tags from being passed to the Decipher object, while 15 implementations would allow arbitrary forgeries in at most 2^32 queries. This query number might be significantly lower depending on the application based on a paper by Ferguson. The other implementations allow some form of breach of cryptographic primitives that isn't interesting in practice, i.e. brute-forcing the tag for the empty message.

What is the feature you are proposing to solve the problem?

Given this wide-spread misuse of the API (answering @tniessen s comment/question on the original issue whether people will use the API correctly in the negative) paired with NIST's recent announcement to revise SP 800-38D to disallow authentication tags of size less than 96 bits and the fact that all other authenticated modes in Node crypto either default to a reasonable expected tag size (ChachaPoly) or enforce that the expected tag size be specified (OCB, CCM), it might be reasonable to re-assess the decision from #17523 to keep the expected tag length optional.

The feature I would propose would be the Chacha-way, defaulting to an expected tag length of the same size as is generated by the Cipher object, i.e. 16 bytes. This way, basically none of the applications using GCM would be affected (I encoutered none that created tags of size less than 16) in a breaking way, while fixing the security vulernability for them. This would effectively turn the API into secure-by-default, while still permitting users with other tag length needs, to specify this.
Note, that this is still a breaking change.

What alternatives have you considered?

• The fix could also be done in the CCM/OCB way, i.e. throwing if no expected length is passed. That would break all existing insecure implementations and even some of the secure ones that have custom length checks on the tag.
• Warn on any use of a Decipher object with a GCM mode and no specified authTagLength option. While that is noisy, it still doesn't solve the current insecure-by-default state. However, it is the only improvement that isn't a breaking change, that I can come up with

[tniessen]

The proposed solution sounds pretty much like what I suggested in #17523 (comment) in 2018. We did not implement it due to the possibility of unnecessarily breaking secure uses. Theoretically, there could also be some applications that do not know the authentication tag length until they receive it, but none come to mind. (And this would go against NIST recommendations, I suppose.)

We did make gradual improvements, which are summarized in #17523 (comment). Before then, Node.js allowed any tag length, even if the tag was a single byte only.

If we do accept further breakage, we could deprecate authentication tags shorter than 128 bits for any Decipher object that was created without specifying the authTagLength option, and then later let authTagLength default to 128 bits.

(If there are applications that cannot migrate to this, we could allow an explicit null value to be passed for authTagLength, which would restore the current behavior.)

WDYT @bnoordhuis?

[github-actions]

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the never-stale Mark issue so that it is never considered stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.

[tniessen]

#52552 should be released in Node.js 23 later this month. If there is no major breakage, I will follow up in a few months.