Skip to content

Allow xxx_drbg_set_entropy_len before xxx_drbg_seed #299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 18, 2019
Merged

Allow xxx_drbg_set_entropy_len before xxx_drbg_seed #299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8f7921e
HMAC_DRBG: support set_entropy_len() before seed()
gilles-peskine-arm Oct 4, 2019
8bf5613
CTR_DRBG: Don't use functions before they're defined
gilles-peskine-arm Oct 2, 2019
50ed86b
CTR_DRBG: support set_entropy_len() before seed()
gilles-peskine-arm Oct 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions include/mbedtls/ctr_drbg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,8 @@ void mbedtls_ctr_drbg_init( mbedtls_ctr_drbg_context *ctx );
* with mbedtls_entropy_init() (which registers the platform's default
* entropy sources).
*
* \p f_entropy is always called with a buffer size equal to the entropy
* length. The entropy length is initially #MBEDTLS_CTR_DRBG_ENTROPY_LEN
* and this value is always used for the initial seeding. You can change
* the entropy length for subsequent seeding by calling
* mbedtls_ctr_drbg_set_entropy_len() after this function.
* The entropy length is #MBEDTLS_CTR_DRBG_ENTROPY_LEN by default.
* You can override it by calling mbedtls_ctr_drbg_set_entropy_len().
*
* You can provide a personalization string in addition to the
* entropy source, to make this instantiation as unique as possible.
Expand Down Expand Up @@ -255,6 +252,8 @@ void mbedtls_ctr_drbg_init( mbedtls_ctr_drbg_context *ctx );
* \param f_entropy The entropy callback, taking as arguments the
* \p p_entropy context, the buffer to fill, and the
* length of the buffer.
* \p f_entropy is always called with a buffer size
* equal to the entropy length.
* \param p_entropy The entropy context to pass to \p f_entropy.
* \param custom The personalization string.
* This can be \c NULL, in which case the personalization
Expand Down Expand Up @@ -298,7 +297,7 @@ void mbedtls_ctr_drbg_set_prediction_resistance( mbedtls_ctr_drbg_context *ctx,

/**
* \brief This function sets the amount of entropy grabbed on each
* subsequent reseed.
* seed or reseed.
*
* The default value is #MBEDTLS_CTR_DRBG_ENTROPY_LEN.
*
Expand Down Expand Up @@ -499,11 +498,6 @@ int mbedtls_ctr_drbg_self_test( int verbose );

#endif /* MBEDTLS_SELF_TEST */

/* Internal functions (do not call directly) */
int mbedtls_ctr_drbg_seed_entropy_len( mbedtls_ctr_drbg_context *,
int (*)(void *, unsigned char *, size_t), void *,
const unsigned char *, size_t, size_t );

#ifdef __cplusplus
}
#endif
Expand Down
8 changes: 3 additions & 5 deletions include/mbedtls/hmac_drbg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,9 @@ void mbedtls_hmac_drbg_init( mbedtls_hmac_drbg_context *ctx );
* entropy length is set with
* mbedtls_hmac_drbg_set_entropy_len() afterwards.
*
* \note The entropy length for the initial seeding is
* the security strength (converted from bits to bytes).
* You can set a different entropy length for subsequent
* seeding by calling mbedtls_hmac_drbg_set_entropy_len()
* after this function.
* \note The default entropy length is the security strength
* (converted from bits to bytes). You can override
* it by calling mbedtls_hmac_drbg_set_entropy_len().
*
* \note During the initial seeding, this function calls
* the entropy source to obtain a nonce
Expand Down
126 changes: 58 additions & 68 deletions library/ctr_drbg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,70 +62,6 @@ void mbedtls_ctr_drbg_init( mbedtls_ctr_drbg_context *ctx )
#endif
}

/*
* Non-public function wrapped by mbedtls_ctr_drbg_seed(). Necessary to allow
* NIST tests to succeed (which require known length fixed entropy)
*/
/* CTR_DRBG_Instantiate with derivation function (SP 800-90A §10.2.1.3.2)
* mbedtls_ctr_drbg_seed_entropy_len(ctx, f_entropy, p_entropy,
* custom, len, entropy_len)
* implements
* CTR_DRBG_Instantiate(entropy_input, nonce, personalization_string,
* security_strength) -> initial_working_state
* with inputs
* custom[:len] = nonce || personalization_string
* where entropy_input comes from f_entropy for entropy_len bytes
* and with outputs
* ctx = initial_working_state
*/
int mbedtls_ctr_drbg_seed_entropy_len(
mbedtls_ctr_drbg_context *ctx,
int (*f_entropy)(void *, unsigned char *, size_t),
void *p_entropy,
const unsigned char *custom,
size_t len,
size_t entropy_len )
{
int ret;
unsigned char key[MBEDTLS_CTR_DRBG_KEYSIZE];

memset( key, 0, MBEDTLS_CTR_DRBG_KEYSIZE );

mbedtls_aes_init( &ctx->aes_ctx );

ctx->f_entropy = f_entropy;
ctx->p_entropy = p_entropy;

ctx->entropy_len = entropy_len;
ctx->reseed_interval = MBEDTLS_CTR_DRBG_RESEED_INTERVAL;

/*
* Initialize with an empty key
*/
if( ( ret = mbedtls_aes_setkey_enc( &ctx->aes_ctx, key,
MBEDTLS_CTR_DRBG_KEYBITS ) ) != 0 )
{
return( ret );
}

if( ( ret = mbedtls_ctr_drbg_reseed( ctx, custom, len ) ) != 0 )
{
return( ret );
}
return( 0 );
}

int mbedtls_ctr_drbg_seed( mbedtls_ctr_drbg_context *ctx,
int (*f_entropy)(void *, unsigned char *, size_t),
void *p_entropy,
const unsigned char *custom,
size_t len )
{
return( mbedtls_ctr_drbg_seed_entropy_len( ctx, f_entropy, p_entropy,
custom, len,
MBEDTLS_CTR_DRBG_ENTROPY_LEN ) );
}

void mbedtls_ctr_drbg_free( mbedtls_ctr_drbg_context *ctx )
{
if( ctx == NULL )
Expand Down Expand Up @@ -445,6 +381,54 @@ int mbedtls_ctr_drbg_reseed( mbedtls_ctr_drbg_context *ctx,
return( ret );
}

/* CTR_DRBG_Instantiate with derivation function (SP 800-90A §10.2.1.3.2)
* mbedtls_ctr_drbg_seed_entropy_len(ctx, f_entropy, p_entropy,
* custom, len, entropy_len)
* implements
* CTR_DRBG_Instantiate(entropy_input, nonce, personalization_string,
* security_strength) -> initial_working_state
* with inputs
* custom[:len] = nonce || personalization_string
* where entropy_input comes from f_entropy for entropy_len bytes
* and with outputs
* ctx = initial_working_state
*/
int mbedtls_ctr_drbg_seed( mbedtls_ctr_drbg_context *ctx,
int (*f_entropy)(void *, unsigned char *, size_t),
void *p_entropy,
const unsigned char *custom,
size_t len )
{
int ret;
unsigned char key[MBEDTLS_CTR_DRBG_KEYSIZE];

memset( key, 0, MBEDTLS_CTR_DRBG_KEYSIZE );

mbedtls_aes_init( &ctx->aes_ctx );

ctx->f_entropy = f_entropy;
ctx->p_entropy = p_entropy;

if( ctx->entropy_len == 0 )
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gilles-peskine-arm, is there any reason you didin't use ctr_drbg_set_entropy_len() here? Maybe you wanted to keep some level of method independency.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of a strong reason one way or the other. Semantically, I want to set the entropy_len field to a certain value. mbedtls_ctr_drbg_set_entropy_len() does that, but so does an assignment. An assignment is simpler than calling a function. Arguably, calling a function is better in case the function changes. But changes to do what? If it changes to add some validation, this validation might not be what is needed at this point inside mbedtls_ctr_drbg_seed. And if the meaning of the field changes, there'll be other changes needed in the code, so who knows what would need to change in mbedtls_ctr_drbg_seed. In terms of code size, the assignment is a couple of bytes smaller than a function call. So I have a very slight preference for the direct assignment. But only very slight. I don't remember thinking about it explicitly at the time.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the perspective.

ctx->entropy_len = MBEDTLS_CTR_DRBG_ENTROPY_LEN;
ctx->reseed_interval = MBEDTLS_CTR_DRBG_RESEED_INTERVAL;

/*
* Initialize with an empty key
*/
if( ( ret = mbedtls_aes_setkey_enc( &ctx->aes_ctx, key,
MBEDTLS_CTR_DRBG_KEYBITS ) ) != 0 )
{
return( ret );
}

if( ( ret = mbedtls_ctr_drbg_reseed( ctx, custom, len ) ) != 0 )
{
return( ret );
}
return( 0 );
}

/* CTR_DRBG_Generate with derivation function (SP 800-90A §10.2.1.5.2)
* mbedtls_ctr_drbg_random_with_add(ctx, output, output_len, additional, add_len)
* implements
Expand Down Expand Up @@ -708,8 +692,11 @@ int mbedtls_ctr_drbg_self_test( int verbose )
mbedtls_printf( " CTR_DRBG (PR = TRUE) : " );

test_offset = 0;
CHK( mbedtls_ctr_drbg_seed_entropy_len( &ctx, ctr_drbg_self_test_entropy,
(void *) entropy_source_pr, nonce_pers_pr, 16, 32 ) );
mbedtls_ctr_drbg_set_entropy_len( &ctx, 32 );
CHK( mbedtls_ctr_drbg_seed( &ctx,
ctr_drbg_self_test_entropy,
(void *) entropy_source_pr,
nonce_pers_pr, 16 ) );
mbedtls_ctr_drbg_set_prediction_resistance( &ctx, MBEDTLS_CTR_DRBG_PR_ON );
CHK( mbedtls_ctr_drbg_random( &ctx, buf, MBEDTLS_CTR_DRBG_BLOCKSIZE ) );
CHK( mbedtls_ctr_drbg_random( &ctx, buf, MBEDTLS_CTR_DRBG_BLOCKSIZE ) );
Expand All @@ -729,8 +716,11 @@ int mbedtls_ctr_drbg_self_test( int verbose )
mbedtls_ctr_drbg_init( &ctx );

test_offset = 0;
CHK( mbedtls_ctr_drbg_seed_entropy_len( &ctx, ctr_drbg_self_test_entropy,
(void *) entropy_source_nopr, nonce_pers_nopr, 16, 32 ) );
mbedtls_ctr_drbg_set_entropy_len( &ctx, 32 );
CHK( mbedtls_ctr_drbg_seed( &ctx,
ctr_drbg_self_test_entropy,
(void *) entropy_source_nopr,
nonce_pers_nopr, 16 ) );
CHK( mbedtls_ctr_drbg_random( &ctx, buf, 16 ) );
CHK( mbedtls_ctr_drbg_reseed( &ctx, NULL, 0 ) );
CHK( mbedtls_ctr_drbg_random( &ctx, buf, 16 ) );
Expand Down
25 changes: 14 additions & 11 deletions library/hmac_drbg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,16 +273,19 @@ int mbedtls_hmac_drbg_seed( mbedtls_hmac_drbg_context *ctx,

ctx->reseed_interval = MBEDTLS_HMAC_DRBG_RESEED_INTERVAL;

/*
* See SP800-57 5.6.1 (p. 65-66) for the security strength provided by
* each hash function, then according to SP800-90A rev1 10.1 table 2,
* min_entropy_len (in bits) is security_strength.
*
* (This also matches the sizes used in the NIST test vectors.)
*/
ctx->entropy_len = md_size <= 20 ? 16 : /* 160-bits hash -> 128 bits */
md_size <= 28 ? 24 : /* 224-bits hash -> 192 bits */
32; /* better (256+) -> 256 bits */
if( ctx->entropy_len == 0 )
{
/*
* See SP800-57 5.6.1 (p. 65-66) for the security strength provided by
* each hash function, then according to SP800-90A rev1 10.1 table 2,
* min_entropy_len (in bits) is security_strength.
*
* (This also matches the sizes used in the NIST test vectors.)
*/
ctx->entropy_len = md_size <= 20 ? 16 : /* 160-bits hash -> 128 bits */
md_size <= 28 ? 24 : /* 224-bits hash -> 192 bits */
32; /* better (256+) -> 256 bits */
}

if( ( ret = hmac_drbg_reseed_core( ctx, custom, len,
1 /* add nonce */ ) ) != 0 )
Expand All @@ -303,7 +306,7 @@ void mbedtls_hmac_drbg_set_prediction_resistance( mbedtls_hmac_drbg_context *ctx
}

/*
* Set entropy length grabbed for reseeds
* Set entropy length grabbed for seeding
*/
void mbedtls_hmac_drbg_set_entropy_len( mbedtls_hmac_drbg_context *ctx, size_t len )
{
Expand Down
6 changes: 3 additions & 3 deletions tests/suites/test_suite_ctr_drbg.function
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ static void ctr_drbg_validate_internal( int reseed_mode, data_t * nonce,

/* CTR_DRBG_Instantiate(entropy[:entropy->len], nonce, perso, <ignored>)
* where nonce||perso = nonce[nonce->len] */
TEST_ASSERT( mbedtls_ctr_drbg_seed_entropy_len(
mbedtls_ctr_drbg_set_entropy_len( &ctx, entropy_chunk_len );
TEST_ASSERT( mbedtls_ctr_drbg_seed(
&ctx,
mbedtls_test_entropy_func, entropy->x,
nonce->x, nonce->len,
entropy_chunk_len ) == 0 );
nonce->x, nonce->len ) == 0 );
if( reseed_mode == RESEED_ALWAYS )
mbedtls_ctr_drbg_set_prediction_resistance(
&ctx,
Expand Down