From a7515f6d894e3bc9d836dcb31d7e6b3ab1219201 Mon Sep 17 00:00:00 2001
From: Gert Doering <gert@greenie.muc.de>
Date: Thu, 18 May 2017 12:05:48 +0200
Subject: [PATCH] Fix NCP behaviour on TLS reconnect.
If a client reconnects on a soft-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 <sbehrens@giantdisaster.de>
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
---
src/openvpn/push.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 8c3104e..bcef0ef 100644
|
a
|
b
|
prepare_push_reply(struct context *c, struct gc_arena *gc, |
| 372 | 372 | /* Push cipher if client supports Negotiable Crypto Parameters */ |
| 373 | 373 | if (tls_peer_info_ncp_ver(peer_info) >= 2 && o->ncp_enabled) |
| 374 | 374 | { |
| 375 | | /* if we have already created our key, we cannot change our own |
| 376 | | * cipher, so disable NCP and warn = explain why |
| | 375 | /* if we have already created our key, we cannot *change* our own |
| | 376 | * cipher -> so log the fact and push the "what we have now" cipher |
| | 377 | * (so the client is always told what we expect it to use) |
| 377 | 378 | */ |
| 378 | 379 | const struct tls_session *session = &tls_multi->session[TM_ACTIVE]; |
| 379 | 380 | if (session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized) |
| 380 | 381 | { |
| 381 | 382 | msg( M_INFO, "PUSH: client wants to negotiate cipher (NCP), but " |
| 382 | 383 | "server has already generated data channel keys, " |
| 383 | | "ignoring client request" ); |
| | 384 | "re-sending previously negotiated cipher '%s'", |
| | 385 | o->ciphername ); |
| 384 | 386 | } |
| 385 | 387 | else |
| 386 | 388 | { |
| … |
… |
prepare_push_reply(struct context *c, struct gc_arena *gc, |
| 388 | 390 | * TODO: actual negotiation, instead of server dictatorship. */ |
| 389 | 391 | char *push_cipher = string_alloc(o->ncp_ciphers, &o->gc); |
| 390 | 392 | o->ciphername = strtok(push_cipher, ":"); |
| 391 | | push_option_fmt(gc, push_list, M_USAGE, "cipher %s", o->ciphername); |
| 392 | 393 | } |
| | 394 | push_option_fmt(gc, push_list, M_USAGE, "cipher %s", o->ciphername); |
| 393 | 395 | } |
| 394 | 396 | else if (o->ncp_enabled) |
| 395 | 397 | { |
