Merge commit '8b9bcec' into dtls

* commit '8b9bcec':
  Stop assuming chars are signed
  Fix len miscalculation in buffer-based allocator
  Fix NULL dereference in buffer-based allocator
  Add test_suite_memory_buffer_alloc
  Add memory_buffer_alloc_self_test()
  Fix missing bound check
  Add test for ctr_drbg_update() input sanitizing
  Refactor for clearer correctness/security
  Stop assuming chars are signed

Conflicts:
	library/ssl_tls.c
This commit is contained in:
Manuel Pégourié-Gonnard 2015-01-20 16:36:03 +00:00
commit f9c8a606b5
14 changed files with 270 additions and 65 deletions

View file

@ -19,6 +19,19 @@ Features
* Add support for Extended Master Secret (draft-ietf-tls-session-hash)
* Add support for Encrypt-then-MAC (RFC 7366)
Security
* NULL pointer dereference in the buffer-based allocator when the buffer is
full and polarssl_free() is called (found by Jean-Philippe Aumasson)
(only possible if POLARSSL_MEMORY_BUFFER_ALLOC_C is enabled, which it is
not by default).
Bugfix
* Stack buffer overflow if ctr_drbg_update() is called with too large
add_len (found by Jean-Philippe Aumasson) (not triggerable remotely).
* Possible buffer overflow of length at most POLARSSL_MEMORY_ALIGN_MULTIPLE
if memory_buffer_alloc_init() was called with buf not aligned and len not
a multiple of POLARSSL_MEMORY_ALIGN_MULTIPLE.
= PolarSSL 1.3.9 released 2014-10-20
Security
* Lowest common hash was selected from signature_algorithms extension in

View file

@ -188,6 +188,10 @@ int ctr_drbg_reseed( ctr_drbg_context *ctx,
* \param ctx CTR_DRBG context
* \param additional Additional data to update state with
* \param add_len Length of additional data
*
* \note If add_len is greater than CTR_DRBG_MAX_SEED_INPUT,
* only the first CTR_DRBG_MAX_SEED_INPUT bytes are used,
* the remaining ones are silently discarded.
*/
void ctr_drbg_update( ctr_drbg_context *ctx,
const unsigned char *additional, size_t add_len );

View file

@ -115,6 +115,15 @@ void memory_buffer_alloc_status( void );
*/
int memory_buffer_alloc_verify( void );
#if defined(POLARSSL_SELF_TEST)
/**
* \brief Checkup routine
*
* \return 0 if successful, or 1 if a test failed
*/
int memory_buffer_alloc_self_test( int verbose );
#endif
#ifdef __cplusplus
}
#endif

View file

@ -137,6 +137,9 @@ static int block_cipher_df( unsigned char *output,
int i, j;
size_t buf_len, use_len;
if( data_len > CTR_DRBG_MAX_SEED_INPUT )
return( POLARSSL_ERR_CTR_DRBG_INPUT_TOO_BIG );
memset( buf, 0, CTR_DRBG_MAX_SEED_INPUT + CTR_DRBG_BLOCKSIZE + 16 );
aes_init( &aes_ctx );
@ -256,6 +259,11 @@ void ctr_drbg_update( ctr_drbg_context *ctx,
if( add_len > 0 )
{
/* MAX_INPUT would be more logical here, but we have to match
* block_cipher_df()'s limits since we can't propagate errors */
if( add_len > CTR_DRBG_MAX_SEED_INPUT )
add_len = CTR_DRBG_MAX_SEED_INPUT;
block_cipher_df( add_input, additional, add_len );
ctr_drbg_update_internal( ctx, add_input );
}

View file

@ -484,7 +484,8 @@ static void buffer_alloc_free( void *ptr )
if( old == NULL )
{
hdr->next_free = heap.first_free;
heap.first_free->prev_free = hdr;
if( heap.first_free != NULL )
heap.first_free->prev_free = hdr;
heap.first_free = hdr;
}
@ -562,9 +563,11 @@ int memory_buffer_alloc_init( unsigned char *buf, size_t len )
if( (size_t) buf % POLARSSL_MEMORY_ALIGN_MULTIPLE )
{
/* Adjust len first since buf is used in the computation */
len -= POLARSSL_MEMORY_ALIGN_MULTIPLE
- (size_t) buf % POLARSSL_MEMORY_ALIGN_MULTIPLE;
buf += POLARSSL_MEMORY_ALIGN_MULTIPLE
- (size_t) buf % POLARSSL_MEMORY_ALIGN_MULTIPLE;
len -= (size_t) buf % POLARSSL_MEMORY_ALIGN_MULTIPLE;
}
heap.buf = buf;
@ -586,4 +589,135 @@ void memory_buffer_alloc_free()
polarssl_zeroize( &heap, sizeof(buffer_alloc_ctx) );
}
#if defined(POLARSSL_SELF_TEST)
static int check_pointer( void *p )
{
if( p == NULL )
return( -1 );
if( (size_t) p % POLARSSL_MEMORY_ALIGN_MULTIPLE != 0 )
return( -1 );
return( 0 );
}
static int check_all_free( )
{
if( heap.current_alloc_size != 0 ||
heap.first != heap.first_free ||
(void *) heap.first != (void *) heap.buf )
{
return( -1 );
}
return( 0 );
}
#define TEST_ASSERT( condition ) \
if( ! (condition) ) \
{ \
if( verbose != 0 ) \
polarssl_printf( "failed\n" ); \
\
ret = 1; \
goto cleanup; \
}
int memory_buffer_alloc_self_test( int verbose )
{
unsigned char buf[1024];
unsigned char *p, *q, *r, *end;
int ret = 0;
if( verbose != 0 )
polarssl_printf( " MBA test #1 (basic alloc-free cycle): " );
memory_buffer_alloc_init( buf, sizeof( buf ) );
p = polarssl_malloc( 1 );
q = polarssl_malloc( 128 );
r = polarssl_malloc( 16 );
TEST_ASSERT( check_pointer( p ) == 0 &&
check_pointer( q ) == 0 &&
check_pointer( r ) == 0 );
polarssl_free( r );
polarssl_free( q );
polarssl_free( p );
TEST_ASSERT( check_all_free( ) == 0 );
/* Memorize end to compare with the next test */
end = heap.buf + heap.len;
memory_buffer_alloc_free( );
if( verbose != 0 )
polarssl_printf( "passed\n" );
if( verbose != 0 )
polarssl_printf( " MBA test #2 (buf not aligned): " );
memory_buffer_alloc_init( buf + 1, sizeof( buf ) - 1 );
TEST_ASSERT( heap.buf + heap.len == end );
p = polarssl_malloc( 1 );
q = polarssl_malloc( 128 );
r = polarssl_malloc( 16 );
TEST_ASSERT( check_pointer( p ) == 0 &&
check_pointer( q ) == 0 &&
check_pointer( r ) == 0 );
polarssl_free( r );
polarssl_free( q );
polarssl_free( p );
TEST_ASSERT( check_all_free( ) == 0 );
memory_buffer_alloc_free( );
if( verbose != 0 )
polarssl_printf( "passed\n" );
if( verbose != 0 )
polarssl_printf( " MBA test #3 (full): " );
memory_buffer_alloc_init( buf, sizeof( buf ) );
p = polarssl_malloc( sizeof( buf ) - sizeof( memory_header ) );
TEST_ASSERT( check_pointer( p ) == 0 );
TEST_ASSERT( polarssl_malloc( 1 ) == NULL );
polarssl_free( p );
p = polarssl_malloc( sizeof( buf ) - 2 * sizeof( memory_header ) - 16 );
q = polarssl_malloc( 16 );
TEST_ASSERT( check_pointer( p ) == 0 && check_pointer( q ) == 0 );
TEST_ASSERT( polarssl_malloc( 1 ) == NULL );
polarssl_free( q );
TEST_ASSERT( polarssl_malloc( 17 ) == NULL );
polarssl_free( p );
TEST_ASSERT( check_all_free( ) == 0 );
memory_buffer_alloc_free( );
if( verbose != 0 )
polarssl_printf( "passed\n" );
cleanup:
memory_buffer_alloc_free( );
return( ret );
}
#endif /* POLARSSL_SELF_TEST */
#endif /* POLARSSL_MEMORY_BUFFER_ALLOC_C */

View file

@ -1145,70 +1145,40 @@ static void ssl_mac( md_context_t *md_ctx, unsigned char *secret,
}
#endif /* POLARSSL_SSL_PROTO_SSL3 */
#define MAC_NONE 0
#define MAC_PLAINTEXT 1
#define MAC_CIPHERTEXT 2
#if defined(POLARSSL_ARC4_C) || defined(POLARSSL_CIPHER_NULL_CIPHER) || \
( defined(POLARSSL_CIPHER_MODE_CBC) && \
( defined(POLARSSL_AES_C) || defined(POLARSSL_CAMELLIA_C) ) )
#define POLARSSL_SOME_MODES_USE_MAC
#endif
/*
* Is MAC applied on ciphertext, cleartext or not at all?
*/
#if defined(POLARSSL_SOME_MODES_USE_MAC)
static char ssl_get_mac_order( ssl_context *ssl,
const ssl_session *session,
cipher_mode_t mode )
{
#if defined(POLARSSL_ARC4_C) || defined(POLARSSL_CIPHER_NULL_CIPHER)
if( mode == POLARSSL_MODE_STREAM )
return( MAC_PLAINTEXT );
#endif
#if defined(POLARSSL_CIPHER_MODE_CBC) && \
( defined(POLARSSL_AES_C) || defined(POLARSSL_CAMELLIA_C) )
if( mode == POLARSSL_MODE_CBC )
{
#if defined(POLARSSL_SSL_ENCRYPT_THEN_MAC)
if( session != NULL && session->encrypt_then_mac == SSL_ETM_ENABLED )
{
SSL_DEBUG_MSG( 3, ( "using encrypt then mac" ) );
return( MAC_CIPHERTEXT );
}
#else
((void) ssl);
((void) session);
#endif
return( MAC_PLAINTEXT );
}
#else
((void) ssl);
((void) session);
#endif
return( MAC_NONE );
}
#endif /* POLARSSL_SOME_MODES_USE_MAC */
/*
* Encryption/decryption functions
*/
static int ssl_encrypt_buf( ssl_context *ssl )
{
const cipher_mode_t mode = cipher_get_cipher_mode(
&ssl->transform_out->cipher_ctx_enc );
cipher_mode_t mode;
int auth_done = 0;
SSL_DEBUG_MSG( 2, ( "=> encrypt buf" ) );
if( ssl->session_out == NULL || ssl->transform_out == NULL )
{
SSL_DEBUG_MSG( 1, ( "should never happen" ) );
return( POLARSSL_ERR_SSL_INTERNAL_ERROR );
}
mode = cipher_get_cipher_mode( &ssl->transform_out->cipher_ctx_enc );
/*
* Add MAC before if needed
*/
#if defined(POLARSSL_SOME_MODES_USE_MAC)
if( ssl_get_mac_order( ssl, ssl->session_out, mode ) == MAC_PLAINTEXT )
if( mode == POLARSSL_MODE_STREAM ||
( mode == POLARSSL_MODE_CBC
#if defined(POLARSSL_SSL_ENCRYPT_THEN_MAC)
&& ssl->session_out->encrypt_then_mac == SSL_ETM_DISABLED
#endif
) )
{
#if defined(POLARSSL_SSL_PROTO_SSL3)
if( ssl->minor_ver == SSL_MINOR_VERSION_0 )
@ -1245,6 +1215,7 @@ static int ssl_encrypt_buf( ssl_context *ssl )
ssl->transform_out->maclen );
ssl->out_msglen += ssl->transform_out->maclen;
auth_done++;
}
#endif /* AEAD not the only option */
@ -1356,6 +1327,7 @@ static int ssl_encrypt_buf( ssl_context *ssl )
}
ssl->out_msglen += taglen;
auth_done++;
SSL_DEBUG_BUF( 4, "after encrypt: tag", enc_msg + enc_msglen, taglen );
}
@ -1446,7 +1418,7 @@ static int ssl_encrypt_buf( ssl_context *ssl )
#endif
#if defined(POLARSSL_SSL_ENCRYPT_THEN_MAC)
if( ssl_get_mac_order( ssl, ssl->session_out, mode ) == MAC_CIPHERTEXT )
if( auth_done == 0 )
{
/*
* MAC(MAC_write_key, seq_num +
@ -1458,6 +1430,8 @@ static int ssl_encrypt_buf( ssl_context *ssl )
*/
unsigned char pseudo_hdr[13];
SSL_DEBUG_MSG( 3, ( "using encrypt then mac" ) );
memcpy( pseudo_hdr + 0, ssl->out_ctr, 8 );
memcpy( pseudo_hdr + 8, ssl->out_hdr, 3 );
pseudo_hdr[11] = (unsigned char)( ( ssl->out_msglen >> 8 ) & 0xFF );
@ -1473,6 +1447,7 @@ static int ssl_encrypt_buf( ssl_context *ssl )
md_hmac_reset( &ssl->transform_out->md_ctx_enc );
ssl->out_msglen += ssl->transform_out->maclen;
auth_done++;
}
#endif /* POLARSSL_SSL_ENCRYPT_THEN_MAC */
}
@ -1484,6 +1459,13 @@ static int ssl_encrypt_buf( ssl_context *ssl )
return( POLARSSL_ERR_SSL_INTERNAL_ERROR );
}
/* Make extra sure authentication was performed, exactly once */
if( auth_done != 1 )
{
SSL_DEBUG_MSG( 1, ( "should never happen" ) );
return( POLARSSL_ERR_SSL_INTERNAL_ERROR );
}
SSL_DEBUG_MSG( 2, ( "<= encrypt buf" ) );
return( 0 );
@ -1494,14 +1476,22 @@ static int ssl_encrypt_buf( ssl_context *ssl )
static int ssl_decrypt_buf( ssl_context *ssl )
{
size_t i;
const cipher_mode_t mode = cipher_get_cipher_mode(
&ssl->transform_in->cipher_ctx_dec );
cipher_mode_t mode;
int auth_done = 0;
#if defined(POLARSSL_SOME_MODES_USE_MAC)
size_t padlen = 0, correct = 1;
#endif
SSL_DEBUG_MSG( 2, ( "=> decrypt buf" ) );
if( ssl->session_in == NULL || ssl->transform_in == NULL )
{
SSL_DEBUG_MSG( 1, ( "should never happen" ) );
return( POLARSSL_ERR_SSL_INTERNAL_ERROR );
}
mode = cipher_get_cipher_mode( &ssl->transform_in->cipher_ctx_dec );
if( ssl->in_msglen < ssl->transform_in->minlen )
{
SSL_DEBUG_MSG( 1, ( "in_msglen (%d) < minlen (%d)",
@ -1598,6 +1588,7 @@ static int ssl_decrypt_buf( ssl_context *ssl )
return( ret );
}
auth_done++;
if( olen != dec_msglen )
{
@ -1647,11 +1638,13 @@ static int ssl_decrypt_buf( ssl_context *ssl )
* Authenticate before decrypt if enabled
*/
#if defined(POLARSSL_SSL_ENCRYPT_THEN_MAC)
if( ssl_get_mac_order( ssl, ssl->session_in, mode ) == MAC_CIPHERTEXT )
if( ssl->session_in->encrypt_then_mac == SSL_ETM_ENABLED )
{
unsigned char computed_mac[POLARSSL_SSL_MAX_MAC_SIZE];
unsigned char pseudo_hdr[13];
SSL_DEBUG_MSG( 3, ( "using encrypt then mac" ) );
dec_msglen -= ssl->transform_in->maclen;
ssl->in_msglen -= ssl->transform_in->maclen;
@ -1680,6 +1673,7 @@ static int ssl_decrypt_buf( ssl_context *ssl )
return( POLARSSL_ERR_SSL_INVALID_MAC );
}
auth_done++;
}
#endif /* POLARSSL_SSL_ENCRYPT_THEN_MAC */
@ -1738,7 +1732,7 @@ static int ssl_decrypt_buf( ssl_context *ssl )
padlen = 1 + ssl->in_msg[ssl->in_msglen - 1];
if( ssl->in_msglen < ssl->transform_in->maclen + padlen &&
ssl_get_mac_order( ssl, ssl->session_in, mode ) == MAC_PLAINTEXT )
auth_done == 0 )
{
#if defined(POLARSSL_SSL_DEBUG_ALL)
SSL_DEBUG_MSG( 1, ( "msglen (%d) < maclen (%d) + padlen (%d)",
@ -1830,10 +1824,8 @@ static int ssl_decrypt_buf( ssl_context *ssl )
* Authenticate if not done yet.
* Compute the MAC regardless of the padding result (RFC4346, CBCTIME).
*/
#if defined(POLARSSL_ARC4_C) || defined(POLARSSL_CIPHER_NULL_CIPHER) || \
( defined(POLARSSL_CIPHER_MODE_CBC) && \
( defined(POLARSSL_AES_C) || defined(POLARSSL_CAMELLIA_C) ) )
if( ssl_get_mac_order( ssl, ssl->session_in, mode ) == MAC_PLAINTEXT )
#if defined(POLARSSL_SOME_MODES_USE_MAC)
if( auth_done == 0 )
{
unsigned char tmp[POLARSSL_SSL_MAX_MAC_SIZE];
@ -1909,6 +1901,7 @@ static int ssl_decrypt_buf( ssl_context *ssl )
#endif
correct = 0;
}
auth_done++;
/*
* Finally check the correct flag
@ -1916,7 +1909,14 @@ static int ssl_decrypt_buf( ssl_context *ssl )
if( correct == 0 )
return( POLARSSL_ERR_SSL_INVALID_MAC );
}
#endif /* AEAD not the only option */
#endif /* POLARSSL_SOME_MODES_USE_MAC */
/* Make extra sure authentication was performed, exactly once */
if( auth_done != 1 )
{
SSL_DEBUG_MSG( 1, ( "should never happen" ) );
return( POLARSSL_ERR_SSL_INTERNAL_ERROR );
}
if( ssl->in_msglen == 0 )
{

View file

@ -146,8 +146,8 @@ struct options
uint32_t hs_to_min; /* Initial value of DTLS handshake timer */
uint32_t hs_to_max; /* Max value of DTLS handshake timer */
int fallback; /* is this a fallback connection? */
char extended_ms; /* negotiate extended master secret? */
char etm; /* negotiate encrypt then mac? ? */
int extended_ms; /* negotiate extended master secret? */
int etm; /* negotiate encrypt then mac? */
} opt;
static void my_debug( void *ctx, int level, const char *str )

View file

@ -226,16 +226,23 @@ int main( int argc, char *argv[] )
#if defined(POLARSSL_MEMORY_BUFFER_ALLOC_C) && defined(POLARSSL_MEMORY_DEBUG)
memory_buffer_alloc_status();
#endif
}
#if defined(POLARSSL_MEMORY_BUFFER_ALLOC_C)
memory_buffer_alloc_free();
if( ( ret = memory_buffer_alloc_self_test( v ) ) != 0 )
return( ret );
#endif
if( v != 0 )
{
printf( " [ All tests passed ]\n\n" );
#if defined(_WIN32)
printf( " Press Enter to exit this program.\n" );
fflush( stdout ); getchar();
#endif
}
#if defined(POLARSSL_MEMORY_BUFFER_ALLOC_C)
memory_buffer_alloc_free();
#endif
return( ret );
}

View file

@ -77,6 +77,7 @@ add_test_suite(hmac_drbg hmac_drbg.pr)
add_test_suite(hmac_shax)
add_test_suite(md)
add_test_suite(mdx)
add_test_suite(memory_buffer_alloc)
add_test_suite(mpi)
add_test_suite(pbkdf2)
add_test_suite(pem)

View file

@ -59,6 +59,7 @@ APPS = test_suite_aes.ecb test_suite_aes.cbc \
test_suite_hmac_drbg.nopr \
test_suite_hmac_drbg.pr \
test_suite_md test_suite_mdx \
test_suite_memory_buffer_alloc \
test_suite_mpi test_suite_pbkdf2 \
test_suite_pem \
test_suite_pkcs1_v21 test_suite_pkcs5 \
@ -337,6 +338,10 @@ test_suite_mdx: test_suite_mdx.c $(DEP)
echo " CC $@.c"
$(CC) $(CFLAGS) $(OFLAGS) $@.c $(LDFLAGS) -o $@
test_suite_memory_buffer_alloc: test_suite_memory_buffer_alloc.c $(DEP)
echo " CC $@.c"
$(CC) $(CFLAGS) $(OFLAGS) $@.c $(LDFLAGS) -o $@
test_suite_mpi: test_suite_mpi.c $(DEP)
echo " CC $@.c"
$(CC) $(CFLAGS) $(OFLAGS) $@.c $(LDFLAGS) -o $@

View file

@ -211,7 +211,8 @@ int main()
char buf[5000];
char *params[50];
#if defined(POLARSSL_MEMORY_BUFFER_ALLOC_C)
#if defined(POLARSSL_MEMORY_BUFFER_ALLOC_C) && \
!defined(TEST_SUITE_MEMORY_BUFFER_ALLOC)
unsigned char alloc_buf[1000000];
memory_buffer_alloc_init( alloc_buf, sizeof(alloc_buf) );
#endif
@ -298,7 +299,8 @@ int main()
fprintf( stdout, " (%d / %d tests (%d skipped))\n",
total_tests - total_errors, total_tests, total_skipped );
#if defined(POLARSSL_MEMORY_BUFFER_ALLOC_C)
#if defined(POLARSSL_MEMORY_BUFFER_ALLOC_C) && \
!defined(TEST_SUITE_MEMORY_BUFFER_ALLOC)
#if defined(POLARSSL_MEMORY_DEBUG)
memory_buffer_alloc_status();
#endif

View file

@ -141,6 +141,10 @@ void ctr_drbg_entropy_usage( )
}
TEST_ASSERT( last_idx == test_offset_idx );
/* Call update with too much data (sizeof entropy > MAX(_SEED)_INPUT)
* (just make sure it doesn't cause memory corruption) */
ctr_drbg_update( &ctx, entropy, sizeof( entropy ) );
/* Now enable PR, so the next few calls should all reseed */
ctr_drbg_set_prediction_resistance( &ctx, CTR_DRBG_PR_ON );
TEST_ASSERT( ctr_drbg_random( &ctx, out, sizeof( out ) ) == 0 );

View file

@ -0,0 +1,2 @@
Memory buffer alloc self test
memory_buffer_alloc_self_test:

View file

@ -0,0 +1,16 @@
/* BEGIN_HEADER */
#include <polarssl/memory_buffer_alloc.h>
#define TEST_SUITE_MEMORY_BUFFER_ALLOC
/* END_HEADER */
/* BEGIN_DEPENDENCIES
* depends_on:POLARSSL_MEMORY_BUFFER_ALLOC_C
* END_DEPENDENCIES
*/
/* BEGIN_CASE depends_on:POLARSSL_SELF_TEST */
void memory_buffer_alloc_self_test( )
{
TEST_ASSERT( memory_buffer_alloc_self_test( 0 ) == 0 );
}
/* END_CASE */