AES-256-CBC-HMAC-SHA256 and similar ciphers are not recognized as authenticated ciphers

[seirdotexe]

Version

18.1.0

Platform

Microsoft Windows NT 10.0.19044.0 x64

Subsystem

crypto

What steps will reproduce the bug?

Run the following code:

import crypto from 'crypto';

let cipher = crypto.createCipheriv('AES-256-CBC-HMAC-SHA256', crypto.randomBytes(32), crypto.randomBytes(16));
let encrypted = cipher.update('My beautiful data', 'utf8', 'hex');
encrypted += cipher.final('hex');

How often does it reproduce? Is there a required condition?

This only reproduces when using:

• aes-128-cbc-hmac-sha1
• aes-128-cbc-hmac-sha256
• aes-256-cbc-hmac-sha1
• aes-256-cbc-hmac-sha256

What is the expected behavior?

On Node 17.1.0:

d8d390a8554e2ca579a1946447c8144509495f521458996db711dd6c9595080d

What do you see instead?

node:internal/crypto/cipher:180
  const ret = this[kHandle].update(data, inputEncoding);
                            ^

Error: Trying to add data in unsupported state
    at Cipheriv.update (node:internal/crypto/cipher:180:29)
    at file:///C:/Users/myuser/Documents/GitHub/something/test/playground.js:22:24
    at ModuleJob.run (node:internal/modules/esm/module_job:198:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:409:24)
    at async loadESM (node:internal/process/esm_loader:85:5)
    at async handleMainPromise (node:internal/modules/run_main:61:12)

Additional information

It seems to work on LTS and above (tested until 17.1.0), but not on V18.

[tniessen]

I am not sure if these algorithms ever worked as expected even when they weren't throwing exceptions, I'll add the label again once I've figured that out. Could you describe your use case for these algorithms?

[seirdotexe]

I'm currently working on a project where I have to encrypt cookies using HMAC and AES. Of course I can use createHmac and separate the process, but these algorithms seem more convenient for me. I used the crypto.getCiphers function and found them and I was expecting them to work on the latest Node, but to my surprise this wasn't the case.

All in all not really breaking anything, but would be nice to have included I guess.

[tniessen]

Looks like it's OpenSSL 3. Node.js versions using OpenSSL 1.1.1 do not throw.

[tniessen]

Upon closer inspection, Node.js has never treated these algorithms as proper AEAD ciphers. In versions that do not throw when using them, they do not guarantee authenticity.

[seirdotexe]

they do not guarantee authenticity

Interesting discovery that you've made, let's hope it can be fixed

[tniessen]

Arguably, OpenSSL is also treating these strangely. Maybe these ciphers mostly exist for TLS compatibility?

Can I ask if you are following some spec that demands using these ciphers?

[seirdotexe]

I'm not following any spec. Just saw them and by their name I expected it to give me an HMAC hash alongside encrypting my data using AES, which is exactly what I was looking for.

[tniessen]

I'm not following any spec. Just saw them and by their name I expected it to give me an HMAC hash alongside encrypting my data using AES, which is exactly what I was looking for.

@seirdotexe In that case, I'd strongly suggest AES-GCM instead. It's widely supported (even in client-side Web Crypto) and generally more efficient than AES-CBC + HMAC.

[seirdotexe]

Yes I'm aware of this but for reasons I can't use it. Anyway I have no other knowledge to pass through about this subject and maybe it's not even worth looking further into this?

[tniessen]

I'd say that your expectation was valid and that action is required. I am not quite sure what to do yet, we probably either need to explicitly mark it as unsupported or guarantee authenticity. Thank you for the report! :)

[AdamMajer]

Upon closer inspection, Node.js has never treated these algorithms as proper AEAD ciphers. In versions that do not throw when using them, they do not guarantee authenticity.

I'm not quite sure what this means. The cipher is authenticated with the hash, so things can't just be changed randomly, right?

But yes, this is openssl3 related and it has to do with block size. So if this test case is changed to have

let encrypted = cipher.update('My beautiful dat', 'utf8', 'hex');

Then it will work with OpenSSL3 also.... so the question is are we doing things wrong in the first place or is this openssl3 bug?

[AdamMajer]

This bug seems to be related to block size handing of block ciphers with this update/final interface. But requiring only block sizes in the update() function seems very strange behaviour. Autopadding is only applicable on final() call anyway.

[AdamMajer]

Finally, it seems HMAC in the cipher seems like a misnomer here?. I'm getting same ciphertext results as without the hmac

EVP_aes_128_cbc_hmac_sha256() vs. EVP_aes_128_cbc(). The only difference is that the latter works in OpenSSL3 with inputs not multiple of block size. :shrug:

Maybe some OpenSSL expert can look into this more closely.

[bnoordhuis]

@AdamMajer if I understand your question correctly: node should call EVP_CIPHER_CTX_ctrl(EVP_CTRL_AEAD_SET_MAC_KEY) but currently doesn't.

Having said that... this is what the openssl docs for EVP_aes_128_cbc_hmac_sha256() say:

WARNING: this is not intended for usage outside of TLS and requires calling of some undocumented ctrl functions. These ciphers do not conform to the EVP AEAD interface.

[AdamMajer]

In light of this, my proposal to fixing this is to blacklist these 4 ciphers in the Cipher interface.

    aes-128-cbc-hmac-sha1
    aes-128-cbc-hmac-sha256
    aes-256-cbc-hmac-sha1
    aes-256-cbc-hmac-sha256

Clearly anyone using these from within Node will be using them incorrectly.

[bnoordhuis]

Sounds like a good idea, +1.

[ronnieroyston]

CBC is not an authenticated AES cipher mode. GCM is. GCM RFC "AES-GCM is an authenticated encryption".

It is possible to implement or bolt on authenticated encryption for CBC mode, e.g. "AES-256-CBC-HMAC-SHA256", but these are purpose built for usage in TLS.

WARNING: this [AES-256-CBC-HMAC-SHA256] is not intended for usage outside of TLS and requires calling of some undocumented ctrl functions. OpenSSL MAN Page

For example, a "Key Encryption Key" (KEK) is the term for the encryption key used to encrypt (authenticate) the "Content Encryption Key" (CEK) so it seems the .createCipheriv method would need to accept additional arguments in order to facilitate implementation of authenticated encryption schemes such as AES-256-CBC-HMAC-SHA256.

The JSON Web Encryption RFC includes A.3. _Example JWE Using AES Key Wrap and AES_128_CBC_HMAC_SHA256. JWS/JWE/JWT may present a larger use case for this type of cipher.

The SubtleCrypto API / crypto.subtle library as documented on MDN indicates that implementing authenticated CBC encryption is error-prone and GCM is therefore preferred.

GCM does provide built-in authentication, and for this reason it's often recommended over the other two AES modes. -MDN

[bnoordhuis]

@ronnieroyston I'm unsure what the point is you're trying to get across.

[ronnieroyston]

@ronnieroyston I'm unsure what the point is you're trying to get across.

What I was saying is: (probably slightly delirious from hours of crypto research screentime) I agree. AES in CBC mode is in fact not an authenticated cipher.