Merge pull request #3945 from paul-elliott-arm/fix_pem_write_2_7
Backport 2.7: Add tests for buffer corruption after PEM write
diff --git a/ChangeLog.d/bugfix-2927.txt b/ChangeLog.d/bugfix-2927.txt
new file mode 100644
index 0000000..2213c6e
--- /dev/null
+++ b/ChangeLog.d/bugfix-2927.txt
@@ -0,0 +1,3 @@
+Bugfix
+ * In CTR_DRBG and HMAC_DRBG, don't reset the reseed interval in seed().
+ Fixes #2927.
diff --git a/ChangeLog.d/ecp-bignum-error-checks.txt b/ChangeLog.d/ecp-bignum-error-checks.txt
new file mode 100644
index 0000000..8cad08e
--- /dev/null
+++ b/ChangeLog.d/ecp-bignum-error-checks.txt
@@ -0,0 +1,5 @@
+Bugfix
+ * Fix a memory leak in mbedtls_mpi_sub_abs() when the result was negative
+ (an error condition) and the second operand was aliased to the result.
+ * Fix a case in elliptic curve arithmetic where an out-of-memory condition
+ could go undetected, resulting in an incorrect result.
diff --git a/include/mbedtls/ctr_drbg.h b/include/mbedtls/ctr_drbg.h
index 7aadaf9..24d9870 100644
--- a/include/mbedtls/ctr_drbg.h
+++ b/include/mbedtls/ctr_drbg.h
@@ -200,6 +200,11 @@
* and prepares it for mbedtls_ctr_drbg_seed()
* or mbedtls_ctr_drbg_free().
*
+ * \note The reseed interval is
+ * #MBEDTLS_CTR_DRBG_RESEED_INTERVAL by default.
+ * You can override it by calling
+ * mbedtls_ctr_drbg_set_reseed_interval().
+ *
* \param ctx The CTR_DRBG context to initialize.
*/
void mbedtls_ctr_drbg_init( mbedtls_ctr_drbg_context *ctx );
@@ -280,7 +285,8 @@
size_t len );
/**
- * \brief This function clears CTR_CRBG context data.
+ * \brief This function resets CTR_DRBG context to the state immediately
+ * after initial call of mbedtls_ctr_drbg_init().
*
* \param ctx The CTR_DRBG context to clear.
*/
diff --git a/include/mbedtls/hmac_drbg.h b/include/mbedtls/hmac_drbg.h
index 289cf48..cd23a16 100644
--- a/include/mbedtls/hmac_drbg.h
+++ b/include/mbedtls/hmac_drbg.h
@@ -138,6 +138,10 @@
* This function makes the context ready for mbedtls_hmac_drbg_seed(),
* mbedtls_hmac_drbg_seed_buf() or mbedtls_hmac_drbg_free().
*
+ * \note The reseed interval is #MBEDTLS_HMAC_DRBG_RESEED_INTERVAL
+ * by default. Override this value by calling
+ * mbedtls_hmac_drbg_set_reseed_interval().
+ *
* \param ctx HMAC_DRBG context to be initialized.
*/
void mbedtls_hmac_drbg_init( mbedtls_hmac_drbg_context *ctx );
@@ -378,7 +382,8 @@
int mbedtls_hmac_drbg_random( void *p_rng, unsigned char *output, size_t out_len );
/**
- * \brief Free an HMAC_DRBG context
+ * \brief This function resets HMAC_DRBG context to the state immediately
+ * after initial call of mbedtls_hmac_drbg_init().
*
* \param ctx The HMAC_DRBG context to free.
*/
diff --git a/library/bignum.c b/library/bignum.c
index 0e39e3a..c4eb7b0 100644
--- a/library/bignum.c
+++ b/library/bignum.c
@@ -1201,7 +1201,10 @@
/* If we ran out of space for the carry, it means that the result
* is negative. */
if( n == X->n )
- return( MBEDTLS_ERR_MPI_NEGATIVE_VALUE );
+ {
+ ret = MBEDTLS_ERR_MPI_NEGATIVE_VALUE;
+ goto cleanup;
+ }
--X->p[n];
}
diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c
index 184e09f..bc5cc8f 100644
--- a/library/ctr_drbg.c
+++ b/library/ctr_drbg.c
@@ -86,11 +86,17 @@
{
memset( ctx, 0, sizeof( mbedtls_ctr_drbg_context ) );
+ ctx->reseed_interval = MBEDTLS_CTR_DRBG_RESEED_INTERVAL;
+
#if defined(MBEDTLS_THREADING_C)
mbedtls_mutex_init( &ctx->mutex );
#endif
}
+/*
+ * This function resets CTR_DRBG context to the state immediately
+ * after initial call of mbedtls_ctr_drbg_init().
+ */
void mbedtls_ctr_drbg_free( mbedtls_ctr_drbg_context *ctx )
{
if( ctx == NULL )
@@ -101,6 +107,10 @@
#endif
mbedtls_aes_free( &ctx->aes_ctx );
mbedtls_zeroize( ctx, sizeof( mbedtls_ctr_drbg_context ) );
+ ctx->reseed_interval = MBEDTLS_CTR_DRBG_RESEED_INTERVAL;
+#if defined(MBEDTLS_THREADING_C)
+ mbedtls_mutex_init( &ctx->mutex );
+#endif
}
void mbedtls_ctr_drbg_set_prediction_resistance( mbedtls_ctr_drbg_context *ctx, int resistance )
@@ -379,7 +389,6 @@
if( ctx->entropy_len == 0 )
ctx->entropy_len = MBEDTLS_CTR_DRBG_ENTROPY_LEN;
- ctx->reseed_interval = MBEDTLS_CTR_DRBG_RESEED_INTERVAL;
/*
* Initialize with an empty key
diff --git a/library/ecp_curves.c b/library/ecp_curves.c
index 26f6258..9331587 100644
--- a/library/ecp_curves.c
+++ b/library/ecp_curves.c
@@ -974,7 +974,7 @@
STORE32; i++; \
cur = c > 0 ? c : 0; STORE32; \
cur = 0; while( ++i < MAX32 ) { STORE32; } \
- if( c < 0 ) fix_negative( N, c, &C, bits );
+ if( c < 0 ) MBEDTLS_MPI_CHK( fix_negative( N, c, &C, bits ) );
/*
* If the result is negative, we get it in the form
diff --git a/library/hmac_drbg.c b/library/hmac_drbg.c
index f24a66c..26b15e9 100644
--- a/library/hmac_drbg.c
+++ b/library/hmac_drbg.c
@@ -87,6 +87,8 @@
{
memset( ctx, 0, sizeof( mbedtls_hmac_drbg_context ) );
+ ctx->reseed_interval = MBEDTLS_HMAC_DRBG_RESEED_INTERVAL;
+
#if defined(MBEDTLS_THREADING_C)
mbedtls_mutex_init( &ctx->mutex );
#endif
@@ -298,8 +300,6 @@
ctx->f_entropy = f_entropy;
ctx->p_entropy = p_entropy;
- ctx->reseed_interval = MBEDTLS_HMAC_DRBG_RESEED_INTERVAL;
-
if( ctx->entropy_len == 0 )
{
/*
@@ -444,7 +444,8 @@
}
/*
- * Free an HMAC_DRBG context
+ * This function resets HMAC_DRBG context to the state immediately
+ * after initial call of mbedtls_hmac_drbg_init().
*/
void mbedtls_hmac_drbg_free( mbedtls_hmac_drbg_context *ctx )
{
@@ -456,6 +457,10 @@
#endif
mbedtls_md_free( &ctx->md_ctx );
mbedtls_zeroize( ctx, sizeof( mbedtls_hmac_drbg_context ) );
+ ctx->reseed_interval = MBEDTLS_HMAC_DRBG_RESEED_INTERVAL;
+#if defined(MBEDTLS_THREADING_C)
+ mbedtls_mutex_init( &ctx->mutex );
+#endif
}
#if defined(MBEDTLS_FS_IO)
diff --git a/tests/suites/test_suite_ctr_drbg.function b/tests/suites/test_suite_ctr_drbg.function
index 134ba9c..13669e5 100644
--- a/tests/suites/test_suite_ctr_drbg.function
+++ b/tests/suites/test_suite_ctr_drbg.function
@@ -145,13 +145,16 @@
memset( out, 0, sizeof( out ) );
memset( add, 0, sizeof( add ) );
+ /* Set reseed interval before seed */
+ mbedtls_ctr_drbg_set_reseed_interval( &ctx, 2 * reps );
+
/* Init must use entropy */
last_idx = test_offset_idx;
TEST_ASSERT( mbedtls_ctr_drbg_seed( &ctx, mbedtls_test_entropy_func, entropy, NULL, 0 ) == 0 );
TEST_ASSERT( last_idx < test_offset_idx );
- /* By default, PR is off and reseed_interval is large,
- * so the next few calls should not use entropy */
+ /* By default, PR is off, and reseed interval was set to
+ * 2 * reps so the next few calls should not use entropy */
last_idx = test_offset_idx;
for( i = 0; i < reps; i++ )
{
@@ -167,15 +170,16 @@
TEST_ASSERT( out[sizeof( out ) - 2] == 0 );
TEST_ASSERT( out[sizeof( out ) - 1] == 0 );
- /* Set reseed_interval to the number of calls done,
- * so the next call should reseed */
- mbedtls_ctr_drbg_set_reseed_interval( &ctx, 2 * reps );
+ /* There have been 2 * reps calls to random. The next call should reseed */
TEST_ASSERT( mbedtls_ctr_drbg_random( &ctx, out, sizeof( out ) ) == 0 );
TEST_ASSERT( last_idx < test_offset_idx );
- /* The new few calls should not reseed */
+ /* Set reseed interval after seed */
+ mbedtls_ctr_drbg_set_reseed_interval( &ctx, 4 * reps + 1 );
+
+ /* The next few calls should not reseed */
last_idx = test_offset_idx;
- for( i = 0; i < reps / 2; i++ )
+ for( i = 0; i < (2 * reps); i++ )
{
TEST_ASSERT( mbedtls_ctr_drbg_random( &ctx, out, sizeof( out ) ) == 0 );
TEST_ASSERT( mbedtls_ctr_drbg_random_with_add( &ctx, out, sizeof( out ) ,
diff --git a/tests/suites/test_suite_hmac_drbg.function b/tests/suites/test_suite_hmac_drbg.function
index a413f5e..dd8ac9a 100644
--- a/tests/suites/test_suite_hmac_drbg.function
+++ b/tests/suites/test_suite_hmac_drbg.function
@@ -48,14 +48,17 @@
md_info = mbedtls_md_info_from_type( md_alg );
TEST_ASSERT( md_info != NULL );
+ /* Set reseed interval before seed */
+ mbedtls_hmac_drbg_set_reseed_interval( &ctx, 2 * reps );
+
/* Init must use entropy */
last_len = entropy.len;
TEST_ASSERT( mbedtls_hmac_drbg_seed( &ctx, md_info, mbedtls_test_entropy_func, &entropy,
NULL, 0 ) == 0 );
TEST_ASSERT( entropy.len < last_len );
- /* By default, PR is off and reseed_interval is large,
- * so the next few calls should not use entropy */
+ /* By default, PR is off, and reseed interval was set to
+ * 2 * reps so the next few calls should not use entropy */
last_len = entropy.len;
for( i = 0; i < reps; i++ )
{
@@ -71,15 +74,16 @@
TEST_ASSERT( out[sizeof( out ) - 2] == 0 );
TEST_ASSERT( out[sizeof( out ) - 1] == 0 );
- /* Set reseed_interval to the number of calls done,
- * so the next call should reseed */
- mbedtls_hmac_drbg_set_reseed_interval( &ctx, 2 * reps );
+ /* There have been 2 * reps calls to random. The next call should reseed */
TEST_ASSERT( mbedtls_hmac_drbg_random( &ctx, out, sizeof( out ) ) == 0 );
TEST_ASSERT( entropy.len < last_len );
+ /* Set reseed interval after seed */
+ mbedtls_hmac_drbg_set_reseed_interval( &ctx, 4 * reps + 1);
+
/* The new few calls should not reseed */
last_len = entropy.len;
- for( i = 0; i < reps / 2; i++ )
+ for( i = 0; i < (2 * reps); i++ )
{
TEST_ASSERT( mbedtls_hmac_drbg_random( &ctx, out, sizeof( out ) ) == 0 );
TEST_ASSERT( mbedtls_hmac_drbg_random_with_add( &ctx, out, sizeof( out ) ,
@@ -200,7 +204,7 @@
TEST_ASSERT( mbedtls_hmac_drbg_random_with_add( &ctx, my_output, out_len,
add2, add2_len ) == 0 );
- /* clear for second run */
+ /* Reset context for second run */
mbedtls_hmac_drbg_free( &ctx );
TEST_ASSERT( memcmp( my_output, output, out_len ) == 0 );
diff --git a/tests/suites/test_suite_mpi.data b/tests/suites/test_suite_mpi.data
index a6a6423..a07e9af 100644
--- a/tests/suites/test_suite_mpi.data
+++ b/tests/suites/test_suite_mpi.data
@@ -427,12 +427,6 @@
Test mbedtls_mpi_add_abs #1
mbedtls_mpi_add_abs:10:"-643808006803554439230129854961492699151386107534013432918073439524138264842370630061369715394739134090922937332590384720397133335969549256322620979036686633213903952966175107096769180017646161851573147596390153":10:"56125680981752282333498088313568935051383833838594899821664631784577337171193624243181360054669678410455329112434552942717084003541384594864129940145043086760031292483340068923506115878221189886491132772739661669044958531131327771":10:"56125680981752282334141896320372489490613963693556392520816017892111350604111697682705498319512049040516698827829292076808006940873974979584527073481012636016353913462376755556720019831187364993587901952757307830896531678727717924"
-Test mbedtls_mpi_add_abs #2 (add to first value)
-mpi_add_abs_add_first:10:"123123":10:"123123":10:"246246"
-
-Test mbedtls_mpi_add_abs #3 (add to second value)
-mpi_add_abs_add_second:10:"123123":10:"123123":10:"246246"
-
Regression mbedtls_mpi_add_abs (add small to very large MPI with carry rollover)
mbedtls_mpi_add_abs:16:"FFFFFFFFFFFFFFFFFFFFFFFFFFFFF8":16:"08":16:"1000000000000000000000000000000"
diff --git a/tests/suites/test_suite_mpi.function b/tests/suites/test_suite_mpi.function
index d023466..0d311e3 100644
--- a/tests/suites/test_suite_mpi.function
+++ b/tests/suites/test_suite_mpi.function
@@ -570,6 +570,15 @@
TEST_ASSERT( mbedtls_mpi_add_mpi( &Z, &X, &Y ) == 0 );
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &Z, &A ) == 0 );
+ /* result == first operand */
+ TEST_ASSERT( mbedtls_mpi_add_mpi( &X, &X, &Y ) == 0 );
+ TEST_ASSERT( mbedtls_mpi_cmp_mpi( &X, &A ) == 0 );
+ TEST_ASSERT( mbedtls_mpi_read_string( &X, radix_X, input_X ) == 0 );
+
+ /* result == second operand */
+ TEST_ASSERT( mbedtls_mpi_add_mpi( &Y, &X, &Y ) == 0 );
+ TEST_ASSERT( mbedtls_mpi_cmp_mpi( &Y, &A ) == 0 );
+
exit:
mbedtls_mpi_free( &X ); mbedtls_mpi_free( &Y ); mbedtls_mpi_free( &Z ); mbedtls_mpi_free( &A );
}
@@ -614,44 +623,17 @@
TEST_ASSERT( mbedtls_mpi_add_abs( &Z, &X, &Y ) == 0 );
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &Z, &A ) == 0 );
-exit:
- mbedtls_mpi_free( &X ); mbedtls_mpi_free( &Y ); mbedtls_mpi_free( &Z ); mbedtls_mpi_free( &A );
-}
-/* END_CASE */
-
-/* BEGIN_CASE */
-void mpi_add_abs_add_first( int radix_X, char *input_X, int radix_Y,
- char *input_Y, int radix_A, char *input_A )
-{
- mbedtls_mpi X, Y, A;
- mbedtls_mpi_init( &X ); mbedtls_mpi_init( &Y ); mbedtls_mpi_init( &A );
-
- TEST_ASSERT( mbedtls_mpi_read_string( &X, radix_X, input_X ) == 0 );
- TEST_ASSERT( mbedtls_mpi_read_string( &Y, radix_Y, input_Y ) == 0 );
- TEST_ASSERT( mbedtls_mpi_read_string( &A, radix_A, input_A ) == 0 );
+ /* result == first operand */
TEST_ASSERT( mbedtls_mpi_add_abs( &X, &X, &Y ) == 0 );
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &X, &A ) == 0 );
-
-exit:
- mbedtls_mpi_free( &X ); mbedtls_mpi_free( &Y ); mbedtls_mpi_free( &A );
-}
-/* END_CASE */
-
-/* BEGIN_CASE */
-void mpi_add_abs_add_second( int radix_X, char *input_X, int radix_Y,
- char *input_Y, int radix_A, char *input_A )
-{
- mbedtls_mpi X, Y, A;
- mbedtls_mpi_init( &X ); mbedtls_mpi_init( &Y ); mbedtls_mpi_init( &A );
-
TEST_ASSERT( mbedtls_mpi_read_string( &X, radix_X, input_X ) == 0 );
- TEST_ASSERT( mbedtls_mpi_read_string( &Y, radix_Y, input_Y ) == 0 );
- TEST_ASSERT( mbedtls_mpi_read_string( &A, radix_A, input_A ) == 0 );
+
+ /* result == second operand */
TEST_ASSERT( mbedtls_mpi_add_abs( &Y, &X, &Y ) == 0 );
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &Y, &A ) == 0 );
exit:
- mbedtls_mpi_free( &X ); mbedtls_mpi_free( &Y ); mbedtls_mpi_free( &A );
+ mbedtls_mpi_free( &X ); mbedtls_mpi_free( &Y ); mbedtls_mpi_free( &Z ); mbedtls_mpi_free( &A );
}
/* END_CASE */
@@ -685,6 +667,15 @@
TEST_ASSERT( mbedtls_mpi_sub_mpi( &Z, &X, &Y ) == 0 );
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &Z, &A ) == 0 );
+ /* result == first operand */
+ TEST_ASSERT( mbedtls_mpi_sub_mpi( &X, &X, &Y ) == 0 );
+ TEST_ASSERT( mbedtls_mpi_cmp_mpi( &X, &A ) == 0 );
+ TEST_ASSERT( mbedtls_mpi_read_string( &X, radix_X, input_X ) == 0 );
+
+ /* result == second operand */
+ TEST_ASSERT( mbedtls_mpi_sub_mpi( &Y, &X, &Y ) == 0 );
+ TEST_ASSERT( mbedtls_mpi_cmp_mpi( &Y, &A ) == 0 );
+
exit:
mbedtls_mpi_free( &X ); mbedtls_mpi_free( &Y ); mbedtls_mpi_free( &Z ); mbedtls_mpi_free( &A );
}
@@ -707,6 +698,17 @@
if( res == 0 )
TEST_ASSERT( mbedtls_mpi_cmp_mpi( &Z, &A ) == 0 );
+ /* result == first operand */
+ TEST_ASSERT( mbedtls_mpi_sub_abs( &X, &X, &Y ) == sub_result );
+ if( sub_result == 0 )
+ TEST_ASSERT( mbedtls_mpi_cmp_mpi( &X, &A ) == 0 );
+ TEST_ASSERT( mbedtls_mpi_read_string( &X, radix_X, input_X ) == 0 );
+
+ /* result == second operand */
+ TEST_ASSERT( mbedtls_mpi_sub_abs( &Y, &X, &Y ) == sub_result );
+ if( sub_result == 0 )
+ TEST_ASSERT( mbedtls_mpi_cmp_mpi( &Y, &A ) == 0 );
+
exit:
mbedtls_mpi_free( &X ); mbedtls_mpi_free( &Y ); mbedtls_mpi_free( &Z ); mbedtls_mpi_free( &A );
}