From 488e7b2a8cfc326bc79cee4e32a99dc2e91d8f1e Mon Sep 17 00:00:00 2001 From: Chen Gang G Date: Wed, 26 Dec 2018 20:56:17 +0800 Subject: [PATCH] hv: fix violations in hkdf.c and crypto_api.c for crypto lib -remove goto -remove multiple return -Modify assignment operator in boolean expression -Modify/fix code style violations -fix attempt to change parameters passed by value -fix value need U suffix -fix use of mixed arithmetic -fix assigment in expression -other fix Tracked-On: #861 Signed-off-by: Chen Gang G Reviewed-by: Bing Zhu Acked-by: Eddie Dong --- hypervisor/lib/crypto/crypto_api.c | 50 +++---- hypervisor/lib/crypto/mbedtls/hkdf.c | 209 +++++++++++++-------------- 2 files changed, 117 insertions(+), 142 deletions(-) diff --git a/hypervisor/lib/crypto/crypto_api.c b/hypervisor/lib/crypto/crypto_api.c index 0053ce40d..b11318161 100644 --- a/hypervisor/lib/crypto/crypto_api.c +++ b/hypervisor/lib/crypto/crypto_api.c @@ -13,50 +13,38 @@ int32_t hkdf_sha256(uint8_t *out_key, size_t out_len, const uint8_t *salt, size_t salt_len, const uint8_t *info, size_t info_len) { + int32_t ret = 0; const mbedtls_md_info_t *md; /* salt and info can be NULL, others can't */ - if (!out_key || !secret) { - return 0; + if ((out_key != NULL) && (secret != NULL)) { + md = mbedtls_md_info_from_type(MBEDTLS_MD_SHA256); + if (md != NULL) { + if (mbedtls_hkdf(md, salt, salt_len, secret, secret_len, + info, info_len, out_key, out_len) == 0) { + ret = 1; + } + } } - md = mbedtls_md_info_from_type(MBEDTLS_MD_SHA256); - if (md == NULL) { - return 0; - } - - if (mbedtls_hkdf(md, - salt, salt_len, - secret, secret_len, - info, info_len, - out_key, out_len) != 0) { - return 0; - } - - return 1; + return ret; } int32_t hmac_sha256(uint8_t *out_key, const uint8_t *secret, size_t secret_len, const uint8_t *salt, size_t salt_len) { + int32_t ret = 0; const mbedtls_md_info_t *md; - if (!out_key || !secret || !salt) { - return 0; + if ((out_key != NULL) && (secret != NULL) && (salt != NULL)) { + md = mbedtls_md_info_from_type(MBEDTLS_MD_SHA256); + if (md != NULL) { + if (mbedtls_md_hmac(md, secret, secret_len, salt, salt_len, out_key) == 0) { + ret = 1; + } + } } - md = mbedtls_md_info_from_type(MBEDTLS_MD_SHA256); - if (md == NULL) { - return 0; - } - - if (mbedtls_md_hmac(md, - secret, secret_len, - salt, salt_len, - out_key) != 0) { - return 0; - } - - return 1; + return ret; } diff --git a/hypervisor/lib/crypto/mbedtls/hkdf.c b/hypervisor/lib/crypto/mbedtls/hkdf.c index 9c2b1c9a9..1af3c9077 100644 --- a/hypervisor/lib/crypto/mbedtls/hkdf.c +++ b/hypervisor/lib/crypto/mbedtls/hkdf.c @@ -21,161 +21,148 @@ #include "hkdf.h" -int32_t mbedtls_hkdf( const mbedtls_md_info_t *md, const uint8_t *salt, +int32_t mbedtls_hkdf(const mbedtls_md_info_t *md, const uint8_t *salt, size_t salt_len, const uint8_t *ikm, size_t ikm_len, const uint8_t *info, size_t info_len, - uint8_t *okm, size_t okm_len ) + uint8_t *okm, size_t okm_len) { int32_t ret; uint8_t prk[MBEDTLS_MD_MAX_SIZE]; - ret = mbedtls_hkdf_extract( md, salt, salt_len, ikm, ikm_len, prk ); + ret = mbedtls_hkdf_extract(md, salt, salt_len, ikm, ikm_len, prk); - if( ret == 0 ) - { - ret = mbedtls_hkdf_expand( md, prk, mbedtls_md_get_size( md ), - info, info_len, okm, okm_len ); + if (ret == 0) { + ret = mbedtls_hkdf_expand(md, prk, (size_t)mbedtls_md_get_size(md), + info, info_len, okm, okm_len); } - mbedtls_platform_zeroize( prk, sizeof( prk ) ); + (void)mbedtls_platform_zeroize(prk, sizeof(prk)); - return( ret ); + return ret; } -int32_t mbedtls_hkdf_extract( const mbedtls_md_info_t *md, +int32_t mbedtls_hkdf_extract(const mbedtls_md_info_t *md, const uint8_t *salt, size_t salt_len, const uint8_t *ikm, size_t ikm_len, - uint8_t *prk ) + uint8_t *prk) { - uint8_t null_salt[MBEDTLS_MD_MAX_SIZE] = { '\0' }; + int32_t ret = 0; + size_t tmp_salt_len = salt_len; + const uint8_t *tmp_salt = salt; + uint8_t null_salt[MBEDTLS_MD_MAX_SIZE] = { 0U }; - if( salt == NULL ) - { + if (tmp_salt == NULL) { size_t hash_len; - if( salt_len != 0 ) - { - return MBEDTLS_ERR_HKDF_BAD_INPUT_DATA; + if (tmp_salt_len != 0U) { + ret = MBEDTLS_ERR_HKDF_BAD_INPUT_DATA; + } else { + + hash_len = mbedtls_md_get_size(md); + + if (hash_len == 0U) { + ret = MBEDTLS_ERR_HKDF_BAD_INPUT_DATA; + } else { + tmp_salt = null_salt; + tmp_salt_len = hash_len; + } } - - hash_len = mbedtls_md_get_size( md ); - - if( hash_len == 0 ) - { - return MBEDTLS_ERR_HKDF_BAD_INPUT_DATA; - } - - salt = null_salt; - salt_len = hash_len; } - return( mbedtls_md_hmac( md, salt, salt_len, ikm, ikm_len, prk ) ); + if (ret == 0) { + ret = mbedtls_md_hmac(md, tmp_salt, tmp_salt_len, ikm, ikm_len, prk); + } + + return ret; } -int32_t mbedtls_hkdf_expand( const mbedtls_md_info_t *md, const uint8_t *prk, +int32_t mbedtls_hkdf_expand(const mbedtls_md_info_t *md, const uint8_t *prk, size_t prk_len, const uint8_t *info, - size_t info_len, uint8_t *okm, size_t okm_len ) + size_t info_len, uint8_t *okm, size_t okm_len) { size_t hash_len; size_t where = 0; size_t n; size_t t_len = 0; + size_t tmp_info_len = info_len; + const uint8_t *tmp_info = info; size_t i; int32_t ret = 0; mbedtls_md_context_t ctx; uint8_t t[MBEDTLS_MD_MAX_SIZE]; - if( okm == NULL ) - { - return( MBEDTLS_ERR_HKDF_BAD_INPUT_DATA ); - } + hash_len = mbedtls_md_get_size(md); - hash_len = mbedtls_md_get_size( md ); + if ((okm == NULL) || (prk_len < hash_len) || (hash_len == 0U)) { + ret = MBEDTLS_ERR_HKDF_BAD_INPUT_DATA; + } else { - if( prk_len < hash_len || hash_len == 0 ) - { - return( MBEDTLS_ERR_HKDF_BAD_INPUT_DATA ); - } - - if( info == NULL ) - { - info = (const uint8_t *) ""; - info_len = 0; - } - - n = okm_len / hash_len; - - if( (okm_len % hash_len) != 0 ) - { - n++; - } - - /* - * Per RFC 5869 Section 2.3, okm_len must not exceed - * 255 times the hash length - */ - if( n > 255 ) - { - return( MBEDTLS_ERR_HKDF_BAD_INPUT_DATA ); - } - - mbedtls_md_init( &ctx ); - - if( (ret = mbedtls_md_setup( &ctx, md) ) != 0 ) - { - goto exit; - } - - /* - * Compute T = T(1) | T(2) | T(3) | ... | T(N) - * Where T(N) is defined in RFC 5869 Section 2.3 - */ - for( i = 1; i <= n; i++ ) - { - size_t num_to_copy; - uint8_t c = i & 0xff; - - ret = mbedtls_md_hmac_starts( &ctx, prk, prk_len ); - if( ret != 0 ) - { - goto exit; + if (tmp_info == NULL) { + tmp_info = (const uint8_t *) ""; + tmp_info_len = 0U; } - ret = mbedtls_md_hmac_update( &ctx, t, t_len ); - if( ret != 0 ) - { - goto exit; + n = okm_len / hash_len; + + if ((okm_len % hash_len) != 0U) { + n++; } - ret = mbedtls_md_hmac_update( &ctx, info, info_len ); - if( ret != 0 ) - { - goto exit; - } + /* + * Per RFC 5869 Section 2.3, okm_len must not exceed + * 255 times the hash length + */ + if (n > 255U) { + ret = MBEDTLS_ERR_HKDF_BAD_INPUT_DATA; + } else { + mbedtls_md_init(&ctx); - /* The constant concatenated to the end of each T(n) is a single octet. - * */ - ret = mbedtls_md_hmac_update( &ctx, &c, 1 ); - if( ret != 0 ) - { - goto exit; - } + ret = mbedtls_md_setup(&ctx, md); + if (ret == 0) { - ret = mbedtls_md_hmac_finish( &ctx, t ); - if( ret != 0 ) - { - goto exit; - } + /* + * Compute T = T(1) | T(2) | T(3) | ... | T(N) + * Where T(N) is defined in RFC 5869 Section 2.3 + */ + for (i = 1U; i <= n; i++) { + size_t num_to_copy; + uint8_t c = (uint8_t)(i & 0xffU); - num_to_copy = (i != n) ? hash_len : (okm_len - where); - memcpy_s( okm + where, num_to_copy, t, num_to_copy ); - where += hash_len; - t_len = hash_len; + ret = mbedtls_md_hmac_starts(&ctx, prk, prk_len); + if (ret == 0) { + ret = mbedtls_md_hmac_update(&ctx, t, t_len); + } + + if (ret == 0) { + ret = mbedtls_md_hmac_update(&ctx, tmp_info, tmp_info_len); + } + + /* The constant concatenated to the end of each T(n) is a single octet. + * */ + if (ret == 0) { + ret = mbedtls_md_hmac_update(&ctx, &c, 1); + } + + if (ret == 0) { + ret = mbedtls_md_hmac_finish(&ctx, t); + } + + if (ret != 0) { + break; + } + + num_to_copy = (i != n) ? hash_len : (okm_len - where); + (void)memcpy_s(okm + where, num_to_copy, t, num_to_copy); + where += hash_len; + t_len = hash_len; + } + } + + mbedtls_md_free(&ctx); + } } -exit: - mbedtls_md_free( &ctx ); - mbedtls_platform_zeroize( t, sizeof( t ) ); + (void)mbedtls_platform_zeroize(t, sizeof(t)); - return( ret ); + return ret; }