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 <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
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 <gert@greenie.muc.de>

diff --git a/Changes.rst b/Changes.rst
index de2d8b0087b78414b534c67ed0674eb88744807a..9258230f1dfbc53453aeb3cc6aeeb08a6007c737 100644 (file)
--- 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 74f1139563fee940b600802e992d16c3c7f87df8..fb0d0dec4b2b8b0349fe79586ffafe001643ca3f 100644 (file)
--- 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 47acd97c45891949ab12ced633eeb8c0d7bca656..db1cfe35e0f9430f8e68623f0b6483a9f2a22e6d 100644 (file)
--- 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 34d163f622a0ea40a6def982fd08bd54d3ede058..f7217d38a4c834e0141932fce65ea7c7539e1f71 100644 (file)
--- 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 7938f41f5f819a0988f9292808364731e45e4a5d..8164bbcd4eb106f878eadea66fc807b8a8e2ddfc 100644 (file)
--- 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 */