Acked-by: Gert Doering <g...@greenie.muc.de>

Change makes sense, resulting code passes t_client tests (with and
without AES ciphers), on OpenSSL 1.1 and mbedTLS builds.  

Thanks for the explanation wrt the ASSERT() calls - indeed, if the 
caller already checks the cipher itself, no need to double-check it 
in the called function.

Reviewing the change to crypto_backend.h I noticed that the comments
seem to be "german invalid grammar" according to what I remember from 
school...

+ * @param ctx           Cipher's context. May not be NULL.

shouldn't that be "Must not be NULL" for "NULL is not allowed" here?
(There are many more of these, so it's not *this* patch introducing
the error - so, not changing these on the fly)


Another observation: cipher_ctx_mode_cbc() and cipher_ctx_mode_ofb_cfb()
seem to want the same thing ("no AEAD, no EVP_CIPH_FLAG_CTS") but 
do it in different ways - this looks a bit weird.

And then, cipher_ctx_mode_aead() does not use "early return" on
if (!ctx)... which is inconsistent to the previous two.

And *then*, _cbc() use EVP_CIPHER_CTX_get_mode(), while _ofb_cfb()
uses EVP_CIPHER_CTX_mode(), which seems to be the 3.0 <-> 1.1 variants,
and we do have a compat call...

Could these be unified in a followup patch, please?


Your patch has been applied to the master branch.

commit 0b1c721e4f19c84ea35a2e266c3913eb893355ec
Author: Arne Schwabe
Date:   Wed Dec 1 19:07:21 2021 +0100

     Remove cipher_ctx_get_cipher_kt and replace with direct context calls

     Signed-off-by: Arne Schwabe <a...@rfc2549.org>
     Acked-by: Gert Doering <g...@greenie.muc.de>
     Message-Id: <20211201180727.2496903-3-a...@rfc2549.org>
     URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23278.html
     Signed-off-by: Gert Doering <g...@greenie.muc.de>


--
kind regards,

Gert Doering



_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to