Reinitialize PK ctx in mbedtls_pk_parse_key before reuse are free

Context: This commit makes a change to mbedtls_pk_parse_key() which
is responsible for parsing of private keys. The function doesn't know
the key format in advance (PEM vs. DER, encrypted vs. unencrypted) and
tries them one by one, resetting the PK context in between.

Issue: The previous code resets the PK context through a call to
mbedtls_pk_free() along, lacking the accompanying mbedtls_pk_init()
call. Practically, this is not an issue because functionally
mbedtls_pk_free() + mbedtls_pk_init() is equivalent to mbedtls_pk_free()
with the current implementation of these functions, but strictly
speaking it's nonetheless a violation of the API semantics according
to which xxx_free() functions leave a context in uninitialized state.
(yet not entirely random, because xxx_free() functions must be idempotent,
so they cannot just fill the context they operate on with garbage).

Change: The commit adds calls to mbedtls_pk_init() after those calls
to mbedtls_pk_free() within mbedtls_pk_parse_key() after which the
PK context might still be used.
This commit is contained in:
Hanno Becker 2018-10-10 11:23:33 +01:00
parent 0592ea772a
commit 780f0a4cc1

View file

@ -1295,6 +1295,7 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk,
return( 0 );
mbedtls_pk_free( pk );
mbedtls_pk_init( pk );
if( ret == MBEDTLS_ERR_PK_PASSWORD_MISMATCH )
{
@ -1306,39 +1307,42 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk,
return( 0 );
mbedtls_pk_free( pk );
mbedtls_pk_init( pk );
#if defined(MBEDTLS_RSA_C)
pk_info = mbedtls_pk_info_from_type( MBEDTLS_PK_RSA );
if( ( ret = mbedtls_pk_setup( pk, pk_info ) ) != 0 ||
( ret = pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *pk ),
key, keylen ) ) != 0 )
{
mbedtls_pk_free( pk );
}
else
if( mbedtls_pk_setup( pk, pk_info ) == 0 &&
pk_parse_key_pkcs1_der( mbedtls_pk_rsa( *pk ), key, keylen ) == 0 )
{
return( 0 );
}
mbedtls_pk_free( pk );
mbedtls_pk_init( pk );
#endif /* MBEDTLS_RSA_C */
#if defined(MBEDTLS_ECP_C)
pk_info = mbedtls_pk_info_from_type( MBEDTLS_PK_ECKEY );
if( ( ret = mbedtls_pk_setup( pk, pk_info ) ) != 0 ||
( ret = pk_parse_key_sec1_der( mbedtls_pk_ec( *pk ),
key, keylen ) ) != 0 )
{
mbedtls_pk_free( pk );
}
else
if( mbedtls_pk_setup( pk, pk_info ) == 0 &&
pk_parse_key_sec1_der( mbedtls_pk_ec( *pk ),
key, keylen ) == 0 )
{
return( 0 );
}
mbedtls_pk_free( pk );
#endif /* MBEDTLS_ECP_C */
/* If MBEDTLS_RSA_C is defined but MBEDTLS_ECP_C isn't,
* it is ok to leave the PK context initialized but not
* freed: It is the caller's responsibility to call pk_init()
* before calling this function, and to call pk_free()
* when it fails. If MBEDTLS_ECP_C is defined but MBEDTLS_RSA_C
* isn't, this leads to mbedtls_pk_free() being called
* twice, once here and once by the caller, but this is
* also ok and in line with the mbedtls_pk_free() calls
* on failed PEM parsing attempts. */
return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT );
}