[Openvpn-devel,9/9] Initialise kt_cipher even when no crypto is enabled

Message ID 20211201180727.2496903-9-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel,1/9] Implement optional cipher in --data-ciphers prefixed with ? | expand

Commit Message

Arne Schwabe Dec. 1, 2021, 7:07 a.m. UTC
This avoids special casing the cipher none/auth none case in other
parts, e.g. in the upcoming buffer/frame rework.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/init.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Gert Doering Dec. 13, 2021, 10:23 a.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

I do not see the immediate benefit, but the commit message points toward
future simplification - so it falls under "always init everything
avoids special cases later", which is good :-)

I do not have a test case for this right now (TLS with --cipher none
does not excercise this code path), so I couldn't *really* test it - 
running openvpn / p2p without --secret does yield the correct warning 
(lots of them), does not crash, and sends nice encapsulated but unecrypted
packets over the wire...

2021-12-13 22:20:09 DEPRECATION: No tls-client or tls-server option in configuration detected. OpenVPN 2.7 will remove the functionality to run a VPN without TLS. See the examples section in the manual page for examples of a similar quick setup with peer-fingerprint.
2021-12-13 22:20:09 ******* WARNING *******: '--cipher none' was specified. This means NO encryption will be performed and tunnelled data WILL be transmitted in clear text over the network! PLEASE DO RECONSIDER THIS SETTING!
2021-12-13 22:20:09 ******* WARNING *******: '--auth none' was specified. This means no authentication will be performed on received packets, meaning you CANNOT trust that the data received by the remote side have NOT been manipulated. PLEASE DO RECONSIDER THIS SETTING!
2021-12-13 22:20:09 ******* WARNING *******: All encryption and authentication features disabled -- All data will be tunnelled as clear text and will not be protected against man-in-the-middle changes. PLEASE DO RECONSIDER THIS CONFIGURATION!

maybe this is overdoing it just so slightly, and we should set "warn"
to "false" here...?  But anyway, I assume that this will go away soonish
anyway.

Your patch has been applied to the master branch.

commit 02d8f792893965a653e6bc99e039e169ad70bef9
Author: Arne Schwabe
Date:   Wed Dec 1 19:07:27 2021 +0100

     Initialise kt_cipher even when no crypto is enabled

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


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 6c82c0dca..2c18313d6 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3068,9 +3068,15 @@  do_init_finalize_tls_frame(struct context *c)
  * No encryption or authentication.
  */
 static void
-do_init_crypto_none(const struct context *c)
+do_init_crypto_none(struct context *c)
 {
     ASSERT(!c->options.test_crypto);
+
+    /* Initialise key_type with auth/cipher "none", so the key_type struct is
+     * valid */
+    init_key_type(&c->c1.ks.key_type, "none", "none",
+                  c->options.test_crypto, true);
+
     msg(M_WARN,
         "******* WARNING *******: All encryption and authentication features "
         "disabled -- All data will be tunnelled as clear text and will not be "