Browse code

Add proper check for crypto modes (CBC or OFB/CFB)

OpenSSL has added AEAD-CBC mode ciphers like AES-128-CBC-HMAC-SHA1, which
have mode EVP_CIPH_CBC_MODE, but require a different API (the AEAD API).
So, add extra checks to filter out those AEAD-mode ciphers.

Adding these made the crypto library agnostic function cfb_ofb_mode()
superfuous, so removed that on the go.

Also update all cipher mode checks to use the new cipher_kt_mode_*()
functions for consistency.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1402244175-31462-3-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/8779
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Steffan Karger authored on 2014/06/09 01:16:13
Showing 6 changed files
... ...
@@ -100,10 +100,10 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
100 100
 	{
101 101
 	  uint8_t iv_buf[OPENVPN_MAX_IV_LENGTH];
102 102
 	  const int iv_size = cipher_ctx_iv_length (ctx->cipher);
103
-	  const unsigned int mode = cipher_ctx_mode (ctx->cipher);
103
+	  const cipher_kt_t *cipher_kt = cipher_ctx_get_cipher_kt (ctx->cipher);
104 104
 	  int outlen;
105 105
 
106
-	  if (mode == OPENVPN_MODE_CBC)
106
+	  if (cipher_kt_mode_cbc(cipher_kt))
107 107
 	    {
108 108
 	      CLEAR (iv_buf);
109 109
 
... ...
@@ -119,7 +119,7 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
119 119
 		  ASSERT (packet_id_write (&pin, buf, BOOL_CAST (opt->flags & CO_PACKET_ID_LONG_FORM), true));
120 120
 		}
121 121
 	    }
122
-	  else if (mode == OPENVPN_MODE_CFB || mode == OPENVPN_MODE_OFB)
122
+	  else if (cipher_kt_mode_ofb_cfb(cipher_kt))
123 123
 	    {
124 124
 	      struct packet_id_net pin;
125 125
 	      struct buffer b;
... ...
@@ -171,7 +171,10 @@ openvpn_encrypt (struct buffer *buf, struct buffer work,
171 171
 	  /* Flush the encryption buffer */
172 172
 	  ASSERT(cipher_ctx_final(ctx->cipher, BPTR (&work) + outlen, &outlen));
173 173
 	  work.len += outlen;
174
-	  ASSERT (mode != OPENVPN_MODE_CBC || outlen == iv_size);
174
+
175
+	  /* For all CBC mode ciphers, check the last block is complete */
176
+	  ASSERT (cipher_kt_mode (cipher_kt) != OPENVPN_MODE_CBC ||
177
+	      outlen == iv_size);
175 178
 
176 179
 	  /* prepend the IV to the ciphertext */
177 180
 	  if (opt->flags & CO_USE_IV)
... ...
@@ -272,8 +275,8 @@ openvpn_decrypt (struct buffer *buf, struct buffer work,
272 272
 
273 273
       if (ctx->cipher)
274 274
 	{
275
-	  const unsigned int mode = cipher_ctx_mode (ctx->cipher);
276 275
 	  const int iv_size = cipher_ctx_iv_length (ctx->cipher);
276
+	  const cipher_kt_t *cipher_kt = cipher_ctx_get_cipher_kt (ctx->cipher);
277 277
 	  uint8_t iv_buf[OPENVPN_MAX_IV_LENGTH];
278 278
 	  int outlen;
279 279
 
... ...
@@ -320,7 +323,7 @@ openvpn_decrypt (struct buffer *buf, struct buffer work,
320 320
 
321 321
 	  /* Get packet ID from plaintext buffer or IV, depending on cipher mode */
322 322
 	  {
323
-	    if (mode == OPENVPN_MODE_CBC)
323
+	    if (cipher_kt_mode_cbc(cipher_kt))
324 324
 	      {
325 325
 		if (opt->packet_id)
326 326
 		  {
... ...
@@ -329,7 +332,7 @@ openvpn_decrypt (struct buffer *buf, struct buffer work,
329 329
 		    have_pin = true;
330 330
 		  }
331 331
 	      }
332
-	    else if (mode == OPENVPN_MODE_CFB || mode == OPENVPN_MODE_OFB)
332
+	    else if (cipher_kt_mode_ofb_cfb(cipher_kt))
333 333
 	      {
334 334
 		struct buffer b;
335 335
 
... ...
@@ -426,10 +429,9 @@ init_key_type (struct key_type *kt, const char *ciphername,
426 426
 
427 427
       /* check legal cipher mode */
428 428
       {
429
-	const unsigned int mode = cipher_kt_mode (kt->cipher);
430
-	if (!(mode == OPENVPN_MODE_CBC
429
+	if (!(cipher_kt_mode_cbc(kt->cipher)
431 430
 #ifdef ENABLE_OFB_CFB_MODE
432
-	      || (cfb_ofb_allowed && (mode == OPENVPN_MODE_CFB || mode == OPENVPN_MODE_OFB))
431
+	      || (cfb_ofb_allowed && cipher_kt_mode_ofb_cfb(kt->cipher))
433 432
 #endif
434 433
 	      ))
435 434
 #ifdef ENABLE_SMALL
... ...
@@ -606,18 +608,10 @@ fixup_key (struct key *key, const struct key_type *kt)
606 606
 void
607 607
 check_replay_iv_consistency (const struct key_type *kt, bool packet_id, bool use_iv)
608 608
 {
609
-  if (cfb_ofb_mode (kt) && !(packet_id && use_iv))
610
-    msg (M_FATAL, "--no-replay or --no-iv cannot be used with a CFB or OFB mode cipher");
611
-}
609
+  ASSERT(kt);
612 610
 
613
-bool
614
-cfb_ofb_mode (const struct key_type* kt)
615
-{
616
-  if (kt && kt->cipher) {
617
-      const unsigned int mode = cipher_kt_mode (kt->cipher);
618
-      return mode == OPENVPN_MODE_CFB || mode == OPENVPN_MODE_OFB;
619
-  }
620
-  return false;
611
+  if (cipher_kt_mode_ofb_cfb(kt->cipher) && !(packet_id && use_iv))
612
+    msg (M_FATAL, "--no-replay or --no-iv cannot be used with a CFB or OFB mode cipher");
621 613
 }
622 614
 
623 615
 /*
... ...
@@ -187,8 +187,6 @@ bool write_key (const struct key *key, const struct key_type *kt,
187 187
 
188 188
 int read_key (struct key *key, const struct key_type *kt, struct buffer *buf);
189 189
 
190
-bool cfb_ofb_mode (const struct key_type* kt);
191
-
192 190
 void init_key_type (struct key_type *kt, const char *ciphername,
193 191
     bool ciphername_defined, const char *authname, bool authname_defined,
194 192
     int keysize, bool cfb_ofb_allowed, bool warn);
... ...
@@ -230,6 +230,26 @@ int cipher_kt_block_size (const cipher_kt_t *cipher_kt);
230 230
  */
231 231
 int cipher_kt_mode (const cipher_kt_t *cipher_kt);
232 232
 
233
+/**
234
+ * Check of the supplied cipher is a supported CBC mode cipher.
235
+ *
236
+ * @param cipher	Static cipher parameters. May not be NULL.
237
+ *
238
+ * @return		true iff the cipher is a CBC mode cipher.
239
+ */
240
+bool cipher_kt_mode_cbc(const cipher_kt_t *cipher)
241
+  __attribute__((nonnull));
242
+
243
+/**
244
+ * Check of the supplied cipher is a supported OFB or CFB mode cipher.
245
+ *
246
+ * @param cipher	Static cipher parameters. May not be NULL.
247
+ *
248
+ * @return		true iff the cipher is a OFB or CFB mode cipher.
249
+ */
250
+bool cipher_kt_mode_ofb_cfb(const cipher_kt_t *cipher)
251
+  __attribute__((nonnull));
252
+
233 253
 
234 254
 /**
235 255
  *
... ...
@@ -288,6 +308,16 @@ int cipher_ctx_block_size (const cipher_ctx_t *ctx);
288 288
 int cipher_ctx_mode (const cipher_ctx_t *ctx);
289 289
 
290 290
 /**
291
+ * Returns the static cipher parameters for this context.
292
+ *
293
+ * @param ctx 		Cipher's context. May not be NULL.
294
+ *
295
+ * @return 		Static cipher parameters for the supplied context.
296
+ */
297
+const cipher_kt_t *cipher_ctx_get_cipher_kt (const cipher_ctx_t *ctx)
298
+  __attribute__((nonnull));
299
+
300
+/**
291 301
  * Resets the given cipher context, setting the IV to the specified value.
292 302
  * Preserves the associated key information.
293 303
  *
... ...
@@ -261,10 +261,9 @@ show_available_ciphers ()
261 261
       const EVP_CIPHER *cipher = EVP_get_cipherbynid (nid);
262 262
       if (cipher)
263 263
 	{
264
-	  const unsigned int mode = EVP_CIPHER_mode (cipher);
265
-	  if (mode == EVP_CIPH_CBC_MODE
264
+	  if (cipher_kt_mode_cbc(cipher)
266 265
 #ifdef ENABLE_OFB_CFB_MODE
267
-	      || mode == EVP_CIPH_CFB_MODE || mode == EVP_CIPH_OFB_MODE
266
+	      || cipher_kt_mode_ofb_cfb(cipher)
268 267
 #endif
269 268
 	      )
270 269
 	    printf ("%s %d bit default key (%s)\n",
... ...
@@ -483,6 +482,29 @@ cipher_kt_mode (const EVP_CIPHER *cipher_kt)
483 483
   return EVP_CIPHER_mode (cipher_kt);
484 484
 }
485 485
 
486
+bool
487
+cipher_kt_mode_cbc(const cipher_kt_t *cipher)
488
+{
489
+  return cipher_kt_mode(cipher) == OPENVPN_MODE_CBC
490
+#ifdef EVP_CIPH_FLAG_AEAD_CIPHER
491
+      /* Exclude AEAD cipher modes, they require a different API */
492
+      && !(EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_AEAD_CIPHER)
493
+#endif
494
+    ;
495
+}
496
+
497
+bool
498
+cipher_kt_mode_ofb_cfb(const cipher_kt_t *cipher)
499
+{
500
+  return (cipher_kt_mode(cipher) == OPENVPN_MODE_OFB ||
501
+	  cipher_kt_mode(cipher) == OPENVPN_MODE_CFB)
502
+#ifdef EVP_CIPH_FLAG_AEAD_CIPHER
503
+      /* Exclude AEAD cipher modes, they require a different API */
504
+      && !(EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_AEAD_CIPHER)
505
+#endif
506
+    ;
507
+}
508
+
486 509
 /*
487 510
  *
488 511
  * Generic cipher context functions
... ...
@@ -536,6 +558,13 @@ cipher_ctx_mode (const EVP_CIPHER_CTX *ctx)
536 536
   return EVP_CIPHER_CTX_mode (ctx);
537 537
 }
538 538
 
539
+const cipher_kt_t *
540
+cipher_ctx_get_cipher_kt (const cipher_ctx_t *ctx)
541
+{
542
+  return EVP_CIPHER_CTX_cipher(ctx);
543
+}
544
+
545
+
539 546
 int
540 547
 cipher_ctx_reset (EVP_CIPHER_CTX *ctx, uint8_t *iv_buf)
541 548
 {
... ...
@@ -416,6 +416,19 @@ cipher_kt_mode (const cipher_info_t *cipher_kt)
416 416
   return cipher_kt->mode;
417 417
 }
418 418
 
419
+bool
420
+cipher_kt_mode_cbc(const cipher_kt_t *cipher)
421
+{
422
+  return cipher_kt_mode(cipher) == OPENVPN_MODE_CBC;
423
+}
424
+
425
+bool
426
+cipher_kt_mode_ofb_cfb(const cipher_kt_t *cipher)
427
+{
428
+  return (cipher_kt_mode(cipher) == OPENVPN_MODE_OFB ||
429
+	  cipher_kt_mode(cipher) == OPENVPN_MODE_CFB);
430
+}
431
+
419 432
 
420 433
 /*
421 434
  *
... ...
@@ -464,6 +477,14 @@ int cipher_ctx_mode (const cipher_context_t *ctx)
464 464
   return cipher_kt_mode(ctx->cipher_info);
465 465
 }
466 466
 
467
+const cipher_kt_t *
468
+cipher_ctx_get_cipher_kt (const cipher_ctx_t *ctx)
469
+{
470
+  ASSERT(NULL != ctx);
471
+
472
+  return ctx->cipher_info;
473
+}
474
+
467 475
 int cipher_ctx_reset (cipher_context_t *ctx, uint8_t *iv_buf)
468 476
 {
469 477
   int retval = cipher_reset(ctx);
... ...
@@ -2186,7 +2186,7 @@ do_init_crypto_tls (struct context *c, const unsigned int flags)
2186 2186
 			       options->use_iv);
2187 2187
 
2188 2188
   /* In short form, unique datagram identifier is 32 bits, in long form 64 bits */
2189
-  packet_id_long_form = cfb_ofb_mode (&c->c1.ks.key_type);
2189
+  packet_id_long_form = cipher_kt_mode_ofb_cfb (c->c1.ks.key_type.cipher);
2190 2190
 
2191 2191
   /* Compute MTU parameters */
2192 2192
   crypto_adjust_frame_parameters (&c->c2.frame,