From: Stefan Behrens <sbehrens@giantdisaster.de>
Date: Wed, 17 May 2017 17:03:09 +0200
Subject: [PATCH] Fixup push message after TLS soft reset for NCP case
Trac #887 describes an issue where the push message to the
client is missing the cipher selection. The server has already
generated the data channel keys and the client wants to use
NCP to negotiate the cipher. The server is not including the
selected cipher in the push message in this case, with the
result that server and client use mismatching ciphers.
This commit fixes this up by including the cipher that the
server is using in the push message.
Works for me, tested with "reneg-sec 180" in the server config
to speed up the TLS soft reset.
This is the server log that shows the missing cipher in the
push message when the keys are already generated and NCP is
enabled:
TLS: soft reset sec=0 bytes=23248/-1 pkts=237/0
TLS: Username/Password authentication succeeded for username 'foo' [CN SET]
Data Channel Encrypt: Cipher 'AES-256-GCM' initialized with 256 bit key
Data Channel Decrypt: Cipher 'AES-256-GCM' initialized with 256 bit key
TLS: move_session: dest=TM_ACTIVE src=TM_UNTRUSTED reinit_src=1
TLS: tls_multi_process: untrusted session promoted to semi-trusted
Control Channel: TLSv1.2, cipher TLSv1/SSLv3 ECDHE-RSA-AES256-GCM-SHA384
PUSH: Received control message: 'PUSH_REQUEST'
PUSH: client wants to negotiate cipher (NCP), but server has already generated data channel keys, ignoring client request
SENT CONTROL [foo]: 'PUSH_REPLY,ping 45,route 3.2.1.69,route 3.2.1.72,route 3.2.1.44,route 10.144.0.1,topology net30,ifconfig 10.144.0.6 10.144.0.5,peer-id 0' (status=1)
AEAD Decrypt error: cipher final failed
Signed-off-by: Stefan Behrens <sbehrens@giantdisaster.de>
---
src/openvpn/push.c | 30 +++++++++++++++++++++++++++---
1 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 8c3104ed7690..7727cfafbf55 100644
|
a
|
b
|
prepare_push_reply(struct context *c, struct gc_arena *gc, |
| 378 | 378 | const struct tls_session *session = &tls_multi->session[TM_ACTIVE]; |
| 379 | 379 | if (session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized) |
| 380 | 380 | { |
| 381 | | msg( M_INFO, "PUSH: client wants to negotiate cipher (NCP), but " |
| 382 | | "server has already generated data channel keys, " |
| 383 | | "ignoring client request" ); |
| | 381 | const struct key_ctx_bi *key = |
| | 382 | &session->key[KS_PRIMARY].crypto_options.key_ctx_bi; |
| | 383 | const char *cipher_encrypt = |
| | 384 | translate_cipher_name_to_openvpn( |
| | 385 | cipher_kt_name(cipher_ctx_get_cipher_kt( |
| | 386 | key->encrypt.cipher))); |
| | 387 | const char *cipher_decrypt = |
| | 388 | translate_cipher_name_to_openvpn( |
| | 389 | cipher_kt_name(cipher_ctx_get_cipher_kt( |
| | 390 | key->decrypt.cipher))); |
| | 391 | |
| | 392 | if (strcmp(cipher_encrypt, cipher_decrypt) == 0) |
| | 393 | { |
| | 394 | msg( M_INFO, "PUSH: client wants to negotiate cipher (NCP), " |
| | 395 | "but server has already generated data channel keys, " |
| | 396 | "ignoring client request, selecting %s", |
| | 397 | cipher_encrypt ); |
| | 398 | push_option_fmt(gc, push_list, M_USAGE, "cipher %s", |
| | 399 | cipher_encrypt); |
| | 400 | } |
| | 401 | else |
| | 402 | { |
| | 403 | msg( M_INFO, "PUSH: client wants to negotiate cipher (NCP), " |
| | 404 | "but server has already generated data channel keys, " |
| | 405 | "and encrypt/decrypt ciphers %s/%s mismatch, ignoring " |
| | 406 | "client request", cipher_encrypt, cipher_decrypt ); |
| | 407 | } |
| 384 | 408 | } |
| 385 | 409 | else |
| 386 | 410 | { |
