Acked-by: Gert Doering <[email protected]>
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 <[email protected]>
Acked-by: Gert Doering <[email protected]>
Message-Id: <[email protected]>
URL:
https://www.mail-archive.com/[email protected]/msg23278.html
Signed-off-by: Gert Doering <[email protected]>
--
kind regards,
Gert Doering
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
