Allow multiple concurrent readers for X.509 CRT frame and PK context

Previously, only one thread could access the parsing cache of an X.509 CRT
at a time. Firstly, this leads to significant performance penalties on
systems running many concurrent threads which share CRT structures --
for example, server threads sharing an SSL configuration containing the
server CRT. Secondly, the locking should be logically unnecessary, because
the threads are supposed to access the CRT frame and PK in a read-only,
or at least thread-safe manner.

This commit modifies the X.509 CRT cache implementation by allowing an
arbitrary number of concurrent readers, locking only the path of setting
up and clearing the cache.
This commit is contained in:
Hanno Becker 2019-05-28 16:11:43 +01:00
parent d687ef0a91
commit 2ba9fbdfe9
3 changed files with 81 additions and 44 deletions

View file

@ -817,26 +817,31 @@ int mbedtls_x509_crt_flush_cache( mbedtls_x509_crt const *crt );
* \return A negative error code on failure.
*/
static inline int mbedtls_x509_crt_frame_acquire( mbedtls_x509_crt const *crt,
mbedtls_x509_crt_frame const **frame_ptr )
mbedtls_x509_crt_frame const **dst )
{
int ret;
int ret = 0;
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_lock( &crt->cache->frame_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif
ret = mbedtls_x509_crt_cache_provide_frame( crt );
if( ret != 0 )
if( crt->cache->frame_readers == 0 )
#endif /* MBEDTLS_THREADING_C */
{
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_unlock( &crt->cache->frame_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif
return( ret );
ret = mbedtls_x509_crt_cache_provide_frame( crt );
}
*frame_ptr = crt->cache->frame;
return( 0 );
#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( mbedtls_mutex_unlock( &crt->cache->frame_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif /* MBEDTLS_THREADING_C */
*dst = crt->cache->frame;
return( ret );
}
/**
@ -846,17 +851,24 @@ static inline int mbedtls_x509_crt_frame_acquire( mbedtls_x509_crt const *crt,
* \param crt The certificate for which a certificate frame has
* previously been acquired.
*/
static inline void mbedtls_x509_crt_frame_release( mbedtls_x509_crt const *crt )
static inline int mbedtls_x509_crt_frame_release( mbedtls_x509_crt const *crt )
{
((void) crt);
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_lock( &crt->cache->frame_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
if( crt->cache->frame_readers == 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
crt->cache->frame_readers--;
mbedtls_mutex_unlock( &crt->cache->frame_mutex );
#endif
#endif /* MBEDTLS_THREADING_C */
#if defined(MBEDTLS_X509_ALWAYS_FLUSH)
(void) mbedtls_x509_crt_flush_cache_frame( crt );
(void) mbedtls_x509_crt_flush_cache_frame( crt );
#endif /* MBEDTLS_X509_ALWAYS_FLUSH */
return( 0 );
}
/**
@ -887,26 +899,31 @@ static inline void mbedtls_x509_crt_frame_release( mbedtls_x509_crt const *crt )
* \return A negative error code on failure.
*/
static inline int mbedtls_x509_crt_pk_acquire( mbedtls_x509_crt const *crt,
mbedtls_pk_context **pk_ptr )
mbedtls_pk_context **dst )
{
int ret;
int ret = 0;
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_lock( &crt->cache->pk_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif
ret = mbedtls_x509_crt_cache_provide_pk( crt );
if( ret != 0 )
if( crt->cache->pk_readers == 0 )
#endif /* MBEDTLS_THREADING_C */
{
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_unlock( &crt->cache->pk_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif
return( ret );
ret = mbedtls_x509_crt_cache_provide_pk( crt );
}
*pk_ptr = crt->cache->pk;
return( 0 );
#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( mbedtls_mutex_unlock( &crt->cache->pk_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif /* MBEDTLS_THREADING_C */
*dst = crt->cache->pk;
return( ret );
}
/**
@ -916,17 +933,25 @@ static inline int mbedtls_x509_crt_pk_acquire( mbedtls_x509_crt const *crt,
* \param crt The certificate for which a certificate frame has
* previously been acquired.
*/
static inline void mbedtls_x509_crt_pk_release( mbedtls_x509_crt const *crt )
static inline int mbedtls_x509_crt_pk_release( mbedtls_x509_crt const *crt )
{
((void) crt);
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_lock( &crt->cache->pk_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
if( crt->cache->pk_readers == 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
crt->cache->pk_readers--;
mbedtls_mutex_unlock( &crt->cache->pk_mutex );
#endif
#endif /* MBEDTLS_THREADING_C */
#if defined(MBEDTLS_X509_ALWAYS_FLUSH)
(void) mbedtls_x509_crt_flush_cache_pk( crt );
(void) mbedtls_x509_crt_flush_cache_pk( crt );
#endif /* MBEDTLS_X509_ALWAYS_FLUSH */
return( 0 );
}
#endif /* MBEDTLS_X509_CRT_PARSE_C */

View file

@ -33,9 +33,13 @@
struct mbedtls_x509_crt;
struct mbedtls_pk_context;
struct mbedtls_x509_crt_frame;
#define MBEDTLS_X509_CACHE_PK_READERS_MAX ((uint32_t) -1)
#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;
mbedtls_threading_mutex_t frame_mutex;
mbedtls_threading_mutex_t pk_mutex;
#endif

View file

@ -112,17 +112,21 @@ 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
* in the legacy struct, so don't free PK context. */
mbedtls_free( crt->cache->pk );
/* The cache holds a shallow copy of the PK context
* in the legacy struct, so don't free PK context. */
mbedtls_free( crt->cache->pk );
#else
mbedtls_pk_free( crt->cache->pk );
mbedtls_free( crt->cache->pk );
mbedtls_pk_free( crt->cache->pk );
mbedtls_free( crt->cache->pk );
#endif /* MBEDTLS_X509_ON_DEMAND_PARSING */
crt->cache->pk = NULL;
crt->cache->pk = NULL;
}
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_unlock( &crt->cache->pk_mutex ) != 0 )
@ -136,10 +140,14 @@ 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
mbedtls_free( crt->cache->frame );
crt->cache->frame = NULL;
/* 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;
}
#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_unlock( &crt->cache->frame_mutex ) != 0 )