From: Gert Doering Date: Thu, 18 May 2017 10:22:46 +0000 (+0200) Subject: Fix NCP behaviour on TLS reconnect. X-Git-Tag: v2.5_beta1~690 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5634cecf71ee9a92227bc9c8414c614d1b741abb;p=thirdparty%2Fopenvpn.git Fix NCP behaviour on TLS reconnect. If a client reconnects on a hard-restart from the same port (due to --bind in use on the client), both sides will handle this as a "reconnect" and not a "full new connect" internally, re-using existing crypto context. The client will still ask the server for pushed options, and the server code to handle this refuses to do NCP if a key has already been negotiated (because there is no way to *change* the cipher after that) - which ends up in "the client uses the non-negotiated cipher from the config file, while the server uses the previously-negotiated NCP cipher", and nothing works. The easy workaround: if we find us in the situation that we think NCP has already been done, just re-push "cipher o->ciphername" with the current cipher for this client context. All credits for this go to Stefan Behrens who found and diagnosed the issue in trac #887, came up with a first patch to solve the issue quite similar to this (simplified) one, and helped testing. Trac: #887 Acked-by: Steffan Karger Message-Id: <20170518102246.5496-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14666.html Signed-off-by: Gert Doering --- diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 8c3104ed7..bcef0ef49 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -372,15 +372,17 @@ prepare_push_reply(struct context *c, struct gc_arena *gc, /* Push cipher if client supports Negotiable Crypto Parameters */ if (tls_peer_info_ncp_ver(peer_info) >= 2 && o->ncp_enabled) { - /* if we have already created our key, we cannot change our own - * cipher, so disable NCP and warn = explain why + /* if we have already created our key, we cannot *change* our own + * cipher -> so log the fact and push the "what we have now" cipher + * (so the client is always told what we expect it to use) */ const struct tls_session *session = &tls_multi->session[TM_ACTIVE]; if (session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized) { msg( M_INFO, "PUSH: client wants to negotiate cipher (NCP), but " "server has already generated data channel keys, " - "ignoring client request" ); + "re-sending previously negotiated cipher '%s'", + o->ciphername ); } else { @@ -388,8 +390,8 @@ prepare_push_reply(struct context *c, struct gc_arena *gc, * TODO: actual negotiation, instead of server dictatorship. */ char *push_cipher = string_alloc(o->ncp_ciphers, &o->gc); o->ciphername = strtok(push_cipher, ":"); - push_option_fmt(gc, push_list, M_USAGE, "cipher %s", o->ciphername); } + push_option_fmt(gc, push_list, M_USAGE, "cipher %s", o->ciphername); } else if (o->ncp_enabled) {