Merge 'iotssl-558-2.1-md5-tls-sigs-restricted'
diff --git a/ChangeLog b/ChangeLog
index b239310..459dabc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,9 +3,24 @@
= mbed TLS 2.1.4 released 2015-12-xx
Security
+
+ * Fix potential double free when mbedtls_asn1_store_named_data() fails to
+ allocate memory. Only used for certificate generation, not triggerable
+ remotely in SSL/TLS. Found by Rafał Przywara. #367
* Disable MD5 handshake signatures in TLS 1.2 by default
(Reported by Karthikeyan Bhargavan and Gaëtan Leurent.)
+Bugfix
+ * Fix over-restrictive length limit in GCM. Found by Andreas-N. #362
+
+Changes
+ * To avoid dropping an entire DTLS datagram if a single record in a datagram
+ is invalid, we now only drop the record and look at subsequent records (if
+ any are present) in the same datagram to avoid interoperability issues.
+ Previously the library was dropping the entire datagram, Where a record is
+ unexpected, the function mbedtls_ssl_read_record() will now return
+ MBEDTLS_ERR_SSL_UNEXPECTED_RECORD.
+
= mbed TLS 2.1.3 released 2015-11-04
Security
diff --git a/include/mbedtls/error.h b/include/mbedtls/error.h
index a182713..5e549f6 100644
--- a/include/mbedtls/error.h
+++ b/include/mbedtls/error.h
@@ -79,7 +79,7 @@
* ECP 4 8 (Started from top)
* MD 5 4
* CIPHER 6 6
- * SSL 6 16 (Started from top)
+ * SSL 6 17 (Started from top)
* SSL 7 31
*
* Module dependent error code (5 bits 0x.00.-0x.F8.)
diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h
index 9cb714a..1e6915a 100644
--- a/include/mbedtls/ssl.h
+++ b/include/mbedtls/ssl.h
@@ -106,6 +106,7 @@
#define MBEDTLS_ERR_SSL_WANT_WRITE -0x6880 /**< Connection requires a write call. */
#define MBEDTLS_ERR_SSL_TIMEOUT -0x6800 /**< The operation timed out. */
#define MBEDTLS_ERR_SSL_CLIENT_RECONNECT -0x6780 /**< The client initiated a reconnect from the same port. */
+#define MBEDTLS_ERR_SSL_UNEXPECTED_RECORD -0x6700 /**< Record header looks valid but is not expected. */
/*
* Various constants
@@ -2087,7 +2088,8 @@
* \note If this function returns something other than 0 or
* MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context
* becomes unusable, and you should either free it or call
- * \c mbedtls_ssl_session_reset() on it before re-using it.
+ * \c mbedtls_ssl_session_reset() on it before re-using it for
+ * a new connection; the current connection must be closed.
*
* \note If DTLS is in use, then you may choose to handle
* MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED specially for logging
@@ -2103,6 +2105,12 @@
* the following state after execution of this function.
* Do not call this function if state is MBEDTLS_SSL_HANDSHAKE_OVER.
*
+ * \note If this function returns something other than 0 or
+ * MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context
+ * becomes unusable, and you should either free it or call
+ * \c mbedtls_ssl_session_reset() on it before re-using it for
+ * a new connection; the current connection must be closed.
+ *
* \param ssl SSL context
*
* \return 0 if successful, or
@@ -2121,6 +2129,12 @@
* \param ssl SSL context
*
* \return 0 if successful, or any mbedtls_ssl_handshake() return value.
+ *
+ * \note If this function returns something other than 0 or
+ * MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context
+ * becomes unusable, and you should either free it or call
+ * \c mbedtls_ssl_session_reset() on it before re-using it for
+ * a new connection; the current connection must be closed.
*/
int mbedtls_ssl_renegotiate( mbedtls_ssl_context *ssl );
#endif /* MBEDTLS_SSL_RENEGOTIATION */
@@ -2138,6 +2152,13 @@
* MBEDTLS_ERR_SSL_CLIENT_RECONNECT (see below), or
* another negative error code.
*
+ * \note If this function returns something other than a positive
+ * value or MBEDTLS_ERR_SSL_WANT_READ/WRITE or
+ * MBEDTLS_ERR_SSL_CLIENT_RECONNECT, then the ssl context
+ * becomes unusable, and you should either free it or call
+ * \c mbedtls_ssl_session_reset() on it before re-using it for
+ * a new connection; the current connection must be closed.
+ *
* \note When this function return MBEDTLS_ERR_SSL_CLIENT_RECONNECT
* (which can only happen server-side), it means that a client
* is initiating a new connection using the same source port.
@@ -2171,6 +2192,12 @@
* or MBEDTLS_ERR_SSL_WANT_WRITE of MBEDTLS_ERR_SSL_WANT_READ,
* or another negative error code.
*
+ * \note If this function returns something other than a positive
+ * value or MBEDTLS_ERR_SSL_WANT_READ/WRITE, the ssl context
+ * becomes unusable, and you should either free it or call
+ * \c mbedtls_ssl_session_reset() on it before re-using it for
+ * a new connection; the current connection must be closed.
+ *
* \note When this function returns MBEDTLS_ERR_SSL_WANT_WRITE/READ,
* it must be called later with the *same* arguments,
* until it returns a positive value.
@@ -2194,6 +2221,12 @@
* \param message The alert message (SSL_ALERT_MSG_*)
*
* \return 0 if successful, or a specific SSL error code.
+ *
+ * \note If this function returns something other than 0 or
+ * MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context
+ * becomes unusable, and you should either free it or call
+ * \c mbedtls_ssl_session_reset() on it before re-using it for
+ * a new connection; the current connection must be closed.
*/
int mbedtls_ssl_send_alert_message( mbedtls_ssl_context *ssl,
unsigned char level,
@@ -2202,6 +2235,14 @@
* \brief Notify the peer that the connection is being closed
*
* \param ssl SSL context
+ *
+ * \return 0 if successful, or a specific SSL error code.
+ *
+ * \note If this function returns something other than 0 or
+ * MBEDTLS_ERR_SSL_WANT_READ/WRITE, then the ssl context
+ * becomes unusable, and you should either free it or call
+ * \c mbedtls_ssl_session_reset() on it before re-using it for
+ * a new connection; the current connection must be closed.
*/
int mbedtls_ssl_close_notify( mbedtls_ssl_context *ssl );
diff --git a/library/asn1write.c b/library/asn1write.c
index 456660d..00ed73c 100644
--- a/library/asn1write.c
+++ b/library/asn1write.c
@@ -339,19 +339,18 @@
}
else if( cur->val.len < val_len )
{
- // Enlarge existing value buffer if needed
- //
- mbedtls_free( cur->val.p );
- cur->val.p = NULL;
-
- cur->val.len = val_len;
- cur->val.p = mbedtls_calloc( 1, val_len );
- if( cur->val.p == NULL )
- {
- mbedtls_free( cur->oid.p );
- mbedtls_free( cur );
+ /*
+ * Enlarge existing value buffer if needed
+ * Preserve old data until the allocation succeeded, to leave list in
+ * a consistent state in case allocation fails.
+ */
+ void *p = mbedtls_calloc( 1, val_len );
+ if( p == NULL )
return( NULL );
- }
+
+ mbedtls_free( cur->val.p );
+ cur->val.p = p;
+ cur->val.len = val_len;
}
if( val != NULL )
diff --git a/library/error.c b/library/error.c
index a1cf83a..debda1d 100644
--- a/library/error.c
+++ b/library/error.c
@@ -430,6 +430,8 @@
mbedtls_snprintf( buf, buflen, "SSL - The operation timed out" );
if( use_ret == -(MBEDTLS_ERR_SSL_CLIENT_RECONNECT) )
mbedtls_snprintf( buf, buflen, "SSL - The client initiated a reconnect from the same port" );
+ if( use_ret == -(MBEDTLS_ERR_SSL_UNEXPECTED_RECORD) )
+ mbedtls_snprintf( buf, buflen, "SSL - Record header looks valid but is not expected" );
#endif /* MBEDTLS_SSL_TLS_C */
#if defined(MBEDTLS_X509_USE_C) || defined(MBEDTLS_X509_CREATE_C)
diff --git a/library/gcm.c b/library/gcm.c
index 4298254..aaacf97 100644
--- a/library/gcm.c
+++ b/library/gcm.c
@@ -362,7 +362,7 @@
/* Total length is restricted to 2^39 - 256 bits, ie 2^36 - 2^5 bytes
* Also check for possible overflow */
if( ctx->len + length < ctx->len ||
- (uint64_t) ctx->len + length > 0x03FFFFE0ull )
+ (uint64_t) ctx->len + length > 0xFFFFFFFE0ull )
{
return( MBEDTLS_ERR_GCM_BAD_INPUT );
}
diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 1bda53c..938ca7a 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -2474,7 +2474,9 @@
{
dn_size = crt->subject_raw.len;
- if( end < p || (size_t)( end - p ) < 2 + dn_size )
+ if( end < p ||
+ (size_t)( end - p ) < dn_size ||
+ (size_t)( end - p ) < 2 + dn_size )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "skipping CAs: buffer too short" ) );
break;
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 52a7ebf..65b8c88 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -3455,6 +3455,18 @@
* uint16 epoch; // DTLS only
* uint48 sequence_number; // DTLS only
* uint16 length;
+ *
+ * Return 0 if header looks sane (and, for DTLS, the record is expected)
+ * MBEDTLS_ERR_SSL_INVALID_RECORD if the header looks bad,
+ * MBEDTLS_ERR_SSL_UNEXPECTED_RECORD (DTLS only) if sane but unexpected.
+ *
+ * With DTLS, mbedtls_ssl_read_record() will:
+ * 1. proceed with the record if this function returns 0
+ * 2. drop only the current record if this function returns UNEXPECTED_RECORD
+ * 3. return CLIENT_RECONNECT if this function returns that value
+ * 4. drop the whole datagram if this function returns anything else.
+ * Point 2 is needed when the peer is resending, and we have already received
+ * the first record from a datagram but are still waiting for the others.
*/
static int ssl_parse_record_header( mbedtls_ssl_context *ssl )
{
@@ -3490,34 +3502,6 @@
return( MBEDTLS_ERR_SSL_INVALID_RECORD );
}
-#if defined(MBEDTLS_SSL_PROTO_DTLS)
- if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM )
- {
- /* Drop unexpected ChangeCipherSpec messages */
- if( ssl->in_msgtype == MBEDTLS_SSL_MSG_CHANGE_CIPHER_SPEC &&
- ssl->state != MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC &&
- ssl->state != MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC )
- {
- MBEDTLS_SSL_DEBUG_MSG( 1, ( "dropping unexpected ChangeCipherSpec" ) );
- return( MBEDTLS_ERR_SSL_INVALID_RECORD );
- }
-
- /* Drop unexpected ApplicationData records,
- * except at the beginning of renegotiations */
- if( ssl->in_msgtype == MBEDTLS_SSL_MSG_APPLICATION_DATA &&
- ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER
-#if defined(MBEDTLS_SSL_RENEGOTIATION)
- && ! ( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS &&
- ssl->state == MBEDTLS_SSL_SERVER_HELLO )
-#endif
- )
- {
- MBEDTLS_SSL_DEBUG_MSG( 1, ( "dropping unexpected ApplicationData" ) );
- return( MBEDTLS_ERR_SSL_INVALID_RECORD );
- }
- }
-#endif
-
/* Check version */
if( major_ver != ssl->major_ver )
{
@@ -3531,53 +3515,6 @@
return( MBEDTLS_ERR_SSL_INVALID_RECORD );
}
- /* Check epoch (and sequence number) with DTLS */
-#if defined(MBEDTLS_SSL_PROTO_DTLS)
- if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM )
- {
- unsigned int rec_epoch = ( ssl->in_ctr[0] << 8 ) | ssl->in_ctr[1];
-
- if( rec_epoch != ssl->in_epoch )
- {
- MBEDTLS_SSL_DEBUG_MSG( 1, ( "record from another epoch: "
- "expected %d, received %d",
- ssl->in_epoch, rec_epoch ) );
-
-#if defined(MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE) && defined(MBEDTLS_SSL_SRV_C)
- /*
- * Check for an epoch 0 ClientHello. We can't use in_msg here to
- * access the first byte of record content (handshake type), as we
- * have an active transform (possibly iv_len != 0), so use the
- * fact that the record header len is 13 instead.
- */
- if( ssl->conf->endpoint == MBEDTLS_SSL_IS_SERVER &&
- ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER &&
- rec_epoch == 0 &&
- ssl->in_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE &&
- ssl->in_left > 13 &&
- ssl->in_buf[13] == MBEDTLS_SSL_HS_CLIENT_HELLO )
- {
- MBEDTLS_SSL_DEBUG_MSG( 1, ( "possible client reconnect "
- "from the same port" ) );
- return( ssl_handle_possible_reconnect( ssl ) );
- }
- else
-#endif /* MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE && MBEDTLS_SSL_SRV_C */
- return( MBEDTLS_ERR_SSL_INVALID_RECORD );
- }
-
-#if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY)
- /* Replay detection only works for the current epoch */
- if( rec_epoch == ssl->in_epoch &&
- mbedtls_ssl_dtls_replay_check( ssl ) != 0 )
- {
- MBEDTLS_SSL_DEBUG_MSG( 1, ( "replayed record" ) );
- return( MBEDTLS_ERR_SSL_INVALID_RECORD );
- }
-#endif
- }
-#endif /* MBEDTLS_SSL_PROTO_DTLS */
-
/* Check length against the size of our buffer */
if( ssl->in_msglen > MBEDTLS_SSL_BUFFER_LEN
- (size_t)( ssl->in_msg - ssl->in_buf ) )
@@ -3627,6 +3564,82 @@
#endif
}
+ /*
+ * DTLS-related tests done last, because most of them may result in
+ * silently dropping the record (but not the whole datagram), and we only
+ * want to consider that after ensuring that the "basic" fields (type,
+ * version, length) are sane.
+ */
+#if defined(MBEDTLS_SSL_PROTO_DTLS)
+ if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM )
+ {
+ unsigned int rec_epoch = ( ssl->in_ctr[0] << 8 ) | ssl->in_ctr[1];
+
+ /* Drop unexpected ChangeCipherSpec messages */
+ if( ssl->in_msgtype == MBEDTLS_SSL_MSG_CHANGE_CIPHER_SPEC &&
+ ssl->state != MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC &&
+ ssl->state != MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "dropping unexpected ChangeCipherSpec" ) );
+ return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD );
+ }
+
+ /* Drop unexpected ApplicationData records,
+ * except at the beginning of renegotiations */
+ if( ssl->in_msgtype == MBEDTLS_SSL_MSG_APPLICATION_DATA &&
+ ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER
+#if defined(MBEDTLS_SSL_RENEGOTIATION)
+ && ! ( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS &&
+ ssl->state == MBEDTLS_SSL_SERVER_HELLO )
+#endif
+ )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "dropping unexpected ApplicationData" ) );
+ return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD );
+ }
+
+ /* Check epoch (and sequence number) with DTLS */
+ if( rec_epoch != ssl->in_epoch )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "record from another epoch: "
+ "expected %d, received %d",
+ ssl->in_epoch, rec_epoch ) );
+
+#if defined(MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE) && defined(MBEDTLS_SSL_SRV_C)
+ /*
+ * Check for an epoch 0 ClientHello. We can't use in_msg here to
+ * access the first byte of record content (handshake type), as we
+ * have an active transform (possibly iv_len != 0), so use the
+ * fact that the record header len is 13 instead.
+ */
+ if( ssl->conf->endpoint == MBEDTLS_SSL_IS_SERVER &&
+ ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER &&
+ rec_epoch == 0 &&
+ ssl->in_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE &&
+ ssl->in_left > 13 &&
+ ssl->in_buf[13] == MBEDTLS_SSL_HS_CLIENT_HELLO )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "possible client reconnect "
+ "from the same port" ) );
+ return( ssl_handle_possible_reconnect( ssl ) );
+ }
+ else
+#endif /* MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE && MBEDTLS_SSL_SRV_C */
+ return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD );
+ }
+
+#if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY)
+ /* Replay detection only works for the current epoch */
+ if( rec_epoch == ssl->in_epoch &&
+ mbedtls_ssl_dtls_replay_check( ssl ) != 0 )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "replayed record" ) );
+ return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD );
+ }
+#endif
+ }
+#endif /* MBEDTLS_SSL_PROTO_DTLS */
+
return( 0 );
}
@@ -3752,13 +3765,26 @@
if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM &&
ret != MBEDTLS_ERR_SSL_CLIENT_RECONNECT )
{
- /* Ignore bad record and get next one; drop the whole datagram
- * since current header cannot be trusted to find the next record
- * in current datagram */
- ssl->next_record_offset = 0;
- ssl->in_left = 0;
+ if( ret == MBEDTLS_ERR_SSL_UNEXPECTED_RECORD )
+ {
+ /* Skip unexpected record (but not whole datagram) */
+ ssl->next_record_offset = ssl->in_msglen
+ + mbedtls_ssl_hdr_len( ssl );
- MBEDTLS_SSL_DEBUG_MSG( 1, ( "discarding invalid record (header)" ) );
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "discarding unexpected record "
+ "(header)" ) );
+ }
+ else
+ {
+ /* Skip invalid record and the rest of the datagram */
+ ssl->next_record_offset = 0;
+ ssl->in_left = 0;
+
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "discarding invalid record "
+ "(header)" ) );
+ }
+
+ /* Get next record */
goto read_record_header;
}
#endif