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
This commit is contained in:
commit
d04c623ed6
3 changed files with 45 additions and 5 deletions
|
@ -83,6 +83,8 @@ Bugfix
|
|||
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
|
||||
|
|
|
@ -24,6 +24,7 @@
|
|||
#define MBEDTLS_SSL_INTERNAL_H
|
||||
|
||||
#include "ssl.h"
|
||||
#include "cipher.h"
|
||||
|
||||
#if defined(MBEDTLS_MD5_C)
|
||||
#include "md5.h"
|
||||
|
@ -138,13 +139,33 @@
|
|||
#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
|
||||
|
|
|
@ -1268,6 +1268,14 @@ static int ssl_encrypt_buf( mbedtls_ssl_context *ssl )
|
|||
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 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl )
|
|||
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;
|
||||
|
|
Loading…
Reference in a new issue