Merge pull request #10 from ARMmbed/development

Merge pull request ARMmbed#3160 from gilles-peskine-arm/hkdf_expand-i…

.github/issue_template.md

@@ -1,7 +1,7 @@
 Note: This is just a template, so feel free to use/remove the unnecessary things
 
 ### Description
-- Type: Bug | Enhancement\Feature Request | Question
+- Type: Bug | Enhancement\Feature Request
 - Priority: Blocker | Major | Minor
 
 ---------------------------------------------------------------
@@ -38,4 +38,4 @@ Version:
 
 ## Question
 
-**Please first check for answers in the [Mbed TLS knowledge Base](https://tls.mbed.org/kb), and preferably file an issue in the [Mbed TLS support forum](https://forums.mbed.com/c/mbed-tls)**  
+**Please first check for answers in the [Mbed TLS knowledge Base](https://tls.mbed.org/kb). If you can't find the answer you're looking for then please use the [Mbed TLS mailing list](https://lists.trustedfirmware.org/mailman/listinfo/mbed-tls)**

ChangeLog

@@ -6,9 +6,19 @@ New deprecations
    * Deprecate MBEDTLS_SSL_HW_RECORD_ACCEL that enables function hooks in the
      SSL module for hardware acceleration of individual records.
 
+Security
+   * Fix issue in DTLS handling of new associations with the same parameters
+     (RFC 6347 section 4.2.8): an attacker able to send forged UDP packets to
+     the server could cause it to drop established associations with
+     legitimate clients, resulting in a Denial of Service. This could only
+     happen when MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE was enabled in config.h
+     (which it is by default).
+
 Bugfix
    * Fix compilation failure when both MBEDTLS_SSL_PROTO_DTLS and
      MBEDTLS_SSL_HW_RECORD_ACCEL are enabled.
+   * Remove a spurious check in ssl_parse_client_psk_identity that triggered
+     a warning with some compilers. Fix contributed by irwir in #2856.
 
 Changes
    * Mbed Crypto is no longer a Git submodule. The crypto part of the library

ChangeLog.d/00README.md

@@ -0,0 +1,67 @@
+# Pending changelog entry directory
+
+This directory contains changelog entries that have not yet been merged
+to the changelog file ([`../ChangeLog`](../ChangeLog)).
+
+## Changelog entry file format
+
+A changelog entry file must have the extension `*.txt` and must have the
+following format:
+
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Security
+   * Change description.
+   * Another change description.
+
+Features
+   * Yet another change description. This is a long change description that
+     spans multiple lines.
+   * Yet again another change description.
+
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The permitted changelog entry categories are as follows:
+<!-- Keep this synchronized with STANDARD_CATEGORIES in assemble_changelog.py! -->
+
+    API changes
+    Default behavior changes
+    Requirement changes
+    New deprecations
+    Removals
+    Features
+    Security
+    Bugfix
+    Changes
+
+Use “Changes” for anything that doesn't fit in the other categories, such as
+performance, documentation and test improvements.
+
+## How to write a changelog entry
+
+Each entry starts with three spaces, an asterisk and a space. Continuation
+lines start with 5 spaces. Lines wrap at 79 characters.
+
+Write full English sentences with proper capitalization and punctuation. Use
+the present tense. Use the imperative where applicable. For example: “Fix a
+bug in mbedtls_xxx() ….”
+
+Include GitHub issue numbers where relevant. Use the format “#1234” for an
+Mbed TLS issue. Add other external references such as CVE numbers where
+applicable.
+
+Credit the author of the contribution if the contribution is not a member of
+the Mbed TLS development team. Also credit bug reporters where applicable.
+
+**Explain why, not how**. Remember that the audience is the users of the
+library, not its developers. In particular, for a bug fix, explain the
+consequences of the bug, not how the bug was fixed. For a new feature, explain
+why one might be interested in the feature. For an API change or a deprecation,
+explain how to update existing applications.
+
+See [existing entries](../ChangeLog) for examples.
+
+## How `ChangeLog` is updated
+
+Run [`../scripts/assemble_changelog.py`](../scripts/assemble_changelog.py)
+from a Git working copy
+to move the entries from files in `ChangeLog.d` to the main `ChangeLog` file.

include/mbedtls/check_config.h

@@ -619,6 +619,23 @@
 #error "MBEDTLS_SSL_PROTO_TLS1_2 defined, but not all prerequisites"
 #endif
 
+#if (defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) ||  \
+     defined(MBEDTLS_SSL_PROTO_TLS1_1) || defined(MBEDTLS_SSL_PROTO_TLS1_2)) && \
+    !(defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) ||                          \
+      defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) ||                      \
+      defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) ||                    \
+      defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) ||                  \
+      defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) ||                     \
+      defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) ||                   \
+      defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) ||                          \
+      defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) ||                      \
+      defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) ||                      \
+      defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) ||                    \
+      defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) )
+#error "One or more versions of the TLS protocol are enabled " \
+        "but no key exchange methods defined with MBEDTLS_KEY_EXCHANGE_xxxx"
+#endif
+
 #if defined(MBEDTLS_SSL_PROTO_DTLS)     && \
     !defined(MBEDTLS_SSL_PROTO_TLS1_1)  && \
     !defined(MBEDTLS_SSL_PROTO_TLS1_2)
@@ -763,6 +780,10 @@
 #error "MBEDTLS_X509_CREATE_C defined, but not all prerequisites"
 #endif
 
+#if defined(MBEDTLS_CERTS_C) && !defined(MBEDTLS_X509_USE_C)
+#error "MBEDTLS_CERTS_C defined, but not all prerequisites"
+#endif
+
 #if defined(MBEDTLS_X509_CRT_PARSE_C) && ( !defined(MBEDTLS_X509_USE_C) )
 #error "MBEDTLS_X509_CRT_PARSE_C defined, but not all prerequisites"
 #endif

include/mbedtls/config.h

@@ -1520,8 +1520,8 @@
 
 /** \def MBEDTLS_SSL_EXTENDED_MASTER_SECRET
  *
- * Enable support for Extended Master Secret, aka Session Hash
- * (draft-ietf-tls-session-hash-02).
+ * Enable support for RFC 7627: Session Hash and Extended Master Secret
+ * Extension.
  *
  * This was introduced as "the proper fix" to the Triple Handshake familiy of
  * attacks, but it is recommended to always use it (even if you disable
@@ -1539,7 +1539,8 @@
 /**
  * \def MBEDTLS_SSL_FALLBACK_SCSV
  *
- * Enable support for FALLBACK_SCSV (draft-ietf-tls-downgrade-scsv-00).
+ * Enable support for RFC 7507: Fallback Signaling Cipher Suite Value (SCSV)
+ * for Preventing Protocol Downgrade Attacks.
  *
  * For servers, it is recommended to always enable this, unless you support
  * only one version of TLS, or know for sure that none of your clients

library/hkdf.c

@@ -115,7 +115,7 @@ int mbedtls_hkdf_expand( const mbedtls_md_info_t *md, const unsigned char *prk,
 
     n = okm_len / hash_len;
 
-    if( (okm_len % hash_len) != 0 )
+    if( okm_len % hash_len != 0 )
     {
         n++;
     }
@@ -131,11 +131,13 @@ int mbedtls_hkdf_expand( const mbedtls_md_info_t *md, const unsigned char *prk,
 
     mbedtls_md_init( &ctx );
 
-    if( (ret = mbedtls_md_setup( &ctx, md, 1) ) != 0 )
+    if( ( ret = mbedtls_md_setup( &ctx, md, 1 ) ) != 0 )
     {
         goto exit;
     }
 
+    memset( t, 0, hash_len );
+
     /*
      * Compute T = T(1) | T(2) | T(3) | ... | T(N)
      * Where T(N) is defined in RFC 5869 Section 2.3

library/ssl_cli.c

@@ -2344,7 +2344,7 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl,
                                       unsigned char *end )
 {
     int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE;
-    size_t  len;
+    uint16_t  len;
     ((void) ssl);
 
     /*
@@ -2361,7 +2361,7 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl,
     len = (*p)[0] << 8 | (*p)[1];
     *p += 2;
 
-    if( end - (*p) < (int) len )
+    if( end - (*p) < len )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message "
                                     "(psk_identity_hint length)" ) );

library/ssl_msg.c

@@ -3197,16 +3197,17 @@ static int ssl_check_dtls_clihlo_cookie(
  * that looks like a ClientHello.
  *
  * - if the input looks like a ClientHello without cookies,
- *   send back HelloVerifyRequest, then
- *   return MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED
+ *   send back HelloVerifyRequest, then return 0
  * - if the input looks like a ClientHello with a valid cookie,
  *   reset the session of the current context, and
  *   return MBEDTLS_ERR_SSL_CLIENT_RECONNECT
  * - if anything goes wrong, return a specific error code
  *
- * mbedtls_ssl_read_record() will ignore the record if anything else than
- * MBEDTLS_ERR_SSL_CLIENT_RECONNECT or 0 is returned, although this function
- * cannot not return 0.
+ * This function is called (through ssl_check_client_reconnect()) when an
+ * unexpected record is found in ssl_get_next_record(), which will discard the
+ * record if we return 0, and bubble up the return value otherwise (this
+ * includes the case of MBEDTLS_ERR_SSL_CLIENT_RECONNECT and of unexpected
+ * errors, and is the right thing to do in both cases).
  */
 static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl )
 {
@@ -3218,6 +3219,8 @@ static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl )
     {
         /* If we can't use cookies to verify reachability of the peer,
          * drop the record. */
+        MBEDTLS_SSL_DEBUG_MSG( 1, ( "no cookie callbacks, "
+                                    "can't check reconnect validity" ) );
         return( 0 );
     }
 
@@ -3233,16 +3236,23 @@ static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl )
 
     if( ret == MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED )
     {
+        int send_ret;
+        MBEDTLS_SSL_DEBUG_MSG( 1, ( "sending HelloVerifyRequest" ) );
+        MBEDTLS_SSL_DEBUG_BUF( 4, "output record sent to network",
+                                  ssl->out_buf, len );
         /* Don't check write errors as we can't do anything here.
          * If the error is permanent we'll catch it later,
          * if it's not, then hopefully it'll work next time. */
-        (void) ssl->f_send( ssl->p_bio, ssl->out_buf, len );
-        ret = 0;
+        send_ret = ssl->f_send( ssl->p_bio, ssl->out_buf, len );
+        MBEDTLS_SSL_DEBUG_RET( 2, "ssl->f_send", send_ret );
+        (void) send_ret;
+
+        return( 0 );
     }
 
     if( ret == 0 )
     {
-        /* Got a valid cookie, partially reset context */
+        MBEDTLS_SSL_DEBUG_MSG( 1, ( "cookie is valid, resetting context" ) );
         if( ( ret = mbedtls_ssl_session_reset_int( ssl, 1 ) ) != 0 )
         {
             MBEDTLS_SSL_DEBUG_RET( 1, "reset", ret );
@@ -4415,6 +4425,7 @@ static int ssl_get_next_record( mbedtls_ssl_context *ssl )
                 ssl->in_msglen = rec.data_len;
 
                 ret = ssl_check_client_reconnect( ssl );
+                MBEDTLS_SSL_DEBUG_RET( 2, "ssl_check_client_reconnect", ret );
                 if( ret != 0 )
                     return( ret );
 #endif

library/ssl_srv.c

@@ -3812,7 +3812,7 @@ static int ssl_parse_client_psk_identity( mbedtls_ssl_context *ssl, unsigned cha
                                           const unsigned char *end )
 {
     int ret = 0;
-    size_t n;
+    uint16_t n;
 
     if( ssl_conf_has_psk_or_cb( ssl->conf ) == 0 )
     {
@@ -3832,7 +3832,7 @@ static int ssl_parse_client_psk_identity( mbedtls_ssl_context *ssl, unsigned cha
     n = ( (*p)[0] << 8 ) | (*p)[1];
     *p += 2;
 
-    if( n < 1 || n > 65535 || n > (size_t) ( end - *p ) )
+    if( n == 0 || n > end - *p )
     {
         MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client key exchange message" ) );
         return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE );

library/x509.c

@@ -1064,7 +1064,7 @@ int mbedtls_x509_self_test( int verbose )
     mbedtls_x509_crt_free( &clicert );
 #else
     ((void) verbose);
-#endif /* MBEDTLS_CERTS_C && MBEDTLS_SHA1_C */
+#endif /* MBEDTLS_CERTS_C && MBEDTLS_SHA256_C */
     return( ret );
 }