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 <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
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
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 <gert@greenie.muc.de>
... | ... |
@@ -372,15 +372,17 @@ 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 |
{ |
... | ... |
@@ -388,8 +390,8 @@ prepare_push_reply(struct context *c, struct gc_arena *gc, |
388 | 388 |
* TODO: actual negotiation, instead of server dictatorship. */ |
389 | 389 |
char *push_cipher = string_alloc(o->ncp_ciphers, &o->gc); |
390 | 390 |
o->ciphername = strtok(push_cipher, ":"); |
391 |
- push_option_fmt(gc, push_list, M_USAGE, "cipher %s", o->ciphername); |
|
392 | 391 |
} |
392 |
+ push_option_fmt(gc, push_list, M_USAGE, "cipher %s", o->ciphername); |
|
393 | 393 |
} |
394 | 394 |
else if (o->ncp_enabled) |
395 | 395 |
{ |