From 9852566bef38d1a618a4c8cd6bdef0e73270e4b7 Mon Sep 17 00:00:00 2001 From: Brian Sipos Date: Tue, 14 Apr 2026 21:32:36 -0400 Subject: [PATCH 1/3] Use weak pointer internally for key storage. Clean up duplicate bsl_data inits. --- src/Data.c | 27 ++++++++++++++++ src/Data.h | 5 +++ src/crypto/CryptoInterface.c | 51 ++++++++++++++++++------------ src/mock_bpa/decode.c | 2 +- src/security_context/BCB_AES_GCM.c | 6 ++-- 5 files changed, 67 insertions(+), 24 deletions(-) diff --git a/src/Data.c b/src/Data.c index a3ee267..64c5a73 100644 --- a/src/Data.c +++ b/src/Data.c @@ -78,10 +78,37 @@ int BSL_Data_InitView(BSL_Data_t *data, size_t len, const BSL_DataPtr_t src) return BSL_SUCCESS; } +void BSL_Data_InitSet(BSL_Data_t *data, const BSL_Data_t *src) +{ + ASSERT_ARG_NONNULL(data); + ASSERT_ARG_NONNULL(src); + ASSERT_ARG_EXPR(data != src); + + bsl_data_int_reset(data); + data->owned = src->owned; + data->len = src->len; + if (src->owned) + { + data->ptr = BSL_malloc(data->len); + if (data->ptr) + { + memcpy(data->ptr, src->ptr, src->len); + } + } + else + { + data->ptr = src->ptr; + } +} + void BSL_Data_InitMove(BSL_Data_t *data, BSL_Data_t *src) { ASSERT_ARG_NONNULL(data); ASSERT_ARG_NONNULL(src); + if (data == src) + { + return; + } *data = *src; bsl_data_int_reset(src); } diff --git a/src/Data.h b/src/Data.h index dcaea67..072ca5c 100644 --- a/src/Data.h +++ b/src/Data.h @@ -90,6 +90,11 @@ int BSL_Data_InitBuffer(BSL_Data_t *data, size_t bytelen); */ int BSL_Data_InitView(BSL_Data_t *data, size_t len, BSL_DataPtr_t src); +/** Initialize a data struct as a copy of existing struct. + * If the source is owned, the data will be copied. Otherwise, they will + * both be a view onto the same data. + */ +void BSL_Data_InitSet(BSL_Data_t *data, const BSL_Data_t *src); /// @overload void BSL_Data_InitMove(BSL_Data_t *data, BSL_Data_t *src); diff --git a/src/crypto/CryptoInterface.c b/src/crypto/CryptoInterface.c index 0afe521..952189d 100644 --- a/src/crypto/CryptoInterface.c +++ b/src/crypto/CryptoInterface.c @@ -27,6 +27,7 @@ #include #include +#include #include #include #include @@ -48,8 +49,10 @@ typedef struct BSL_CryptoKey_s BSL_Crypto_KeyStats_t stats; } BSL_CryptoKey_t; -static int BSL_CryptoKey_Init(BSL_CryptoKey_t *key) +static void BSL_CryptoKey_Init(BSL_CryptoKey_t *key) { + ASSERT_ARG_NONNULL(key); + key->pkey = NULL; BSL_Data_Init(&(key->raw)); @@ -57,12 +60,12 @@ static int BSL_CryptoKey_Init(BSL_CryptoKey_t *key) { key->stats.stats[i] = 0; } - - return 0; } -static int BSL_CryptoKey_Deinit(BSL_CryptoKey_t *key) +static void BSL_CryptoKey_Deinit(BSL_CryptoKey_t *key) { + ASSERT_ARG_NONNULL(key); + if (key->pkey) { EVP_PKEY_free(key->pkey); @@ -74,19 +77,26 @@ static int BSL_CryptoKey_Deinit(BSL_CryptoKey_t *key) { key->stats.stats[i] = 0; } - return 0; } + /** M*LIB OPLIST for ::BSL_CryptoKey_t */ -#define M_OPL_BSL_CryptoKey_t() M_OPEXTEND(M_POD_OPLIST, CLEAR(API_2(BSL_CryptoKey_Deinit))) +#define M_OPL_BSL_CryptoKey_t() \ + M_OPEXTEND(M_POD_OPLIST, INIT(API_2(BSL_CryptoKey_Init)), INIT_SET(0), SET(0), CLEAR(API_2(BSL_CryptoKey_Deinit))) + +/** @struct BSL_CryptoKeyPtr_t + * Non-thread-safe shared pointer to memory-stable ::BSL_CryptoKey_t struct. + */ /** @struct BSL_CryptoKeyDict_t - * Stable dict of crypto keys (key: key ID | value: key) + * Stable dict of crypto keys (key: key ID | value: BSL_CryptoKeyPtr_t) */ /// @cond Doxygen_Suppress // NOLINTBEGIN // GCOV_EXCL_START -DICT_DEF2(BSL_CryptoKeyDict, string_t, STRING_OPLIST, BSL_CryptoKey_t, M_OPL_BSL_CryptoKey_t()) +M_SHARED_WEAK_PTR_DEF(BSL_CryptoKeyPtr, BSL_CryptoKey_t, M_OPL_BSL_CryptoKey_t()) +M_DICT_DEF2(BSL_CryptoKeyDict, m_string_t, M_STRING_OPLIST, BSL_CryptoKeyPtr_t *, + M_SHARED_PTR_OPLIST(BSL_CryptoKeyPtr, M_OPL_BSL_CryptoKey_t())) // GCOV_EXCL_STOP // NOLINTEND /// @endcond @@ -634,7 +644,7 @@ int BSL_Crypto_GenKey(size_t key_length, void **key_out) CHK_PROPERTY(new_key); BSL_CryptoKey_Init(new_key); - BSL_Data_InitBuffer(&new_key->raw, key_length); + BSL_Data_Resize(&new_key->raw, key_length); if (rand_bytes_generator(new_key->raw.ptr, (int)new_key->raw.len) != 1) { return -2; @@ -670,19 +680,17 @@ int BSL_Crypto_AddRegistryKey(const char *keyid, const uint8_t *secret, size_t s CHK_ARG_NONNULL(secret); CHK_ARG_EXPR(secret_len > 0); - BSL_CryptoKey_t key; - BSL_CryptoKey_Init(&key); + BSL_CryptoKeyPtr_t *key_ptr = BSL_CryptoKeyPtr_new(); + BSL_CryptoKey_t *key = BSL_CryptoKeyPtr_ref(key_ptr); EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_HMAC, NULL); int res = EVP_PKEY_keygen_init(ctx); CHK_PROPERTY(res == 1); - key.pkey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, secret, (int)secret_len); + key->pkey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, secret, (int)secret_len); EVP_PKEY_CTX_free(ctx); - BSL_Data_Init(&key.raw); - int ecode = 0; - if ((ecode = BSL_Data_CopyFrom(&key.raw, secret_len, secret)) < 0) + if ((ecode = BSL_Data_CopyFrom(&key->raw, secret_len, secret)) < 0) { BSL_LOG_ERR("Failed to copy key"); return ecode; @@ -692,9 +700,10 @@ int BSL_Crypto_AddRegistryKey(const char *keyid, const uint8_t *secret, size_t s string_init_set_str(keyid_str, keyid); pthread_mutex_lock(&StaticCryptoMutex); - BSL_CryptoKeyDict_set_at(StaticKeyRegistry, keyid_str, key); + BSL_CryptoKeyDict_set_at(StaticKeyRegistry, keyid_str, key_ptr); pthread_mutex_unlock(&StaticCryptoMutex); + BSL_CryptoKeyPtr_release(key_ptr); string_clear(keyid_str); return 0; } @@ -709,14 +718,14 @@ int BSL_Crypto_GetRegistryKey(const char *keyid, void **key_handle) int retval = BSL_SUCCESS; pthread_mutex_lock(&StaticCryptoMutex); - BSL_CryptoKey_t *found = BSL_CryptoKeyDict_get(StaticKeyRegistry, keyid_str); + BSL_CryptoKeyPtr_t **found = BSL_CryptoKeyDict_get(StaticKeyRegistry, keyid_str); if (!found) { retval = BSL_ERR_NOT_FOUND; } else { - *key_handle = found; + *key_handle = BSL_CryptoKeyPtr_ref(*found); } pthread_mutex_unlock(&StaticCryptoMutex); string_clear(keyid_str); @@ -746,16 +755,18 @@ int BSL_Crypto_GetKeyStatistics(const char *keyid, BSL_Crypto_KeyStats_t *stats) int retval = BSL_SUCCESS; pthread_mutex_lock(&StaticCryptoMutex); - BSL_CryptoKey_t *found = BSL_CryptoKeyDict_get(StaticKeyRegistry, keyid_str); + BSL_CryptoKeyPtr_t **found = BSL_CryptoKeyDict_get(StaticKeyRegistry, keyid_str); if (!found) { retval = BSL_ERR_NOT_FOUND; } else { + BSL_CryptoKey_t *key = BSL_CryptoKeyPtr_ref(*found); + for (uint64_t i = 0; i < BSL_CRYPTO_KEYSTATS_MAX_INDEX; i++) { - stats->stats[i] = found->stats.stats[i]; + stats->stats[i] = key->stats.stats[i]; } } pthread_mutex_unlock(&StaticCryptoMutex); diff --git a/src/mock_bpa/decode.c b/src/mock_bpa/decode.c index 254173c..9f5fa5c 100644 --- a/src/mock_bpa/decode.c +++ b/src/mock_bpa/decode.c @@ -227,7 +227,7 @@ int bsl_mock_decode_primary(QCBORDecodeContext *dec, MockBPA_PrimaryBlock_t *blk return 4; } - BSL_Data_InitBuffer(&blk->encoded, end - begin); + BSL_Data_Resize(&blk->encoded, end - begin); memcpy(blk->encoded.ptr, (const uint8_t *)buf.ptr + begin, blk->encoded.len); return 0; diff --git a/src/security_context/BCB_AES_GCM.c b/src/security_context/BCB_AES_GCM.c index e232467..8941e8b 100644 --- a/src/security_context/BCB_AES_GCM.c +++ b/src/security_context/BCB_AES_GCM.c @@ -54,7 +54,7 @@ int BSLX_BCB_ComputeAAD(BSLX_BCB_t *bcb_context) // See: https://www.rfc-editor.org/rfc/rfc9173.html#name-aad-scope-flags // Note, this over-allocates and is resized downward later. const size_t aad_len = 1024; - if (BSL_SUCCESS != BSL_Data_InitBuffer(&bcb_context->aad, aad_len)) + if (BSL_SUCCESS != BSL_Data_Resize(&bcb_context->aad, aad_len)) { BSL_LOG_ERR("Failed to allocate AAD space"); return BSL_ERR_INSUFFICIENT_SPACE; @@ -260,7 +260,7 @@ int BSLX_BCB_Encrypt(BSLX_BCB_t *bcb_context) // https://www.rfc-editor.org/rfc/rfc9173.html#name-initialization-vector-iv // "A value of 12 bytes SHOULD be used unless local security policy requires a different length" - BSL_Data_InitBuffer(&bcb_context->iv, RFC9173_BCB_DEFAULT_IV_LEN); + BSL_Data_Resize(&bcb_context->iv, RFC9173_BCB_DEFAULT_IV_LEN); void *iv_ptr = bcb_context->iv.ptr; const size_t iv_len = bcb_context->iv.len; if (BSL_SUCCESS != BSL_Crypto_GenIV(iv_ptr, iv_len)) @@ -550,7 +550,7 @@ int BSLX_BCB_Init(BSLX_BCB_t *bcb_context, BSL_BundleRef_t *bundle, const BSL_Se bcb_context->bundle = bundle; - if (BSL_SUCCESS != BSL_Data_InitBuffer(&bcb_context->debugstr, 512)) + if (BSL_SUCCESS != BSL_Data_Resize(&bcb_context->debugstr, 512)) { BSL_LOG_ERR("Failed to allocated debug str"); return BSL_ERR_INSUFFICIENT_SPACE; From e13e5085f5c9e7e0d9a326663dbb6cdbffcc0aaf Mon Sep 17 00:00:00 2001 From: Brian Sipos Date: Tue, 14 Apr 2026 21:39:48 -0400 Subject: [PATCH 2/3] Remove unneeded function --- src/Data.c | 23 ----------------------- src/Data.h | 6 +----- src/crypto/CryptoInterface.c | 7 ++++--- 3 files changed, 5 insertions(+), 31 deletions(-) diff --git a/src/Data.c b/src/Data.c index 64c5a73..b783e74 100644 --- a/src/Data.c +++ b/src/Data.c @@ -78,29 +78,6 @@ int BSL_Data_InitView(BSL_Data_t *data, size_t len, const BSL_DataPtr_t src) return BSL_SUCCESS; } -void BSL_Data_InitSet(BSL_Data_t *data, const BSL_Data_t *src) -{ - ASSERT_ARG_NONNULL(data); - ASSERT_ARG_NONNULL(src); - ASSERT_ARG_EXPR(data != src); - - bsl_data_int_reset(data); - data->owned = src->owned; - data->len = src->len; - if (src->owned) - { - data->ptr = BSL_malloc(data->len); - if (data->ptr) - { - memcpy(data->ptr, src->ptr, src->len); - } - } - else - { - data->ptr = src->ptr; - } -} - void BSL_Data_InitMove(BSL_Data_t *data, BSL_Data_t *src) { ASSERT_ARG_NONNULL(data); diff --git a/src/Data.h b/src/Data.h index 072ca5c..f7adc27 100644 --- a/src/Data.h +++ b/src/Data.h @@ -90,12 +90,8 @@ int BSL_Data_InitBuffer(BSL_Data_t *data, size_t bytelen); */ int BSL_Data_InitView(BSL_Data_t *data, size_t len, BSL_DataPtr_t src); -/** Initialize a data struct as a copy of existing struct. - * If the source is owned, the data will be copied. Otherwise, they will - * both be a view onto the same data. +/** Initialize a data struct with move semantics from an existing struct. */ -void BSL_Data_InitSet(BSL_Data_t *data, const BSL_Data_t *src); -/// @overload void BSL_Data_InitMove(BSL_Data_t *data, BSL_Data_t *src); /** De-initialize a data struct, freeing if necessary. diff --git a/src/crypto/CryptoInterface.c b/src/crypto/CryptoInterface.c index 952189d..d0b428f 100644 --- a/src/crypto/CryptoInterface.c +++ b/src/crypto/CryptoInterface.c @@ -79,7 +79,6 @@ static void BSL_CryptoKey_Deinit(BSL_CryptoKey_t *key) } } - /** M*LIB OPLIST for ::BSL_CryptoKey_t */ #define M_OPL_BSL_CryptoKey_t() \ @@ -681,9 +680,11 @@ int BSL_Crypto_AddRegistryKey(const char *keyid, const uint8_t *secret, size_t s CHK_ARG_EXPR(secret_len > 0); BSL_CryptoKeyPtr_t *key_ptr = BSL_CryptoKeyPtr_new(); + CHK_PROPERTY(key_ptr != NULL); + // actual key struct BSL_CryptoKey_t *key = BSL_CryptoKeyPtr_ref(key_ptr); - EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_HMAC, NULL); - int res = EVP_PKEY_keygen_init(ctx); + EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_HMAC, NULL); + int res = EVP_PKEY_keygen_init(ctx); CHK_PROPERTY(res == 1); key->pkey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, secret, (int)secret_len); From c2b511f162b4447d16e645e67d27ddf05250848a Mon Sep 17 00:00:00 2001 From: Brian Sipos Date: Tue, 14 Apr 2026 21:43:09 -0400 Subject: [PATCH 3/3] take sonar suggestion for const --- src/crypto/CryptoInterface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crypto/CryptoInterface.c b/src/crypto/CryptoInterface.c index d0b428f..0b3a290 100644 --- a/src/crypto/CryptoInterface.c +++ b/src/crypto/CryptoInterface.c @@ -763,7 +763,7 @@ int BSL_Crypto_GetKeyStatistics(const char *keyid, BSL_Crypto_KeyStats_t *stats) } else { - BSL_CryptoKey_t *key = BSL_CryptoKeyPtr_ref(*found); + const BSL_CryptoKey_t *key = BSL_CryptoKeyPtr_cref(*found); for (uint64_t i = 0; i < BSL_CRYPTO_KEYSTATS_MAX_INDEX; i++) {