Closed rapier1 closed 2 years ago
This doesn't cause EVP_CipherInit to fail though. Instead it returns 0 and it looks like everything is working.
But a 0 return from EVP_CipherInit
is a failure? It should return 1 for success?
Apologies for the mistake. It returns 1. I'm working with the OpenSSH code and it gets a little twisty at times. I updated the original question. Anyway, for example,
if (EVP_CipherInit(cc->evp, type, NULL, (u_char *)iv, (do_encrypt == CIPHER_ENCRYPT)) == 0) { ret = SSH_ERR_LIBCRYPTO_ERROR; goto out; }
Does not fail in this circumstance even though 'type' (evp_aes_ctr_mt) seems like it would be unknown under OpenSSL3 as there is no provider for it. Also, it's my assumption the problem is happening in CipherInit rather than elsewhere.
This is confirmed as a bug. The fix is in #19300. It has also been assessed as a low severity security issue (CVE-2022-3358). Since it is low severity we are fixing this in public as per our security policy.
The bug here is that the custom cipher is not being properly handled and instead of just using it, we try to fetch an equivalent cipher from a provider. Unfortunately the problem is exacerbated by the use of NID_undef
for the cipher NID when you call EVP_CIPHER_meth_new
. This is probably a bad idea (you should really use a "proper" NID for this). Unfortunately when we try to find an equivalent cipher to fetch we find the NID_undef which corresponds with the NULL cipher, and that is what ends up being used.
Matt,
Thanks for the update. Also, thanks again for your help with the providers issue. Chris
@rapier1...please can you drop me an email regarding this on matt@openssl.org? I don't seem to have your email address.
Hello,
I know this is a corner case and one that has been warned about but I thought it would be good to report this anyway.
Under 1.1.1 I was using a custom EVP so that we could implement a multithreaded AES_CTR in our code (hpnssh, a variant of openssh). This was defined as follows:
const EVP_CIPHER * evp_aes_ctr_mt(void) { static EVP_CIPHER *aes_ctr; aes_ctr = EVP_CIPHER_meth_new(NID_undef, 16/*block*/, 16/*key*/); EVP_CIPHER_meth_set_iv_length(aes_ctr, AES_BLOCK_SIZE); EVP_CIPHER_meth_set_init(aes_ctr, ssh_aes_ctr_init); EVP_CIPHER_meth_set_cleanup(aes_ctr, ssh_aes_ctr_cleanup); EVP_CIPHER_meth_set_do_cipher(aes_ctr, ssh_aes_ctr); EVP_CIPHER_meth_set_flags(aes_ctr, EVP_CIPH_CBC_MODE | EVP_CIPH_VARIABLE_LENGTH | EVP_CIPH_ALWAYS_CALL_INIT | EVP_CIPH_CUSTOM_IV); return (aes_ctr); }
This compiles with deprecated warnings under OpenSSL 3 but it does compile. However, interesting things happen when we try to use it. I am not entirely sure but what believe what is happening is that when we try to init this type (evp_aes_ctr_mt) the OpenSSL code looks for a provider for this, can't find one, and possibly returns NULL. This doesn't cause EVP_CipherInit to fail though. Instead it returns 1 and it looks like everything is working. Except that when we move data across this cipher the result is that everything is en clear. I confirmed this via a tcpdump between a server and client both built against OpenSSL 3.0.2 and was able to observe all of the data being transmitted across the wire unencrypted.
Additionally, a connection between a 1.1.1 and 3.0.2 did not work as each side saw the other as being corrupted. Which is what you'd expect if the above is true.
I know that using any of the meth_ functions in OpenSSL 3 is "being deprecated in OpenSSL 3.0, and users of these APIs should know that their use can likely bypass provider selection and configuration, with unintended consequences." However, this seems like a severe unintended consequence.
In any event, I'm not expecting a bug fix but I wanted to let you know in case you needed to know what some of these unintended consequences look like.