tls-crypt-v2: fix client reconnect bug
PATCH 1/1 tls-crypt-v2: fix client reconnect bug
As reported by tincantech on the openvpn-devel IRC channel, a tls-crypt-v2 client could be caused to trigger an assert in tls_crypt_wrap() because the client key might not be correctly initialized after a reconnect attempt.
This was caused by code that was written before the connection-block tls-auth/tls-crypt logic was integrated (57d6f103), rebased on that change, but not sufficiently changed to be compatible with the new logic.
This commit fixes that bug.
Note that I also moved the violating hunk of code to the same function where the tls-auth and tls-crypt (v1) keys are initialized. Once moved there, it is immediately clear that v2 didn't follow the same (new) logic.
--- src/openvpn/init.c | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-)
src/openvpn/init.c +23 -21
@@ -2594,6 +2594,26 @@ do_init_tls_wrap_key(struct context *c) options->ce.tls_crypt_file, options->ce.tls_crypt_inline, options->tls_server); } + +/* tls-crypt with client-specific keys (--tls-crypt-v2) */ +if (options->ce.tls_crypt_v2_file) + { +if (options->tls_server) + { +tls_crypt_v2_init_server_key (&c->c1.ks.tls_crypt_v2_server_key, + true, options->ce.tls_crypt_v2_file, + options->ce.tls_crypt_v2_inline); + } +else + { +tls_crypt_v2_init_client_key (&c->c1.ks.tls_wrap_key, + &c->c1.ks.tls_crypt_v2_wkc, + options->ce.tls_crypt_v2_file, + options->ce.tls_crypt_v2_inline); + } + } + + }/*
@@ -2645,27 +2665,9 @@ do_init_crypto_tls_c1(struct context *c)/* Initialize PRNG with config-specified digest */ prng_init (options->prng_hash, options->prng_nonce_secret_len); -/* initialize tls-auth/crypt key */ +/* initialize tls-auth/crypt/crypt-v2 key */ do_init_tls_wrap_key (c); -/* tls-crypt with client-specific keys (--tls-crypt-v2) */ -if (options->tls_crypt_v2_file) - { -if (options->tls_server) - { -tls_crypt_v2_init_server_key (&c->c1.ks.tls_crypt_v2_server_key, - true, options->tls_crypt_v2_file, - options->tls_crypt_v2_inline); - } -else - { -tls_crypt_v2_init_client_key (&c->c1.ks.tls_wrap_key, - &c->c1.ks.tls_crypt_v2_wkc, - options->tls_crypt_v2_file, - options->tls_crypt_v2_inline); - } - } -#if 0 /* was: #if ENABLE_INLINE_FILES -- Note that enabling this code will break restarts */ if (options->priv_key_file_inline) {
@@ -2891,13 +2893,13 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) to.tls_wrap.opt.flags |=CO_PACKET_ID_LONG_FORM ;tls_crypt_adjust_frame_parameters (&to.frame); -if (options->tls_crypt_v2_file) +if (options->ce.tls_crypt_v2_file) { to.tls_wrap.tls_crypt_v2_wkc = &c->c1.ks.tls_crypt_v2_wkc; } } -if (options->tls_crypt_v2_file) +if (options->ce.tls_crypt_v2_file) { to.tls_crypt_v2 = true;if (options->tls_server)
Signature · 1 line
2.7.4
Tested this with Ubuntu client, patched applied & Arch server, current master and it worked correctly.
Client did not crash when trying to "reconnect" to a server when the server had been changed from TLS Crypt V2 to V1.
When the server was changed back to V2 the client connected successfully. The client ran the same instance throughout.
(Would test on Windows but can't since my old rig died)
Hi,
Quoted 8 lines · Steffan Karger
As reported by tincantech on the openvpn-devel IRC channel, a tls-crypt-v2 client could be caused to trigger an assert in
tls_crypt_wrap()because the client key might not be correctly initialized after a reconnect attempt.This was caused by code that was written before the connection-block tls-auth/tls-crypt logic was integrated (
57d6f103), rebased on that change, but not sufficiently changed to be compatible with the new logic.
Quoted 7 lines
This commit fixes that bug.
Note that I also moved the violating hunk of code to the same function where the tls-auth and tls-crypt (v1) keys are initialized. Once moved there, it is immediately clear that v2 didn't follow the same (new) logic.
Signed-off-by Steffan Karger <[email protected]...>
Yeah, it probably was a "conflict" that went unnoticed.
We now need to rely on the data stored in the Connection Entry (ce member of the options structure) as the tls-crypt* logic is "per connection block" and not global anymore.
I performed some basic testing and all seems good. Thanks for fixing this!
Signature · 1 line
Antonio Quartulli
Your patch has been applied to the master branch.
(Gave it a test run, but didn't really look hard at the code)
commit 19d6d9c3533b83f934ea93359bca086a5d06011a
Author: Steffan Karger Date: Wed Oct 31 14:07:16 2018 +0100
tls-crypt-v2: fix client reconnect bug
Signed-off-by: Steffan Karger <[email protected]...>
Acked-by: Antonio Quartulli <[email protected]...>
Message-Id: <[email protected]...>
URL: https://www.mail-archive.com/[email protected]/msg17866.html
Signed-off-by: Gert Doering <[email protected]...>--
kind regards,
Gert Doering