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?


Your patch has been applied to the master branch.

commit ce2954a0ca3f352df8d1492f5a2f2f809d309918
Author: Arne Schwabe
Date:   Mon Dec 13 16:06:53 2021 +0100

     Remove cipher_kt_t and change type to const char* in API

     Signed-off-by: Arne Schwabe <a...@rfc2549.org>
     Acked-by: Gert Doering <g...@greenie.muc.de>
     Message-Id: <20211213150654.3993358-1-a...@rfc2549.org>
     URL: 
https://www.mail-archive.com/search?l=mid&q=20211213150654.3993358-1-a...@rfc2549.org
     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