Merge branch 'iotssl-1155-hashlen-type'

Introduces additional checks in the PK module for 64-bit systems only. The
problem is that the API functions in the PK abstraction accept a size_t value
for the hashlen, while the RSA module accepts an unsigned int for the hashlen.
Instead of silently casting size_t to unsigned int, this change checks whether
the hashlen overflows an unsigned int and returns an error.
This commit is contained in:
Simon Butcher 2017-02-25 15:59:18 +00:00
commit 53695beeda
5 changed files with 72 additions and 1 deletions

View File

@ -3,6 +3,11 @@ mbed TLS ChangeLog (Sorted per branch, date)
= mbed TLS 2.x.x branch released xxxx-xx-xx = mbed TLS 2.x.x branch released xxxx-xx-xx
Security Security
* Add checks to prevent signature forgeries for very large messages while
using RSA through the PK module in 64-bit systems. The issue was caused by
some data loss when casting a size_t to an unsigned int value in the
functions rsa_verify_wrap(), rsa_sign_wrap(), rsa_alt_sign_wrap() and
mbedtls_pk_sign(). Found by Jean-Philippe Aumasson.
* Removed MD5 from the allowed hash algorithms for CertificateRequest and * Removed MD5 from the allowed hash algorithms for CertificateRequest and
CertificateVerify messages, to prevent SLOTH attacks against TLS 1.2. CertificateVerify messages, to prevent SLOTH attacks against TLS 1.2.
Introduced by interoperability fix for #513. Introduced by interoperability fix for #513.

View File

@ -29,6 +29,8 @@
#include "mbedtls/pk.h" #include "mbedtls/pk.h"
#include "mbedtls/pk_internal.h" #include "mbedtls/pk_internal.h"
#include "mbedtls/bignum.h"
#if defined(MBEDTLS_RSA_C) #if defined(MBEDTLS_RSA_C)
#include "mbedtls/rsa.h" #include "mbedtls/rsa.h"
#endif #endif
@ -39,6 +41,8 @@
#include "mbedtls/ecdsa.h" #include "mbedtls/ecdsa.h"
#endif #endif
#include <limits.h>
/* Implementation that should never be optimized out by the compiler */ /* Implementation that should never be optimized out by the compiler */
static void mbedtls_zeroize( void *v, size_t n ) { static void mbedtls_zeroize( void *v, size_t n ) {
volatile unsigned char *p = v; while( n-- ) *p++ = 0; volatile unsigned char *p = v; while( n-- ) *p++ = 0;
@ -209,6 +213,11 @@ int mbedtls_pk_verify_ext( mbedtls_pk_type_t type, const void *options,
int ret; int ret;
const mbedtls_pk_rsassa_pss_options *pss_opts; const mbedtls_pk_rsassa_pss_options *pss_opts;
#if defined(MBEDTLS_HAVE_INT64)
if( md_alg == MBEDTLS_MD_NONE && UINT_MAX < hash_len )
return( MBEDTLS_ERR_PK_BAD_INPUT_DATA );
#endif /* MBEDTLS_HAVE_INT64 */
if( options == NULL ) if( options == NULL )
return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); return( MBEDTLS_ERR_PK_BAD_INPUT_DATA );
@ -232,7 +241,7 @@ int mbedtls_pk_verify_ext( mbedtls_pk_type_t type, const void *options,
return( 0 ); return( 0 );
#else #else
return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE ); return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE );
#endif #endif /* MBEDTLS_RSA_C && MBEDTLS_PKCS1_V21 */
} }
/* General case: no options */ /* General case: no options */

View File

@ -30,6 +30,7 @@
/* Even if RSA not activated, for the sake of RSA-alt */ /* Even if RSA not activated, for the sake of RSA-alt */
#include "mbedtls/rsa.h" #include "mbedtls/rsa.h"
#include "mbedtls/bignum.h"
#include <string.h> #include <string.h>
@ -49,6 +50,8 @@
#define mbedtls_free free #define mbedtls_free free
#endif #endif
#include <limits.h>
#if defined(MBEDTLS_PK_RSA_ALT_SUPPORT) #if defined(MBEDTLS_PK_RSA_ALT_SUPPORT)
/* Implementation that should never be optimized out by the compiler */ /* Implementation that should never be optimized out by the compiler */
static void mbedtls_zeroize( void *v, size_t n ) { static void mbedtls_zeroize( void *v, size_t n ) {
@ -74,6 +77,11 @@ static int rsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg,
{ {
int ret; int ret;
#if defined(MBEDTLS_HAVE_INT64)
if( md_alg == MBEDTLS_MD_NONE && UINT_MAX < hash_len )
return( MBEDTLS_ERR_PK_BAD_INPUT_DATA );
#endif /* MBEDTLS_HAVE_INT64 */
if( sig_len < ((mbedtls_rsa_context *) ctx)->len ) if( sig_len < ((mbedtls_rsa_context *) ctx)->len )
return( MBEDTLS_ERR_RSA_VERIFY_FAILED ); return( MBEDTLS_ERR_RSA_VERIFY_FAILED );
@ -93,6 +101,11 @@ static int rsa_sign_wrap( void *ctx, mbedtls_md_type_t md_alg,
unsigned char *sig, size_t *sig_len, unsigned char *sig, size_t *sig_len,
int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) int (*f_rng)(void *, unsigned char *, size_t), void *p_rng )
{ {
#if defined(MBEDTLS_HAVE_INT64)
if( md_alg == MBEDTLS_MD_NONE && UINT_MAX < hash_len )
return( MBEDTLS_ERR_PK_BAD_INPUT_DATA );
#endif /* MBEDTLS_HAVE_INT64 */
*sig_len = ((mbedtls_rsa_context *) ctx)->len; *sig_len = ((mbedtls_rsa_context *) ctx)->len;
return( mbedtls_rsa_pkcs1_sign( (mbedtls_rsa_context *) ctx, f_rng, p_rng, MBEDTLS_RSA_PRIVATE, return( mbedtls_rsa_pkcs1_sign( (mbedtls_rsa_context *) ctx, f_rng, p_rng, MBEDTLS_RSA_PRIVATE,
@ -402,6 +415,11 @@ static int rsa_alt_sign_wrap( void *ctx, mbedtls_md_type_t md_alg,
{ {
mbedtls_rsa_alt_context *rsa_alt = (mbedtls_rsa_alt_context *) ctx; mbedtls_rsa_alt_context *rsa_alt = (mbedtls_rsa_alt_context *) ctx;
#if defined(MBEDTLS_HAVE_INT64)
if( UINT_MAX < hash_len )
return( MBEDTLS_ERR_PK_BAD_INPUT_DATA );
#endif /* MBEDTLS_HAVE_INT64 */
*sig_len = rsa_alt->key_len_func( rsa_alt->key ); *sig_len = rsa_alt->key_len_func( rsa_alt->key );
return( rsa_alt->sign_func( rsa_alt->key, f_rng, p_rng, MBEDTLS_RSA_PRIVATE, return( rsa_alt->sign_func( rsa_alt->key, f_rng, p_rng, MBEDTLS_RSA_PRIVATE,

View File

@ -150,3 +150,6 @@ Check pair #5 (RSA vs EC)
depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C
mbedtls_pk_check_pair:"data_files/ec_256_pub.pem":"data_files/server1.key":MBEDTLS_ERR_PK_TYPE_MISMATCH mbedtls_pk_check_pair:"data_files/ec_256_pub.pem":"data_files/server1.key":MBEDTLS_ERR_PK_TYPE_MISMATCH
RSA hash_len overflow (size_t vs unsigned int)
depends_on:MBEDTLS_RSA_C:MBEDTLS_HAVE_INT64
pk_rsa_overflow:

View File

@ -5,6 +5,9 @@
#include "mbedtls/ecp.h" #include "mbedtls/ecp.h"
#include "mbedtls/rsa.h" #include "mbedtls/rsa.h"
/* For detecting 64-bit compilation */
#include "mbedtls/bignum.h"
static int rnd_std_rand( void *rng_state, unsigned char *output, size_t len ); static int rnd_std_rand( void *rng_state, unsigned char *output, size_t len );
#define RSA_KEY_SIZE 512 #define RSA_KEY_SIZE 512
@ -414,6 +417,34 @@ exit:
} }
/* END_CASE */ /* END_CASE */
/* BEGIN_CASE depends_on:MBEDTLS_RSA_C:MBEDTLS_HAVE_INT64 */
void pk_rsa_overflow( )
{
mbedtls_pk_context pk;
size_t hash_len = (size_t)-1;
mbedtls_pk_init( &pk );
TEST_ASSERT( mbedtls_pk_setup( &pk,
mbedtls_pk_info_from_type( MBEDTLS_PK_RSA ) ) == 0 );
#if defined(MBEDTLS_PKCS1_V21)
TEST_ASSERT( mbedtls_pk_verify_ext( MBEDTLS_PK_RSASSA_PSS, NULL, &pk,
MBEDTLS_MD_NONE, NULL, hash_len, NULL, 0 ) ==
MBEDTLS_ERR_PK_BAD_INPUT_DATA );
#endif /* MBEDTLS_PKCS1_V21 */
TEST_ASSERT( mbedtls_pk_verify( &pk, MBEDTLS_MD_NONE, NULL, hash_len,
NULL, 0 ) == MBEDTLS_ERR_PK_BAD_INPUT_DATA );
TEST_ASSERT( mbedtls_pk_sign( &pk, MBEDTLS_MD_NONE, NULL, hash_len, NULL, 0,
rnd_std_rand, NULL ) == MBEDTLS_ERR_PK_BAD_INPUT_DATA );
exit:
mbedtls_pk_free( &pk );
}
/* END_CASE */
/* BEGIN_CASE depends_on:MBEDTLS_RSA_C:MBEDTLS_PK_RSA_ALT_SUPPORT */ /* BEGIN_CASE depends_on:MBEDTLS_RSA_C:MBEDTLS_PK_RSA_ALT_SUPPORT */
void pk_rsa_alt( ) void pk_rsa_alt( )
{ {
@ -461,6 +492,11 @@ void pk_rsa_alt( )
/* Test signature */ /* Test signature */
TEST_ASSERT( mbedtls_pk_sign( &alt, MBEDTLS_MD_NONE, hash, sizeof hash, TEST_ASSERT( mbedtls_pk_sign( &alt, MBEDTLS_MD_NONE, hash, sizeof hash,
sig, &sig_len, rnd_std_rand, NULL ) == 0 ); sig, &sig_len, rnd_std_rand, NULL ) == 0 );
#if defined(MBEDTLS_HAVE_INT64)
TEST_ASSERT( mbedtls_pk_sign( &alt, MBEDTLS_MD_NONE, hash, (size_t)-1,
NULL, NULL, rnd_std_rand, NULL ) ==
MBEDTLS_ERR_PK_BAD_INPUT_DATA );
#endif /* MBEDTLS_HAVE_INT64 */
TEST_ASSERT( sig_len == RSA_KEY_LEN ); TEST_ASSERT( sig_len == RSA_KEY_LEN );
TEST_ASSERT( mbedtls_pk_verify( &rsa, MBEDTLS_MD_NONE, TEST_ASSERT( mbedtls_pk_verify( &rsa, MBEDTLS_MD_NONE,
hash, sizeof hash, sig, sig_len ) == 0 ); hash, sizeof hash, sig, sig_len ) == 0 );