Make X.509 CRT cache reference counting unconditional

Previously, reference counting for the CRT frames and PK contexts
handed out by mbedtls_x509_crt_{frame|pk}_acquire() was implemented
only in case threading support was enabled, which leaves the door
open for a potential use-after-free should a single-threaded application
use nested calls to mbedtls_x509_crt_acquire().

Since Mbed TLS itself does not use such nested calls, it might be
preferred long-term to forbid nesting of acquire calls on the API
level, and hence get rid of reference counting in the interest of
code-size benefits. However, this can be considered as an optimization
of X.509 on demand parsing, and for now this commit introduces
reference counting unconditionally to have a safe version of
on demand parsing to build further optimizations upon.
This commit is contained in:
Hanno Becker 2019-06-28 10:34:23 +01:00
parent 94a94f6c33
commit a4bfaa8204
3 changed files with 11 additions and 12 deletions

View file

@ -831,19 +831,17 @@ static inline int mbedtls_x509_crt_frame_acquire( mbedtls_x509_crt const *crt,
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_lock( &crt->cache->frame_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif /* MBEDTLS_THREADING_C */
if( crt->cache->frame_readers == 0 )
#endif /* MBEDTLS_THREADING_C */
{
ret = mbedtls_x509_crt_cache_provide_frame( crt );
}
#if defined(MBEDTLS_THREADING_C)
if( crt->cache->frame_readers == MBEDTLS_X509_CACHE_FRAME_READERS_MAX )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
crt->cache->frame_readers++;
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_unlock( &crt->cache->frame_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif /* MBEDTLS_THREADING_C */
@ -864,12 +862,14 @@ static inline int mbedtls_x509_crt_frame_release( mbedtls_x509_crt const *crt )
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_lock( &crt->cache->frame_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif /* MBEDTLS_THREADING_C */
if( crt->cache->frame_readers == 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
crt->cache->frame_readers--;
#if defined(MBEDTLS_THREADING_C)
mbedtls_mutex_unlock( &crt->cache->frame_mutex );
#endif /* MBEDTLS_THREADING_C */
@ -919,19 +919,17 @@ static inline int mbedtls_x509_crt_pk_acquire( mbedtls_x509_crt const *crt,
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_lock( &crt->cache->pk_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif /* MBEDTLS_THREADING_C */
if( crt->cache->pk_readers == 0 )
#endif /* MBEDTLS_THREADING_C */
{
ret = mbedtls_x509_crt_cache_provide_pk( crt );
}
#if defined(MBEDTLS_THREADING_C)
if( crt->cache->pk_readers == MBEDTLS_X509_CACHE_PK_READERS_MAX )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
crt->cache->pk_readers++;
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_unlock( &crt->cache->pk_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif /* MBEDTLS_THREADING_C */
@ -952,12 +950,14 @@ static inline int mbedtls_x509_crt_pk_release( mbedtls_x509_crt const *crt )
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_lock( &crt->cache->pk_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif /* MBEDTLS_THREADING_C */
if( crt->cache->pk_readers == 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
crt->cache->pk_readers--;
#if defined(MBEDTLS_THREADING_C)
mbedtls_mutex_unlock( &crt->cache->pk_mutex );
#endif /* MBEDTLS_THREADING_C */

View file

@ -37,9 +37,9 @@ struct mbedtls_x509_crt_frame;
#define MBEDTLS_X509_CACHE_FRAME_READERS_MAX ((uint32_t) -1)
typedef struct mbedtls_x509_crt_cache
{
#if defined(MBEDTLS_THREADING_C)
uint32_t frame_readers;
uint32_t pk_readers;
#if defined(MBEDTLS_THREADING_C)
mbedtls_threading_mutex_t frame_mutex;
mbedtls_threading_mutex_t pk_mutex;
#endif

View file

@ -112,10 +112,9 @@ int mbedtls_x509_crt_flush_cache_pk( mbedtls_x509_crt const *crt )
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_lock( &crt->cache->pk_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif
/* Can only free the PK context if nobody is using it. */
if( crt->cache->pk_readers == 0 )
#endif
{
#if !defined(MBEDTLS_X509_ON_DEMAND_PARSING)
/* The cache holds a shallow copy of the PK context
@ -140,10 +139,10 @@ int mbedtls_x509_crt_flush_cache_frame( mbedtls_x509_crt const *crt )
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_lock( &crt->cache->frame_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif
/* Can only free the frame if nobody is using it. */
if( crt->cache->frame_readers == 0 )
#endif
{
mbedtls_free( crt->cache->frame );
crt->cache->frame = NULL;