From: Steffan Karger Date: Wed, 7 Dec 2016 18:01:24 +0000 (+0100) Subject: Fix (and cleanup) crypto flags in combination with NCP X-Git-Tag: v2.4_rc2~13 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=84f88ca4d57cd0dc40fd945e09ab1cea1b2cd0b7;p=thirdparty%2Fopenvpn.git Fix (and cleanup) crypto flags in combination with NCP tls_session_update_crypto_params() did not properly set crypto_flags_or, but instead set crypto_flags_and twice if a OFB/CFB mode was selected. Also, the crypto flags in ks->crypto_options.flags were set before tls_session_update_crypto_params() was called, causing those to not be adjusted. To fix this, set the crypto flags in tls_session_generate_data_channel_keys() instead of key_state_init(). While touching that code, remove the to _or and _and variables, which are not needed at all. Finally, refuse to accept --no-iv if NCP is enabled (we might otherwise negotiate invalid combinations and ASSERT out later, and using --no-iv is a bad idea anyway). Trac: #784 Signed-off-by: Steffan Karger Acked-by: Gert Doering Message-Id: <1481133684-5325-1-git-send-email-steffan@karger.me> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13428.html Signed-off-by: Gert Doering --- diff --git a/Changes.rst b/Changes.rst index de2d8b008..9258230f1 100644 --- a/Changes.rst +++ b/Changes.rst @@ -271,6 +271,11 @@ User-visible Changes /etc/openvpn/client (depending on unit file). This also avoids these new unit files and how they work to collide with older pre-existing unit files. +- using ``--no-iv`` (which is generally not a recommended setup) will + require explicitly disabling NCP with ``--disable-ncp``. This is + intentional because NCP will by default use AES-GCM, which requires + an IV - so we want users of that option to consciously reconsider. + Maintainer-visible changes -------------------------- diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 74f113956..fb0d0dec4 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2341,9 +2341,9 @@ do_init_crypto_tls (struct context *c, const unsigned int flags) if (options->mute_replay_warnings) to.crypto_flags |= CO_MUTE_REPLAY_WARNINGS; - to.crypto_flags_and = ~(CO_PACKET_ID_LONG_FORM); + to.crypto_flags &= ~(CO_PACKET_ID_LONG_FORM); if (packet_id_long_form) - to.crypto_flags_or = CO_PACKET_ID_LONG_FORM; + to.crypto_flags |= CO_PACKET_ID_LONG_FORM; to.ssl_ctx = c->c1.ks.ssl_ctx; to.key_type = c->c1.ks.key_type; diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 47acd97c4..db1cfe35e 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -2234,6 +2234,10 @@ options_postprocess_verify_ce (const struct options *options, const struct conne { msg (M_USAGE, "NCP cipher list contains unsupported ciphers."); } + if (options->ncp_enabled && !options->use_iv) + { + msg (M_USAGE, "--no-iv not allowed when NCP is enabled."); + } /* * Check consistency of replay options diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 34d163f62..f7217d38a 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -881,9 +881,6 @@ key_state_init (struct tls_session *session, struct key_state *ks) } ks->crypto_options.pid_persist = NULL; - ks->crypto_options.flags = session->opt->crypto_flags; - ks->crypto_options.flags &= session->opt->crypto_flags_and; - ks->crypto_options.flags |= session->opt->crypto_flags_or; #ifdef MANAGEMENT_DEF_AUTH ks->mda_key_id = session->opt->mda_context->mda_key_id_counter++; @@ -1823,6 +1820,7 @@ tls_session_generate_data_channel_keys(struct tls_session *session) ASSERT (ks->authenticated); + ks->crypto_options.flags = session->opt->crypto_flags; if (!generate_key_expansion (&ks->crypto_options.key_ctx_bi, &session->opt->key_type, ks->key_src, client_sid, server_sid, session->opt->server)) @@ -1857,9 +1855,9 @@ tls_session_update_crypto_params(struct tls_session *session, options->authname, options->keysize, true, true); bool packet_id_long_form = cipher_kt_mode_ofb_cfb (session->opt->key_type.cipher); - session->opt->crypto_flags_and &= ~(CO_PACKET_ID_LONG_FORM); + session->opt->crypto_flags &= ~(CO_PACKET_ID_LONG_FORM); if (packet_id_long_form) - session->opt->crypto_flags_and = CO_PACKET_ID_LONG_FORM; + session->opt->crypto_flags |= CO_PACKET_ID_LONG_FORM; /* Update frame parameters: undo worst-case overhead, add actual overhead */ frame_add_to_extra_frame (frame, -(crypto_max_overhead())); diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 7938f41f5..8164bbcd4 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -279,8 +279,6 @@ struct tls_options /* struct crypto_option flags */ unsigned int crypto_flags; - unsigned int crypto_flags_and; - unsigned int crypto_flags_or; int replay_window; /* --replay-window parm */ int replay_time; /* --replay-window parm */