Am 13.12.21 um 18:41 schrieb Gert Doering:
Acked-by: Gert Doering <g...@greenie.muc.de>Thanks to Selva for the v2 review, and thanks for the timely v3 + v4. I have tested this most thoroughly this time... - Linux, OpenSSL 1.1.1l (full client + server side tests) -> OK - Linux, mbedTLS 2.27.0 (full client + server side tests) -> ???? - Linux, OpenSSL 3.0.0 (full client side tests) -> OK - FreeBSD, OpenSSL 1.1.1h (fairly thorough client side tests) -> OK - FreeBSD, mbedTLS 2.16.11 (fairly thorough client side tests) -> and everything went well. Plus, stare-at code, which also looks good (Selva's points addressed). I have not fixed the "TYPE* x" vs. "TYPE *x" this round, as it will lead to extra rounds of conflicts with the later patchsets. We have agreed on IRC to do an uncrustify run when the "frame" set (xx/21) is in. Some observations and request for a followup patch: - crypto_mbedtls/cipher_ctx_init() has a sequence of events that need swapping + const mbedtls_cipher_info_t *kt = cipher_get(ciphername); + int key_len = kt->key_bitlen/8; + + ASSERT(kt); ... if kt is NULL, it won't reach the ASSERT()... (this is unlikely to happen, so not a showstopper today, but still) - crypto_openssl.c has functions that do not check the return value of cipher_get() (e.g. cipher_kt_iv_size()) and some that do (e.g. cipher_kt_block_size()) - is this intentional, due to "it is ok to pass NULL to EVP_CIPHER_iv_length()", or an oversight? - cipher_kt_mode_cbc(), cipher_kt_mode_ofb_cfb() and cipher_kt_mode_aead() do very similar things - except that the coding style of the third is totally different... - cipher_ctx_init() ASSERTs on ciphername & ctx, but not on *kt - intentional and guaranteed safe?
This was the same mess before but less obvious to be honest. Just now with instead blindly taking ctx having a cipher_get it is now become obvious that some of these functions expect their input to be always correct.
Arne _______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
