Merge remote-tracking branch 'restricted/pr/528' into development

* restricted/pr/528:
  Update query_config.c
  Fix failure in SSLv3 per-version suites test
  Adjust DES exclude lists in test scripts
  Clarify 3DES changes in ChangeLog
  Fix documentation for 3DES removal
  Exclude 3DES tests in test scripts
  Fix wording of ChangeLog and 3DES_REMOVE docs
  Reduce priority of 3DES ciphersuites

10 files changed, 94 insertions, 31 deletions

diff --git a/ChangeLog b/ChangeLog
index 1e9a46d..48a488b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,6 +9,9 @@ Features
      buffer, but at the benefit of reduced RAM consumption.
    * Add a new function mbedtls_asn1_write_named_bitstring() to write ASN.1
      named bitstring in DER as required by RFC 5280 Appendix B.
+   * Add MBEDTLS_REMOVE_3DES_CIPHERSUITES to allow removing 3DES ciphersuites
+     from the default list (enabled by default). See
+     https://sweet32.info/SWEET32_CCS16.pdf.
 
 API Changes
    * Add a new X.509 API call `mbedtls_x509_parse_der_nocopy()`.
@@ -63,6 +66,8 @@ Changes
    * Ensure that ssl-opt.h can be run in OS X. #2029
    * Re-enable certain interoperability tests in ssl-opt.sh which had previously
      been disabled for lack of a sufficiently recent version of GnuTLS on the CI.
+   * Ciphersuites based on 3DES now have the lowest priority by default when
+     they are enabled.
 
 = mbed TLS 2.16.0 branch released 2018-12-21
 
diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h
index e6abf24..c8a0d99 100644
--- a/include/mbedtls/config.h
+++ b/include/mbedtls/config.h
@@ -688,6 +688,26 @@
 #define MBEDTLS_REMOVE_ARC4_CIPHERSUITES
 
 /**
+ * \def MBEDTLS_REMOVE_3DES_CIPHERSUITES
+ *
+ * Remove 3DES ciphersuites by default in SSL / TLS.
+ * This flag removes the ciphersuites based on 3DES from the default list as
+ * returned by mbedtls_ssl_list_ciphersuites(). However, it is still possible
+ * to enable (some of) them with mbedtls_ssl_conf_ciphersuites() by including
+ * them explicitly.
+ *
+ * A man-in-the-browser attacker can recover authentication tokens sent through
+ * a TLS connection using a 3DES based cipher suite (see "On the Practical
+ * (In-)Security of 64-bit Block Ciphers" by Karthikeyan Bhargavan and Gaëtan
+ * Leurent, see https://sweet32.info/SWEET32_CCS16.pdf). If this attack falls
+ * in your threat model or you are unsure, then you should keep this option
+ * enabled to remove 3DES based cipher suites.
+ *
+ * Comment this macro to keep 3DES in the default ciphersuite list.
+ */
+#define MBEDTLS_REMOVE_3DES_CIPHERSUITES
+
+/**
  * \def MBEDTLS_ECP_DP_SECP192R1_ENABLED
  *
  * MBEDTLS_ECP_XXXX_ENABLED: Enables specific curves within the Elliptic Curve
diff --git a/library/ssl_ciphersuites.c b/library/ssl_ciphersuites.c
index 745474e..518f7dd 100644
--- a/library/ssl_ciphersuites.c
+++ b/library/ssl_ciphersuites.c
@@ -43,11 +43,11 @@
 /*
  * Ordered from most preferred to least preferred in terms of security.
  *
- * Current rule (except rc4, weak and null which come last):
+ * Current rule (except RC4 and 3DES, weak and null which come last):
  * 1. By key exchange:
  *    Forward-secure non-PSK > forward-secure PSK > ECJPAKE > other non-PSK > other PSK
  * 2. By key length and cipher:
- *    ChaCha > AES-256 > Camellia-256 > ARIA-256 > AES-128 > Camellia-128 > ARIA-128 > 3DES
+ *    ChaCha > AES-256 > Camellia-256 > ARIA-256 > AES-128 > Camellia-128 > ARIA-128
  * 3. By cipher mode when relevant GCM > CCM > CBC > CCM_8
  * 4. By hash function used when relevant
  * 5. By key exchange/auth again: EC > non-EC
@@ -126,11 +126,6 @@ static const int ciphersuite_preference[] =
     MBEDTLS_TLS_ECDHE_RSA_WITH_ARIA_128_CBC_SHA256,
     MBEDTLS_TLS_DHE_RSA_WITH_ARIA_128_CBC_SHA256,
 
-    /* All remaining >= 128-bit ephemeral suites */
-    MBEDTLS_TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA,
-    MBEDTLS_TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA,
-    MBEDTLS_TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA,
-
     /* The PSK ephemeral suites */
     MBEDTLS_TLS_ECDHE_PSK_WITH_CHACHA20_POLY1305_SHA256,
     MBEDTLS_TLS_DHE_PSK_WITH_CHACHA20_POLY1305_SHA256,
@@ -162,9 +157,6 @@ static const int ciphersuite_preference[] =
     MBEDTLS_TLS_ECDHE_PSK_WITH_ARIA_128_CBC_SHA256,
     MBEDTLS_TLS_DHE_PSK_WITH_ARIA_128_CBC_SHA256,
 
-    MBEDTLS_TLS_ECDHE_PSK_WITH_3DES_EDE_CBC_SHA,
-    MBEDTLS_TLS_DHE_PSK_WITH_3DES_EDE_CBC_SHA,
-
     /* The ECJPAKE suite */
     MBEDTLS_TLS_ECJPAKE_WITH_AES_128_CCM_8,
 
@@ -228,11 +220,6 @@ static const int ciphersuite_preference[] =
     MBEDTLS_TLS_ECDH_RSA_WITH_ARIA_128_CBC_SHA256,
     MBEDTLS_TLS_RSA_WITH_ARIA_128_CBC_SHA256,
 
-    /* All remaining >= 128-bit suites */
-    MBEDTLS_TLS_RSA_WITH_3DES_EDE_CBC_SHA,
-    MBEDTLS_TLS_ECDH_RSA_WITH_3DES_EDE_CBC_SHA,
-    MBEDTLS_TLS_ECDH_ECDSA_WITH_3DES_EDE_CBC_SHA,
-
     /* The RSA PSK suites */
     MBEDTLS_TLS_RSA_PSK_WITH_CHACHA20_POLY1305_SHA256,
     MBEDTLS_TLS_RSA_PSK_WITH_AES_256_GCM_SHA384,
@@ -251,8 +238,6 @@ static const int ciphersuite_preference[] =
     MBEDTLS_TLS_RSA_PSK_WITH_ARIA_128_GCM_SHA256,
     MBEDTLS_TLS_RSA_PSK_WITH_ARIA_128_CBC_SHA256,
 
-    MBEDTLS_TLS_RSA_PSK_WITH_3DES_EDE_CBC_SHA,
-
     /* The PSK suites */
     MBEDTLS_TLS_PSK_WITH_CHACHA20_POLY1305_SHA256,
     MBEDTLS_TLS_PSK_WITH_AES_256_GCM_SHA384,
@@ -275,6 +260,16 @@ static const int ciphersuite_preference[] =
     MBEDTLS_TLS_PSK_WITH_ARIA_128_GCM_SHA256,
     MBEDTLS_TLS_PSK_WITH_ARIA_128_CBC_SHA256,
 
+    /* 3DES suites */
+    MBEDTLS_TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA,
+    MBEDTLS_TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA,
+    MBEDTLS_TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA,
+    MBEDTLS_TLS_ECDHE_PSK_WITH_3DES_EDE_CBC_SHA,
+    MBEDTLS_TLS_DHE_PSK_WITH_3DES_EDE_CBC_SHA,
+    MBEDTLS_TLS_RSA_WITH_3DES_EDE_CBC_SHA,
+    MBEDTLS_TLS_ECDH_RSA_WITH_3DES_EDE_CBC_SHA,
+    MBEDTLS_TLS_ECDH_ECDSA_WITH_3DES_EDE_CBC_SHA,
+    MBEDTLS_TLS_RSA_PSK_WITH_3DES_EDE_CBC_SHA,
     MBEDTLS_TLS_PSK_WITH_3DES_EDE_CBC_SHA,
 
     /* RC4 suites */
@@ -2187,6 +2182,26 @@ const int *mbedtls_ssl_list_ciphersuites( void )
 static int supported_ciphersuites[MAX_CIPHERSUITES];
 static int supported_init = 0;
 
+static int ciphersuite_is_removed( const mbedtls_ssl_ciphersuite_t *cs_info )
+{
+    (void)cs_info;
+
+#if defined(MBEDTLS_REMOVE_ARC4_CIPHERSUITES)
+    if( cs_info->cipher == MBEDTLS_CIPHER_ARC4_128 )
+        return( 1 );
+#endif /* MBEDTLS_REMOVE_ARC4_CIPHERSUITES */
+
+#if defined(MBEDTLS_REMOVE_3DES_CIPHERSUITES)
+    if( cs_info->cipher == MBEDTLS_CIPHER_DES_EDE3_ECB ||
+        cs_info->cipher == MBEDTLS_CIPHER_DES_EDE3_CBC )
+    {
+        return( 1 );
+    }
+#endif /* MBEDTLS_REMOVE_3DES_CIPHERSUITES */
+
+    return( 0 );
+}
+
 const int *mbedtls_ssl_list_ciphersuites( void )
 {
     /*
@@ -2202,14 +2217,12 @@ const int *mbedtls_ssl_list_ciphersuites( void )
              *p != 0 && q < supported_ciphersuites + MAX_CIPHERSUITES - 1;
              p++ )
         {
-#if defined(MBEDTLS_REMOVE_ARC4_CIPHERSUITES)
             const mbedtls_ssl_ciphersuite_t *cs_info;
             if( ( cs_info = mbedtls_ssl_ciphersuite_from_id( *p ) ) != NULL &&
-                cs_info->cipher != MBEDTLS_CIPHER_ARC4_128 )
-#else
-            if( mbedtls_ssl_ciphersuite_from_id( *p ) != NULL )
-#endif
+                !ciphersuite_is_removed( cs_info ) )
+            {
                 *(q++) = *p;
+            }
         }
         *q = 0;
 
diff --git a/library/version_features.c b/library/version_features.c
index 61094d4..57be0f3 100644
--- a/library/version_features.c
+++ b/library/version_features.c
@@ -303,6 +303,9 @@ static const char *features[] = {
 #if defined(MBEDTLS_REMOVE_ARC4_CIPHERSUITES)
     "MBEDTLS_REMOVE_ARC4_CIPHERSUITES",
 #endif /* MBEDTLS_REMOVE_ARC4_CIPHERSUITES */
+#if defined(MBEDTLS_REMOVE_3DES_CIPHERSUITES)
+    "MBEDTLS_REMOVE_3DES_CIPHERSUITES",
+#endif /* MBEDTLS_REMOVE_3DES_CIPHERSUITES */
 #if defined(MBEDTLS_ECP_DP_SECP192R1_ENABLED)
     "MBEDTLS_ECP_DP_SECP192R1_ENABLED",
 #endif /* MBEDTLS_ECP_DP_SECP192R1_ENABLED */
diff --git a/programs/ssl/query_config.c b/programs/ssl/query_config.c
index 329a5df..1c3da54 100644
--- a/programs/ssl/query_config.c
+++ b/programs/ssl/query_config.c
@@ -850,6 +850,14 @@ int query_config( const char *config )
     }
 #endif /* MBEDTLS_REMOVE_ARC4_CIPHERSUITES */
 
+#if defined(MBEDTLS_REMOVE_3DES_CIPHERSUITES)
+    if( strcmp( "MBEDTLS_REMOVE_3DES_CIPHERSUITES", config ) == 0 )
+    {
+        MACRO_EXPANSION_TO_STR( MBEDTLS_REMOVE_3DES_CIPHERSUITES );
+        return( 0 );
+    }
+#endif /* MBEDTLS_REMOVE_3DES_CIPHERSUITES */
+
 #if defined(MBEDTLS_ECP_DP_SECP192R1_ENABLED)
     if( strcmp( "MBEDTLS_ECP_DP_SECP192R1_ENABLED", config ) == 0 )
     {
diff --git a/scripts/config.pl b/scripts/config.pl
index 18e1dc8..84069d2 100755
--- a/scripts/config.pl
+++ b/scripts/config.pl
@@ -30,6 +30,7 @@
 #   MBEDTLS_NO_PLATFORM_ENTROPY
 #   MBEDTLS_PSA_CRYPTO_C
 #   MBEDTLS_REMOVE_ARC4_CIPHERSUITES
+#   MBEDTLS_REMOVE_3DES_CIPHERSUITES
 #   MBEDTLS_SSL_HW_RECORD_ACCEL
 #   MBEDTLS_RSA_NO_CRT
 #   MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3
@@ -98,6 +99,7 @@ MBEDTLS_NO_PLATFORM_ENTROPY
 MBEDTLS_PSA_CRYPTO_C
 MBEDTLS_RSA_NO_CRT
 MBEDTLS_REMOVE_ARC4_CIPHERSUITES
+MBEDTLS_REMOVE_3DES_CIPHERSUITES
 MBEDTLS_SSL_HW_RECORD_ACCEL
 MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3
 MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION
diff --git a/tests/compat.sh b/tests/compat.sh
index 1814528..0eae1ea 100755
--- a/tests/compat.sh
+++ b/tests/compat.sh
@@ -62,7 +62,8 @@ FILTER=""
 #   avoid plain DES but keep 3DES-EDE-CBC (mbedTLS), DES-CBC3 (OpenSSL)
 # - ARIA: not in default config.h + requires OpenSSL >= 1.1.1
 # - ChachaPoly: requires OpenSSL >= 1.1.0
-EXCLUDE='NULL\|DES-CBC-\|RC4\|ARCFOUR\|ARIA\|CHACHA20-POLY1305'
+# - 3DES: not in default config
+EXCLUDE='NULL\|DES\|RC4\|ARCFOUR\|ARIA\|CHACHA20-POLY1305'
 VERBOSE=""
 MEMCHECK=0
 PEERS="OpenSSL$PEER_GNUTLS mbedTLS"
diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh
index 2688159..734af65 100755
--- a/tests/scripts/all.sh
+++ b/tests/scripts/all.sh
@@ -730,8 +730,8 @@ component_test_full_cmake_clang () {
     msg "test: ssl-opt.sh default, ECJPAKE, SSL async (full config)" # ~ 1s
     if_build_succeeded tests/ssl-opt.sh -f 'Default\|ECJPAKE\|SSL async private'
 
-    msg "test: compat.sh RC4, DES & NULL (full config)" # ~ 2 min
-    if_build_succeeded env OPENSSL_CMD="$OPENSSL_LEGACY" GNUTLS_CLI="$GNUTLS_LEGACY_CLI" GNUTLS_SERV="$GNUTLS_LEGACY_SERV" tests/compat.sh -e '3DES\|DES-CBC3' -f 'NULL\|DES\|RC4\|ARCFOUR'
+    msg "test: compat.sh RC4, DES, 3DES & NULL (full config)" # ~ 2 min
+    if_build_succeeded env OPENSSL_CMD="$OPENSSL_LEGACY" GNUTLS_CLI="$GNUTLS_LEGACY_CLI" GNUTLS_SERV="$GNUTLS_LEGACY_SERV" tests/compat.sh -e '^$' -f 'NULL\|DES\|RC4\|ARCFOUR'
 
     msg "test: compat.sh ARIA + ChachaPoly"
     if_build_succeeded env OPENSSL_CMD="$OPENSSL_NEXT" tests/compat.sh -e '^$' -f 'ARIA\|CHACHA'
diff --git a/tests/scripts/basic-build-test.sh b/tests/scripts/basic-build-test.sh
index 28fc687..ab95e22 100755
--- a/tests/scripts/basic-build-test.sh
+++ b/tests/scripts/basic-build-test.sh
@@ -91,7 +91,7 @@ OPENSSL_CMD="$OPENSSL_LEGACY"                               \
 OPENSSL_CMD="$OPENSSL_LEGACY"                                       \
     GNUTLS_CLI="$GNUTLS_LEGACY_CLI"                                 \
     GNUTLS_SERV="$GNUTLS_LEGACY_SERV"                               \
-    sh compat.sh -e '3DES\|DES-CBC3' -f 'NULL\|DES\|RC4\|ARCFOUR' | \
+    sh compat.sh -e '^$' -f 'NULL\|DES\|RC4\|ARCFOUR' |             \
     tee -a compat-test-$TEST_OUTPUT
 OPENSSL_CMD="$OPENSSL_NEXT"                     \
     sh compat.sh -e '^$' -f 'ARIA\|CHACHA' |    \
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index a8eb434..45b2c4e 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -4341,26 +4341,37 @@ run_test    "ECJPAKE: working, DTLS, nolog" \
 # Tests for ciphersuites per version
 
 requires_config_enabled MBEDTLS_SSL_PROTO_SSL3
+requires_config_enabled MBEDTLS_CAMELLIA_C
+requires_config_enabled MBEDTLS_AES_C
 run_test    "Per-version suites: SSL3" \
-            "$P_SRV min_version=ssl3 version_suites=TLS-RSA-WITH-3DES-EDE-CBC-SHA,TLS-RSA-WITH-AES-256-CBC-SHA,TLS-RSA-WITH-AES-128-CBC-SHA,TLS-RSA-WITH-AES-128-GCM-SHA256" \
+            "$P_SRV min_version=ssl3 version_suites=TLS-RSA-WITH-CAMELLIA-128-CBC-SHA,TLS-RSA-WITH-AES-256-CBC-SHA,TLS-RSA-WITH-AES-128-CBC-SHA,TLS-RSA-WITH-AES-128-GCM-SHA256" \
             "$P_CLI force_version=ssl3" \
             0 \
-            -c "Ciphersuite is TLS-RSA-WITH-3DES-EDE-CBC-SHA"
+            -c "Ciphersuite is TLS-RSA-WITH-CAMELLIA-128-CBC-SHA"
 
+requires_config_enabled MBEDTLS_SSL_PROTO_TLS1
+requires_config_enabled MBEDTLS_CAMELLIA_C
+requires_config_enabled MBEDTLS_AES_C
 run_test    "Per-version suites: TLS 1.0" \
-            "$P_SRV arc4=1 version_suites=TLS-RSA-WITH-3DES-EDE-CBC-SHA,TLS-RSA-WITH-AES-256-CBC-SHA,TLS-RSA-WITH-AES-128-CBC-SHA,TLS-RSA-WITH-AES-128-GCM-SHA256" \
+            "$P_SRV version_suites=TLS-RSA-WITH-CAMELLIA-128-CBC-SHA,TLS-RSA-WITH-AES-256-CBC-SHA,TLS-RSA-WITH-AES-128-CBC-SHA,TLS-RSA-WITH-AES-128-GCM-SHA256" \
             "$P_CLI force_version=tls1 arc4=1" \
             0 \
             -c "Ciphersuite is TLS-RSA-WITH-AES-256-CBC-SHA"
 
+requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1
+requires_config_enabled MBEDTLS_CAMELLIA_C
+requires_config_enabled MBEDTLS_AES_C
 run_test    "Per-version suites: TLS 1.1" \
-            "$P_SRV version_suites=TLS-RSA-WITH-3DES-EDE-CBC-SHA,TLS-RSA-WITH-AES-256-CBC-SHA,TLS-RSA-WITH-AES-128-CBC-SHA,TLS-RSA-WITH-AES-128-GCM-SHA256" \
+            "$P_SRV version_suites=TLS-RSA-WITH-CAMELLIA-128-CBC-SHA,TLS-RSA-WITH-AES-256-CBC-SHA,TLS-RSA-WITH-AES-128-CBC-SHA,TLS-RSA-WITH-AES-128-GCM-SHA256" \
             "$P_CLI force_version=tls1_1" \
             0 \
             -c "Ciphersuite is TLS-RSA-WITH-AES-128-CBC-SHA"
 
+requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2
+requires_config_enabled MBEDTLS_CAMELLIA_C
+requires_config_enabled MBEDTLS_AES_C
 run_test    "Per-version suites: TLS 1.2" \
-            "$P_SRV version_suites=TLS-RSA-WITH-3DES-EDE-CBC-SHA,TLS-RSA-WITH-AES-256-CBC-SHA,TLS-RSA-WITH-AES-128-CBC-SHA,TLS-RSA-WITH-AES-128-GCM-SHA256" \
+            "$P_SRV version_suites=TLS-RSA-WITH-CAMELLIA-128-CBC-SHA,TLS-RSA-WITH-AES-256-CBC-SHA,TLS-RSA-WITH-AES-128-CBC-SHA,TLS-RSA-WITH-AES-128-GCM-SHA256" \
             "$P_CLI force_version=tls1_2" \
             0 \
             -c "Ciphersuite is TLS-RSA-WITH-AES-128-GCM-SHA256"