Browse code

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>

Steffan Karger authored on 2016/12/08 03:01:24
Showing 5 changed files
... ...
@@ -271,6 +271,11 @@ User-visible Changes
271 271
   /etc/openvpn/client (depending on unit file).  This also avoids these new
272 272
   unit files and how they work to collide with older pre-existing unit files.
273 273
 
274
+- using ``--no-iv`` (which is generally not a recommended setup) will
275
+  require explicitly disabling NCP with ``--disable-ncp``.  This is
276
+  intentional because NCP will by default use AES-GCM, which requires
277
+  an IV - so we want users of that option to consciously reconsider.
278
+
274 279
 
275 280
 Maintainer-visible changes
276 281
 --------------------------
... ...
@@ -2341,9 +2341,9 @@ do_init_crypto_tls (struct context *c, const unsigned int flags)
2341 2341
   if (options->mute_replay_warnings)
2342 2342
     to.crypto_flags |= CO_MUTE_REPLAY_WARNINGS;
2343 2343
 
2344
-  to.crypto_flags_and = ~(CO_PACKET_ID_LONG_FORM);
2344
+  to.crypto_flags &= ~(CO_PACKET_ID_LONG_FORM);
2345 2345
   if (packet_id_long_form)
2346
-    to.crypto_flags_or = CO_PACKET_ID_LONG_FORM;
2346
+    to.crypto_flags |= CO_PACKET_ID_LONG_FORM;
2347 2347
 
2348 2348
   to.ssl_ctx = c->c1.ks.ssl_ctx;
2349 2349
   to.key_type = c->c1.ks.key_type;
... ...
@@ -2234,6 +2234,10 @@ options_postprocess_verify_ce (const struct options *options, const struct conne
2234 2234
     {
2235 2235
       msg (M_USAGE, "NCP cipher list contains unsupported ciphers.");
2236 2236
     }
2237
+  if (options->ncp_enabled && !options->use_iv)
2238
+    {
2239
+      msg (M_USAGE, "--no-iv not allowed when NCP is enabled.");
2240
+    }
2237 2241
 
2238 2242
   /*
2239 2243
    * Check consistency of replay options
... ...
@@ -881,9 +881,6 @@ key_state_init (struct tls_session *session, struct key_state *ks)
881 881
     }
882 882
 
883 883
   ks->crypto_options.pid_persist = NULL;
884
-  ks->crypto_options.flags = session->opt->crypto_flags;
885
-  ks->crypto_options.flags &= session->opt->crypto_flags_and;
886
-  ks->crypto_options.flags |= session->opt->crypto_flags_or;
887 884
 
888 885
 #ifdef MANAGEMENT_DEF_AUTH
889 886
   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)
1823 1823
 
1824 1824
   ASSERT (ks->authenticated);
1825 1825
 
1826
+  ks->crypto_options.flags = session->opt->crypto_flags;
1826 1827
   if (!generate_key_expansion (&ks->crypto_options.key_ctx_bi,
1827 1828
       &session->opt->key_type, ks->key_src, client_sid, server_sid,
1828 1829
       session->opt->server))
... ...
@@ -1857,9 +1855,9 @@ tls_session_update_crypto_params(struct tls_session *session,
1857 1857
       options->authname, options->keysize, true, true);
1858 1858
 
1859 1859
   bool packet_id_long_form = cipher_kt_mode_ofb_cfb (session->opt->key_type.cipher);
1860
-  session->opt->crypto_flags_and &= ~(CO_PACKET_ID_LONG_FORM);
1860
+  session->opt->crypto_flags &= ~(CO_PACKET_ID_LONG_FORM);
1861 1861
   if (packet_id_long_form)
1862
-    session->opt->crypto_flags_and = CO_PACKET_ID_LONG_FORM;
1862
+    session->opt->crypto_flags |= CO_PACKET_ID_LONG_FORM;
1863 1863
 
1864 1864
   /* Update frame parameters: undo worst-case overhead, add actual overhead */
1865 1865
   frame_add_to_extra_frame (frame, -(crypto_max_overhead()));
... ...
@@ -279,8 +279,6 @@ struct tls_options
279 279
 
280 280
   /* struct crypto_option flags */
281 281
   unsigned int crypto_flags;
282
-  unsigned int crypto_flags_and;
283
-  unsigned int crypto_flags_or;
284 282
 
285 283
   int replay_window;                   /* --replay-window parm */
286 284
   int replay_time;                     /* --replay-window parm */