From 57d96cddf5d49039adfd67e909effe13d6f8ded9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 19 Sep 2019 10:45:14 +0200 Subject: [PATCH] Move NULL check inside accessor function This achieves two related goals: 1. Those members are now only accessed via the accessor function (except in code paths that we don't care about: those guarded by MBEDTLS_PK_RSA_ALT_SUPPORT or MBEDTLS_ECP_RESTARTABLE) 2. When we turn on compile-time dispatch, we don't obviously don't want to keep a runtime NULL check. For debug this requires changing the signature or the accessor function to return int; this is done without changing the signature of the accessed function. --- include/mbedtls/pk_internal.h | 29 ++++++++++++------------ library/pk.c | 42 +++++++++++++++++------------------ 2 files changed, 36 insertions(+), 35 deletions(-) diff --git a/include/mbedtls/pk_internal.h b/include/mbedtls/pk_internal.h index d3b501dd9..0fda01d76 100644 --- a/include/mbedtls/pk_internal.h +++ b/include/mbedtls/pk_internal.h @@ -41,18 +41,19 @@ struct mbedtls_pk_info_t /** Type name */ const char *name; - /** Get key size in bits */ + /** Get key size in bits (must be valid)*/ size_t (*get_bitlen)( const void * ); - /** Tell if the context implements this type (e.g. ECKEY can do ECDSA) */ + /** Tell if the context implements this type (e.g. ECKEY can do ECDSA) + * (must be valid) */ int (*can_do)( mbedtls_pk_type_t type ); - /** Verify signature */ + /** Verify signature (may be NULL) */ int (*verify_func)( void *ctx, mbedtls_md_type_t md_alg, const unsigned char *hash, size_t hash_len, const unsigned char *sig, size_t sig_len ); - /** Make signature */ + /** Make signature (may be NULL)*/ int (*sign_func)( void *ctx, mbedtls_md_type_t md_alg, const unsigned char *hash, size_t hash_len, unsigned char *sig, size_t *sig_len, @@ -60,13 +61,13 @@ struct mbedtls_pk_info_t void *p_rng ); #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) - /** Verify signature (restartable) */ + /** Verify signature (restartable) (may be NULL) */ int (*verify_rs_func)( void *ctx, mbedtls_md_type_t md_alg, const unsigned char *hash, size_t hash_len, const unsigned char *sig, size_t sig_len, void *rs_ctx ); - /** Make signature (restartable) */ + /** Make signature (restartable) (may be NULL) */ int (*sign_rs_func)( void *ctx, mbedtls_md_type_t md_alg, const unsigned char *hash, size_t hash_len, unsigned char *sig, size_t *sig_len, @@ -74,36 +75,36 @@ struct mbedtls_pk_info_t void *p_rng, void *rs_ctx ); #endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ - /** Decrypt message */ + /** Decrypt message (may be NULL) */ int (*decrypt_func)( void *ctx, const unsigned char *input, size_t ilen, unsigned char *output, size_t *olen, size_t osize, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ); - /** Encrypt message */ + /** Encrypt message (may be NULL ) */ int (*encrypt_func)( void *ctx, const unsigned char *input, size_t ilen, unsigned char *output, size_t *olen, size_t osize, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ); - /** Check public-private key pair */ + /** Check public-private key pair (may be NULL) */ int (*check_pair_func)( const void *pub, const void *prv ); - /** Allocate a new context */ + /** Allocate a new context (must be valid) */ void * (*ctx_alloc_func)( void ); - /** Free the given context */ + /** Free the given context (must be valid) */ void (*ctx_free_func)( void *ctx ); #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) - /** Allocate the restart context */ + /** Allocate the restart context (may be NULL)*/ void * (*rs_alloc_func)( void ); - /** Free the restart context */ + /** Free the restart context (may be NULL) */ void (*rs_free_func)( void *rs_ctx ); #endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ - /** Interface with the debug module */ + /** Interface with the debug module (may be NULL) */ void (*debug_func)( const void *ctx, mbedtls_pk_debug_item *items ); }; diff --git a/library/pk.c b/library/pk.c index 2519d8765..9ded7e06d 100644 --- a/library/pk.c +++ b/library/pk.c @@ -92,6 +92,9 @@ MBEDTLS_ALWAYS_INLINE static inline int pk_info_verify_func( const unsigned char *hash, size_t hash_len, const unsigned char *sig, size_t sig_len ) { + if( info->verify_func == NULL ) + return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); + return( info->verify_func( ctx, md_alg, hash, hash_len, sig, sig_len ) ); } @@ -102,6 +105,9 @@ MBEDTLS_ALWAYS_INLINE static inline int pk_info_sign_func( int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { + if( info->sign_func == NULL ) + return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); + return( info->sign_func( ctx, md_alg, hash, hash_len, sig, sig_len, f_rng, p_rng ) ); } @@ -113,6 +119,9 @@ MBEDTLS_ALWAYS_INLINE static inline int pk_info_decrypt_func( int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { + if( info->decrypt_func == NULL ) + return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); + return( info->decrypt_func( ctx, input, ilen, output, olen, osize, f_rng, p_rng ) ); } @@ -124,6 +133,9 @@ MBEDTLS_ALWAYS_INLINE static inline int pk_info_encrypt_func( int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { + if( info->encrypt_func == NULL ) + return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); + return( info->encrypt_func( ctx, input, ilen, output, olen, osize, f_rng, p_rng ) ); } @@ -131,6 +143,9 @@ MBEDTLS_ALWAYS_INLINE static inline int pk_info_encrypt_func( MBEDTLS_ALWAYS_INLINE static inline int pk_info_check_pair_func( const mbedtls_pk_info_t *info, const void *pub, const void *prv ) { + if( info->check_pair_func == NULL ) + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + return( info->check_pair_func( pub, prv ) ); } @@ -146,11 +161,15 @@ MBEDTLS_ALWAYS_INLINE static inline void pk_info_ctx_free_func( info->ctx_free_func( ctx ); } -MBEDTLS_ALWAYS_INLINE static inline void pk_info_debug_func( +MBEDTLS_ALWAYS_INLINE static inline int pk_info_debug_func( const mbedtls_pk_info_t *info, const void *ctx, mbedtls_pk_debug_item *items ) { + if( info->debug_func == NULL ) + return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); + info->debug_func( ctx, items ); + return( 0 ); } /* @@ -388,9 +407,6 @@ int mbedtls_pk_verify_restartable( mbedtls_pk_context *ctx, (void) rs_ctx; #endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ - if( ctx->pk_info->verify_func == NULL ) - return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); - return( pk_info_verify_func( ctx->pk_info, ctx->pk_ctx, md_alg, hash, hash_len, sig, sig_len ) ); } @@ -511,9 +527,6 @@ int mbedtls_pk_sign_restartable( mbedtls_pk_context *ctx, (void) rs_ctx; #endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ - if( ctx->pk_info->sign_func == NULL ) - return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); - return( pk_info_sign_func( ctx->pk_info, ctx->pk_ctx, md_alg, hash, hash_len, sig, sig_len, f_rng, p_rng ) ); } @@ -546,9 +559,6 @@ int mbedtls_pk_decrypt( mbedtls_pk_context *ctx, if( ctx->pk_info == NULL ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); - if( ctx->pk_info->decrypt_func == NULL ) - return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); - return( pk_info_decrypt_func( ctx->pk_info, ctx->pk_ctx, input, ilen, output, olen, osize, f_rng, p_rng ) ); } @@ -569,9 +579,6 @@ int mbedtls_pk_encrypt( mbedtls_pk_context *ctx, if( ctx->pk_info == NULL ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); - if( ctx->pk_info->encrypt_func == NULL ) - return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); - return( pk_info_encrypt_func( ctx->pk_info, ctx->pk_ctx, input, ilen, output, olen, osize, f_rng, p_rng ) ); } @@ -600,9 +607,6 @@ int mbedtls_pk_check_pair( const mbedtls_pk_context *pub, const mbedtls_pk_conte return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); } - if( prv->pk_info->check_pair_func == NULL ) - return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); - return( pk_info_check_pair_func( prv->pk_info, pub->pk_ctx, prv->pk_ctx ) ); } @@ -628,11 +632,7 @@ int mbedtls_pk_debug( const mbedtls_pk_context *ctx, mbedtls_pk_debug_item *item if( ctx->pk_info == NULL ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); - if( ctx->pk_info->debug_func == NULL ) - return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); - - pk_info_debug_func( ctx->pk_info, ctx->pk_ctx, items ); - return( 0 ); + return( pk_info_debug_func( ctx->pk_info, ctx->pk_ctx, items ) ); } /*