Re: [Openvpn-devel] [PATCH (master)] Discourage using 64-bit block ciphers

2016-08-16 Thread Arne Schwabe


Am 16.08.16 um 16:45 schrieb Steffan Karger:
> As discussed with the development team, we should start moving away from
> ciphers with a small block size.  For OpenVPN in particular this means
> moving away from 64-bit block ciphers, towards 128-bit block ciphers.
> This patch makes a start with that by moving ciphers with a block
> size < 128 bits to the bottom of the --show-ciphers output, and printing
> a warning in the connection phase if such a cipher is used.
>
> While touching this function, improve the output of --show-ciphers by
> ordering the output alphabetically, and changing the output format
> slightly.
>
ACK again from me. The patch is not C89 safe:

+  for (int nid = 0; nid < 1; ++nid)

Arne



--
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH (master)] Discourage using 64-bit block ciphers

2016-08-16 Thread Steffan Karger
As discussed with the development team, we should start moving away from
ciphers with a small block size.  For OpenVPN in particular this means
moving away from 64-bit block ciphers, towards 128-bit block ciphers.
This patch makes a start with that by moving ciphers with a block
size < 128 bits to the bottom of the --show-ciphers output, and printing
a warning in the connection phase if such a cipher is used.

While touching this function, improve the output of --show-ciphers by
ordering the output alphabetically, and changing the output format
slightly.

Signed-off-by: Steffan Karger 
---
 src/openvpn/crypto.c |  11 +++--
 src/openvpn/crypto_mbedtls.c |  44 -
 src/openvpn/crypto_openssl.c | 111 ++-
 tests/t_lpback.sh|   2 +-
 4 files changed, 131 insertions(+), 37 deletions(-)

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index d6d0251..6f57841 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -825,9 +825,14 @@ init_key_ctx (struct key_ctx *ctx, struct key *key,
   dmsg (D_SHOW_KEYS, "%s: CIPHER KEY: %s", prefix,
   format_hex (key->cipher, kt->cipher_length, 0, &gc));
   dmsg (D_CRYPTO_DEBUG, "%s: CIPHER block_size=%d iv_size=%d",
-  prefix,
-  cipher_kt_block_size(kt->cipher),
-  cipher_kt_iv_size(kt->cipher));
+  prefix, cipher_kt_block_size(kt->cipher),
+ cipher_kt_iv_size(kt->cipher));
+  if (cipher_kt_block_size(kt->cipher) < 128/8)
+   {
+ msg (M_WARN, "WARNING: this cipher's block size is less than 128 bit "
+ "(%d bit).  Consider using a --cipher with a larger block size.",
+ cipher_kt_block_size(kt->cipher)*8);
+   }
 }
   if (kt->digest && kt->hmac_length > 0)
 {
diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
index da1b417..92cba49 100644
--- a/src/openvpn/crypto_mbedtls.c
+++ b/src/openvpn/crypto_mbedtls.c
@@ -132,6 +132,25 @@ const cipher_name_pair cipher_name_translation_table[] = {
 const size_t cipher_name_translation_table_count =
 sizeof (cipher_name_translation_table) / sizeof 
(*cipher_name_translation_table);
 
+static void print_cipher(const cipher_kt_t *info)
+{
+  if (info && (cipher_kt_mode_cbc(info)
+#ifdef HAVE_AEAD_CIPHER_MODES
+  || cipher_kt_mode_aead(info)
+#endif
+  ))
+{
+  const char *ssl_only = cipher_kt_mode_cbc(info) ?
+ "" : ", TLS client/server mode only";
+  const char *var_key_size = info->flags & MBEDTLS_CIPHER_VARIABLE_KEY_LEN 
?
+ " by default" : "";
+
+  printf ("%s  (%d bit key%s, %d bit block%s)\n",
+ cipher_kt_name(info), cipher_kt_key_size(info) * 8, var_key_size,
+ cipher_kt_block_size(info) * 8, ssl_only);
+}
+}
+
 void
 show_available_ciphers ()
 {
@@ -147,20 +166,23 @@ show_available_ciphers ()
   while (*ciphers != 0)
 {
   const cipher_kt_t *info = mbedtls_cipher_info_from_type(*ciphers);
-
-  if (info && (cipher_kt_mode_cbc(info)
-#ifdef HAVE_AEAD_CIPHER_MODES
-  || cipher_kt_mode_aead(info)
-#endif
-  ))
+  if (info && cipher_kt_block_size(info) >= 128/8)
{
- const char *ssl_only = cipher_kt_mode_cbc(info) ?
- "" : " (TLS client/server mode)";
-
- printf ("%s %d bit default key%s\n",
- cipher_kt_name(info), cipher_kt_key_size(info) * 8, ssl_only);
+ print_cipher(info);
}
+  ciphers++;
+}
 
+  printf ("\nThe following ciphers have a block size of less than 128 bits, \n"
+ "and are therefore deprecated.  Do not use unless you have to.\n\n");
+  ciphers = mbedtls_cipher_list();
+  while (*ciphers != 0)
+{
+  const cipher_kt_t *info = mbedtls_cipher_info_from_type(*ciphers);
+  if (info && cipher_kt_block_size(info) < 128/8)
+   {
+ print_cipher(info);
+   }
   ciphers++;
 }
   printf ("\n");
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 376c8be..ef7aae0 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -249,11 +249,42 @@ const size_t cipher_name_translation_table_count =
 sizeof (cipher_name_translation_table) / sizeof 
(*cipher_name_translation_table);
 
 
+static int
+cipher_name_cmp(const void *a, const void *b)
+{
+  const EVP_CIPHER * const *cipher_a = a;
+  const EVP_CIPHER * const *cipher_b = b;
+
+  const char *cipher_name_a =
+  translate_cipher_name_to_openvpn(EVP_CIPHER_name(*cipher_a));
+  const char *cipher_name_b =
+  translate_cipher_name_to_openvpn(EVP_CIPHER_name(*cipher_b));
+
+  return strcmp(cipher_name_a, cipher_name_b);
+}
+
+static void
+print_cipher(const EVP_CIPHER *cipher)
+{
+  const char *var_key_size =
+   (EVP_CIPHER_flags (cipher) & EVP_CIPH_VARIABLE_LENGTH) ?
+" by default" : "";
+  const char *ssl_only = cipher_kt_mode_cbc(cipher) ?
+   "" : ", TLS client/server mode only";
+
+  printf ("%s