Merge branch 'development'

ChangeLog

@@ -3,6 +3,17 @@ mbed TLS ChangeLog (Sorted per branch, date)
 = mbed TLS 2.x.x branch released xxxx-xx-xx
 
 Bugfix
+   * Fix the redefinition of macro ssl_set_bio to an undefined symbol
+     mbedtls_ssl_set_bio_timeout in compat-1.3.h, by removing it.
+     Found by omlib-lin. #673
+   * Fix unused variable/function compilation warnings in pem.c, x509_crt.c and
+     x509_csr.c that are reported when building mbed TLS with a config.h that
+     does not define MBEDTLS_PEM_PARSE_C. Found by omnium21. #562
+   * Fix incorrect renegotiation condition in ssl_check_ctr_renegotiate() that
+     would compare 64 bits of the record counter instead of 48 bits as indicated
+     in RFC 6347 Section 4.3.1. This could cause the execution of the
+     renegotiation routines at unexpected times when the protocol is DTLS. Found
+     by wariua. #687
    * Fixed potential arithmetic overflow in mbedtls_ctr_drbg_reseed() that could
      cause buffer bound checks to be bypassed. Found by Eyal Itkin.
    * Fixed potential arithmetic overflows in mbedtls_cipher_update() that could

include/mbedtls/compat-1.3.h

@@ -2453,7 +2453,6 @@
 #define ssl_set_arc4_support mbedtls_ssl_conf_arc4_support
 #define ssl_set_authmode mbedtls_ssl_conf_authmode
 #define ssl_set_bio mbedtls_ssl_set_bio
-#define ssl_set_bio mbedtls_ssl_set_bio_timeout
 #define ssl_set_ca_chain mbedtls_ssl_conf_ca_chain
 #define ssl_set_cbc_record_splitting mbedtls_ssl_conf_cbc_record_splitting
 #define ssl_set_ciphersuites mbedtls_ssl_conf_ciphersuites

include/mbedtls/ssl.h

@@ -1146,7 +1146,7 @@ void mbedtls_ssl_conf_read_timeout( mbedtls_ssl_config *conf, uint32_t timeout )
  *
  * \note           See the documentation of \c mbedtls_ssl_set_timer_t and
  *                 \c mbedtls_ssl_get_timer_t for the conventions this pair of
- *                 callbacks must fallow.
+ *                 callbacks must follow.
  *
  * \note           On some platforms, timing.c provides
  *                 \c mbedtls_timing_set_delay() and
@@ -2183,7 +2183,7 @@ void mbedtls_ssl_conf_renegotiation_enforced( mbedtls_ssl_config *conf, int max_
 
 /**
  * \brief          Set record counter threshold for periodic renegotiation.
- *                 (Default: 2^64 - 256.)
+ *                 (Default: 2^48 - 1)
  *
  *                 Renegotiation is automatically triggered when a record
  *                 counter (outgoing or ingoing) crosses the defined
@@ -2194,9 +2194,17 @@ void mbedtls_ssl_conf_renegotiation_enforced( mbedtls_ssl_config *conf, int max_
  *                 Lower values can be used to enforce policies such as "keys
  *                 must be refreshed every N packets with cipher X".
  *
+ *                 The renegotiation period can be disabled by setting
+ *                 conf->disable_renegotiation to
+ *                 MBEDTLS_SSL_RENEGOTIATION_DISABLED.
+ *
+ * \note           When the configured transport is
+ *                 MBEDTLS_SSL_TRANSPORT_DATAGRAM the maximum renegotiation
+ *                 period is 2^48 - 1, and for MBEDTLS_SSL_TRANSPORT_STREAM,
+ *                 the maximum renegotiation period is 2^64 - 1.
+ *
  * \param conf     SSL configuration
  * \param period   The threshold value: a big-endian 64-bit number.
- *                 Set to 2^64 - 1 to disable periodic renegotiation
  */
 void mbedtls_ssl_conf_renegotiation_period( mbedtls_ssl_config *conf,
                                    const unsigned char period[8] );

library/pem.c

@@ -44,12 +44,12 @@
 #define mbedtls_free       free
 #endif
 
+#if defined(MBEDTLS_PEM_PARSE_C)
 /* Implementation that should never be optimized out by the compiler */
 static void mbedtls_zeroize( void *v, size_t n ) {
     volatile unsigned char *p = v; while( n-- ) *p++ = 0;
 }
 
-#if defined(MBEDTLS_PEM_PARSE_C)
 void mbedtls_pem_init( mbedtls_pem_context *ctx )
 {
     memset( ctx, 0, sizeof( mbedtls_pem_context ) );

library/ssl_tls.c

@@ -6482,6 +6482,10 @@ int mbedtls_ssl_renegotiate( mbedtls_ssl_context *ssl )
  */
 static int ssl_check_ctr_renegotiate( mbedtls_ssl_context *ssl )
 {
+    size_t ep_len = ssl_ep_len( ssl );
+    int in_ctr_cmp;
+    int out_ctr_cmp;
+
     if( ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER ||
         ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_PENDING ||
         ssl->conf->disable_renegotiation == MBEDTLS_SSL_RENEGOTIATION_DISABLED )
@@ -6489,8 +6493,12 @@ static int ssl_check_ctr_renegotiate( mbedtls_ssl_context *ssl )
         return( 0 );
     }
 
-    if( memcmp( ssl->in_ctr,  ssl->conf->renego_period, 8 ) <= 0 &&
+    in_ctr_cmp = memcmp( ssl->in_ctr + ep_len,
-        memcmp( ssl->out_ctr, ssl->conf->renego_period, 8 ) <= 0 )
+                        ssl->conf->renego_period + ep_len, 8 - ep_len );
+    out_ctr_cmp = memcmp( ssl->out_ctr + ep_len,
+                          ssl->conf->renego_period + ep_len, 8 - ep_len );
+
+    if( in_ctr_cmp <= 0 && out_ctr_cmp <= 0 )
     {
         return( 0 );
     }
@@ -7231,8 +7239,8 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf,
 
 #if defined(MBEDTLS_SSL_RENEGOTIATION)
     conf->renego_max_records = MBEDTLS_SSL_RENEGO_MAX_RECORDS_DEFAULT;
-    memset( conf->renego_period, 0xFF, 7 );
+    memset( conf->renego_period,     0x00, 2 );
-    conf->renego_period[7] = 0x00;
+    memset( conf->renego_period + 2, 0xFF, 6 );
 #endif
 
 #if defined(MBEDTLS_DHM_C) && defined(MBEDTLS_SSL_SRV_C)

library/x509_crt.c

@@ -969,8 +969,8 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, const unsigned char *bu
  */
 int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen )
 {
-    int success = 0, first_error = 0, total_failed = 0;
 #if defined(MBEDTLS_PEM_PARSE_C)
+    int success = 0, first_error = 0, total_failed = 0;
     int buf_format = MBEDTLS_X509_FORMAT_DER;
 #endif
 

library/x509_csr.c

@@ -265,8 +265,8 @@ int mbedtls_x509_csr_parse_der( mbedtls_x509_csr *csr,
  */
 int mbedtls_x509_csr_parse( mbedtls_x509_csr *csr, const unsigned char *buf, size_t buflen )
 {
-    int ret;
 #if defined(MBEDTLS_PEM_PARSE_C)
+    int ret;
     size_t use_len;
     mbedtls_pem_context pem;
 #endif

programs/ssl/ssl_server2.c

@@ -63,6 +63,8 @@ int main( void )
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <stdint.h>
+#include <inttypes.h>
 
 #if !defined(_WIN32)
 #include <signal.h>
@@ -113,7 +115,7 @@ int main( void )
 #define DFL_ALLOW_LEGACY        -2
 #define DFL_RENEGOTIATE         0
 #define DFL_RENEGO_DELAY        -2
-#define DFL_RENEGO_PERIOD       -1
+#define DFL_RENEGO_PERIOD       ( (uint64_t)-1 )
 #define DFL_EXCHANGES           1
 #define DFL_MIN_VERSION         -1
 #define DFL_MAX_VERSION         -1
@@ -292,7 +294,7 @@ int main( void )
     "    renegotiation=%%d    default: 0 (disabled)\n"      \
     "    renegotiate=%%d      default: 0 (disabled)\n"      \
     "    renego_delay=%%d     default: -2 (library default)\n" \
-    "    renego_period=%%d    default: (library default)\n"
+    "    renego_period=%%d    default: (2^64 - 1 for TLS, 2^48 - 1 for DTLS)\n"
 #else
 #define USAGE_RENEGO ""
 #endif
@@ -351,6 +353,19 @@ int main( void )
     "    force_ciphersuite=<name>    default: all enabled\n"            \
     " acceptable ciphersuite names:\n"
 
+
+#define PUT_UINT64_BE(out_be,in_le,i)                                   \
+{                                                                       \
+    (out_be)[(i) + 0] = (unsigned char)( ( (in_le) >> 56 ) & 0xFF );    \
+    (out_be)[(i) + 1] = (unsigned char)( ( (in_le) >> 48 ) & 0xFF );    \
+    (out_be)[(i) + 2] = (unsigned char)( ( (in_le) >> 40 ) & 0xFF );    \
+    (out_be)[(i) + 3] = (unsigned char)( ( (in_le) >> 32 ) & 0xFF );    \
+    (out_be)[(i) + 4] = (unsigned char)( ( (in_le) >> 24 ) & 0xFF );    \
+    (out_be)[(i) + 5] = (unsigned char)( ( (in_le) >> 16 ) & 0xFF );    \
+    (out_be)[(i) + 6] = (unsigned char)( ( (in_le) >> 8  ) & 0xFF );    \
+    (out_be)[(i) + 7] = (unsigned char)( ( (in_le) >> 0  ) & 0xFF );    \
+}
+
 /*
  * global options
  */
@@ -377,7 +392,7 @@ struct options
     int allow_legacy;           /* allow legacy renegotiation               */
     int renegotiate;            /* attempt renegotiation?                   */
     int renego_delay;           /* delay before enforcing renegotiation     */
-    int renego_period;          /* period for automatic renegotiation       */
+    uint64_t renego_period;     /* period for automatic renegotiation       */
     int exchanges;              /* number of data exchanges                 */
     int min_version;            /* minimum protocol version accepted        */
     int max_version;            /* maximum protocol version accepted        */
@@ -1041,8 +1056,8 @@ int main( int argc, char *argv[] )
         }
         else if( strcmp( p, "renego_period" ) == 0 )
         {
-            opt.renego_period = atoi( q );
+            if( sscanf( q, "%" SCNu64, &opt.renego_period ) != 1 ||
-            if( opt.renego_period < 2 || opt.renego_period > 255 )
+                opt.renego_period < 2 )
                 goto usage;
         }
         else if( strcmp( p, "exchanges" ) == 0 )
@@ -1757,7 +1772,7 @@ int main( int argc, char *argv[] )
 
     if( opt.renego_period != DFL_RENEGO_PERIOD )
     {
-        renego_period[7] = opt.renego_period;
+        PUT_UINT64_BE( renego_period, opt.renego_period, 0 );
         mbedtls_ssl_conf_renegotiation_period( &conf, renego_period );
     }
 #endif

scripts/data_files/rename-1.3-2.0.txt

@@ -1996,7 +1996,6 @@ ssl_set_alpn_protocols mbedtls_ssl_conf_alpn_protocols
 ssl_set_arc4_support mbedtls_ssl_conf_arc4_support
 ssl_set_authmode mbedtls_ssl_conf_authmode
 ssl_set_bio mbedtls_ssl_set_bio
-ssl_set_bio_timeout mbedtls_ssl_set_bio_timeout
 ssl_set_ca_chain mbedtls_ssl_conf_ca_chain
 ssl_set_cbc_record_splitting mbedtls_ssl_conf_cbc_record_splitting
 ssl_set_ciphersuites mbedtls_ssl_conf_ciphersuites

tests/ssl-opt.sh

@@ -1601,6 +1601,19 @@ run_test    "Renegotiation: DTLS, server-initiated" \
             -s "=> renegotiate" \
             -s "write hello request"
 
+run_test    "Renegotiation: DTLS, renego_period overflow" \
+            "$P_SRV debug_level=3 dtls=1 exchanges=4 renegotiation=1 renego_period=18446462598732840962 auth_mode=optional" \
+            "$P_CLI debug_level=3 dtls=1 exchanges=4 renegotiation=1" \
+            0 \
+            -c "client hello, adding renegotiation extension" \
+            -s "received TLS_EMPTY_RENEGOTIATION_INFO" \
+            -s "found renegotiation extension" \
+            -s "server hello, secure renegotiation extension" \
+            -s "record counter limit reached: renegotiate" \
+            -c "=> renegotiate" \
+            -s "=> renegotiate" \
+            -s "write hello request" \
+
 requires_gnutls
 run_test    "Renegotiation: DTLS, gnutls server, client-initiated" \
             "$G_SRV -u --mtu 4096" \