Handle connecting clients without NCP or OCC without crashing.

ssl_ncp.c:ncp_get_best_cipher() would crash if a client connects without
NCP (or with a NCP cipher list that does not contain the first NCP cipher
in the server list) due to a NULL pointer strcmp().

Work around / fix by just assigning an empty string to remote_cipher here
("not NULL but will never match either").

Add new warning message in multi.c for the "we do not know what the
client can do" case (no NCP and non-helpful OCC), rewrapped the existing
message to keep line lenght limit.

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

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index a2af071a22beb35b821265a0efe1ba0b6cca15a9..c2ffcb9db3c3c0436fb2bf790067f38aab61e4d7 100644 (file)
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1833,10 +1833,19 @@ multi_client_set_protocol_options(struct context *c)
             {
                 struct gc_arena gc = gc_new();
                 const char *peer_ciphers = tls_peer_ncp_list(peer_info, &gc);
-                msg(M_INFO, "PUSH: No common cipher between server and client."
-                    "Expect this connection not to work. "
-                    "Server ncp-ciphers: '%s', client supported ciphers '%s'",
-                    o->ncp_ciphers, peer_ciphers);
+                if (strlen(peer_ciphers) > 0)
+                {
+                    msg(M_INFO, "PUSH: No common cipher between server and "
+                        "client. Expect this connection not to work. Server "
+                        "ncp-ciphers: '%s', client supported ciphers '%s'",
+                        o->ncp_ciphers, peer_ciphers);
+                }
+                else
+                {
+                    msg(M_INFO, "No NCP data received from peer, falling back "
+                        "to --cipher '%s'. Peer reports in OCC --cipher '%s'",
+                        o->ciphername, np(tls_multi->remote_ciphername));
+                }
                 gc_free(&gc);
             }
         }

diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c
index ea1dc960bc83aab4e3a80b3b0a52f0e7aae96369..e057a40b812887c4d19ce4a7bce872dc47eebc08 100644 (file)
--- a/src/openvpn/ssl_ncp.c
+++ b/src/openvpn/ssl_ncp.c
@@ -225,6 +225,12 @@ ncp_get_best_cipher(const char *server_list, const char *server_cipher,
 
     const char *peer_ncp_list = tls_peer_ncp_list(peer_info, &gc_tmp);
 
+    /* non-NCP client without OCC?  "assume nothing" */
+    if (remote_cipher == NULL)
+    {
+        remote_cipher = "";
+    }
+
     char *tmp_ciphers = string_alloc(server_list, &gc_tmp);
 
     const char *token = strsep(&tmp_ciphers, ":");