Hi, On Mon, Jul 13, 2020 at 08:33:03AM +0200, Gert Doering wrote: > On Mon, Jul 13, 2020 at 08:10:23AM +0200, Gert Doering wrote: > > Ouch. This is not good. My gut feeling is "2.3 with --enable-small = > > no OCC *and* no NCP = the server runs across a NULL pointer here". > > Bäm. Fully reproduceable here > > Program received signal SIGSEGV, Segmentation fault. > 0x00007ffff7af51be in ?? () from /lib64/libc.so.6 > (gdb) where > #0 0x00007ffff7af51be in ?? () from /lib64/libc.so.6 > #1 0x00005555555d4a7b in ncp_get_best_cipher (server_list=<optimized out>, > server_cipher=0x5555555f28da "BF-CBC", > peer_info=peer_info@entry=0x5555556781c0 > "IV_VER=2.3.18\nIV_PLAT=freebsd\nIV_PROTO=2\n", remote_cipher=0x0, > gc=gc@entry=0x55555565e070) at ssl_ncp.c:231
... and this is why (added a msg() call):
2020-07-13 08:36:59 us=802772 ncp_get_best_cipher(), peer_ncp_list=,
tmp_ciphers=AES-256-GCM:AES-128-GCM:AES-128-CBC:AES-192-CBC:AES-256-CBC,
remote_cipher=(null), server_cipher=BF-CBC
if "remote_cipher" is NULL (= no OCC) we pass that to "strcmp()", and that
does not want it.
Returning NULL from ncp_get_best_cipher() if there is nothing the client
has to offer works fine, though it triggers this warning
2020-07-13 08:43:01 us=483904 cron2-freebsd-tc-amd64-23/194.97.140.21:30927
PUSH: No common cipher between server and client.Expect this connection not to
work. Server ncp-ciphers:
'AES-256-GCM:AES-128-GCM:AES-128-CBC:AES-192-CBC:AES-256-CBC', client supported
ciphers ''
which we might want to reword for this case ("No information about cipher
support received from client, cannot ensure correct operation" or so).
Patch appended.
Comments?
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
From c67f62a8ecd7d65af8d22bd65e86d1bcacaa5294 Mon Sep 17 00:00:00 2001
From: Gert Doering <g...@greenie.muc.de>
Date: Mon, 13 Jul 2020 08:54:28 +0200
Subject: [PATCH] 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" (no NCP and no OCC) case.
Signed-off-by: Gert Doering <g...@greenie.muc.de>
---
src/openvpn/multi.c | 16 ++++++++++++----
src/openvpn/ssl_ncp.c | 6 ++++++
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index a2af071a..3302959e 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1833,10 +1833,18 @@ 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, "PUSH: No cipher info received from client "
+ "(no NCP and no OCC). Cannot ensure compatibility.");
+ }
gc_free(&gc);
}
}
diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c
index ea1dc960..fe7f5855 100644
--- 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, ":");
--
2.26.2
signature.asc
Description: PGP signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
