| Message ID | 1540991236-4016-1-git-send-email-steffan.karger@fox-it.com |
|---|---|
| State | Accepted |
| Delegated to: | Antonio Quartulli |
| Headers | show |
| Series | [Openvpn-devel] tls-crypt-v2: fix client reconnect bug | expand |
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)
Tested-by: Richard Bonhomme <tincanteksup@gmail.com>
Hi, On 31/10/2018 23:07, Steffan Karger wrote: > 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. > > Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> 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! Acked-by: Antonio Quartulli <antonio@openvpn.net>
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 <steffan.karger@fox-it.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1540991236-4016-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17866.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 39e8ca5..2a1b38e 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -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)
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. Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> --- src/openvpn/init.c | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-)