Merge remote-tracking branch 'restricted/pr/403' into development-restricted
* restricted/pr/403:
Correct record header size in case of TLS
Don't allocate space for DTLS header if DTLS is disabled
Improve debugging output
Adapt ChangeLog
Add run-time check for handshake message size in ssl_write_record
Add run-time check for record content size in ssl_encrypt_buf
Add compile-time checks for size of record content and payload
diff --git a/ChangeLog b/ChangeLog
index 6649589..a740d5d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -83,6 +83,8 @@
Reported by Yolan Romailler.
* Fix word size check in in pk.c to not depend on MBEDTLS_HAVE_INT64.
* Fix incorrect unit in benchmark output. #850
+ * Add size-checks for record and handshake message content, securing
+ fragile yet non-exploitable code-paths.
Changes
* Extend cert_write example program by options to set the CRT version
diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h
index 8d3ab61..f87397f 100644
--- a/include/mbedtls/ssl_internal.h
+++ b/include/mbedtls/ssl_internal.h
@@ -24,6 +24,7 @@
#define MBEDTLS_SSL_INTERNAL_H
#include "ssl.h"
+#include "cipher.h"
#if defined(MBEDTLS_MD5_C)
#include "md5.h"
@@ -138,14 +139,34 @@
#define MBEDTLS_SSL_PADDING_ADD 0
#endif
-#define MBEDTLS_SSL_BUFFER_LEN ( MBEDTLS_SSL_MAX_CONTENT_LEN \
- + MBEDTLS_SSL_COMPRESSION_ADD \
- + 29 /* counter + header + IV */ \
- + MBEDTLS_SSL_MAC_ADD \
- + MBEDTLS_SSL_PADDING_ADD \
+#define MBEDTLS_SSL_PAYLOAD_LEN ( MBEDTLS_SSL_MAX_CONTENT_LEN \
+ + MBEDTLS_SSL_COMPRESSION_ADD \
+ + MBEDTLS_MAX_IV_LENGTH \
+ + MBEDTLS_SSL_MAC_ADD \
+ + MBEDTLS_SSL_PADDING_ADD \
)
/*
+ * Check that we obey the standard's message size bounds
+ */
+
+#if MBEDTLS_SSL_MAX_CONTENT_LEN > 16384
+#error Bad configuration - record content too large.
+#endif
+
+#if MBEDTLS_SSL_PAYLOAD_LEN > 16384 + 2048
+#error Bad configuration - protected record payload too large.
+#endif
+
+/* Note: Even though the TLS record header is only 5 bytes
+ long, we're internally using 8 bytes to store the
+ implicit sequence number. */
+#define MBEDTLS_SSL_HEADER_LEN 13
+
+#define MBEDTLS_SSL_BUFFER_LEN \
+ ( ( MBEDTLS_SSL_HEADER_LEN ) + ( MBEDTLS_SSL_PAYLOAD_LEN ) )
+
+/*
* TLS extension flags (for extensions with outgoing ServerHello content
* that need it (e.g. for RENEGOTIATION_INFO the server already knows because
* of state of the renegotiation flag, so no indicator is required)
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index a8aa1c0..090b2f3 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -1268,6 +1268,14 @@
MBEDTLS_SSL_DEBUG_BUF( 4, "before encrypt: output payload",
ssl->out_msg, ssl->out_msglen );
+ if( ssl->out_msglen > MBEDTLS_SSL_MAX_CONTENT_LEN )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "Record content %u too large, maximum %d",
+ (unsigned) ssl->out_msglen,
+ MBEDTLS_SSL_MAX_CONTENT_LEN ) );
+ return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+ }
+
/*
* Add MAC before if needed
*/
@@ -2733,6 +2741,15 @@
if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM )
{
/* Make room for the additional DTLS fields */
+ if( MBEDTLS_SSL_MAX_CONTENT_LEN - ssl->out_msglen < 8 )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1, ( "DTLS handshake message too large: "
+ "size %u, maximum %u",
+ (unsigned) ( ssl->in_hslen - 4 ),
+ (unsigned) ( MBEDTLS_SSL_MAX_CONTENT_LEN - 12 ) ) );
+ return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+ }
+
memmove( ssl->out_msg + 12, ssl->out_msg + 4, len - 4 );
ssl->out_msglen += 8;
len += 8;