Skip to content

Commit a10a10a

Browse files
committed
BLE - Nordic: Release crypto cell when not in use.
Previously, the CryptoToolbox was allocated once as part of the security manager. This was inneficient memory wise as it is only use to prepare key at initialization and when we need to compute shared keys. This was also inneficient power consumption wise as the Crypto cell was kept enabled even when it wasn't used. This fix creates a CryptoToolbox whenever it is needed and release it once it has fulfilled its purpose. Note that CryptoToolbox allocation happens on the heap as mbed tls data structure are huge and there's an high risk of crushing the stack.
1 parent ebf3a19 commit a10a10a

File tree

2 files changed

+12
-7
lines changed

2 files changed

+12
-7
lines changed

features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NORDIC_SOFTDEVICE/TARGET_NRF52/source/nRF5xPalSecurityManager.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,15 +111,17 @@ nRF5xSecurityManager::~nRF5xSecurityManager()
111111
ble_error_t nRF5xSecurityManager::initialize()
112112
{
113113
#if defined(MBEDTLS_ECDH_C)
114-
if (_crypto.generate_keys(
114+
// Note: we do not use the object on the stack as the CryptoToolbox is quite large
115+
// Please do not change or we risk a stack overflow.
116+
CryptoToolbox* crypto = new CryptoToolbox();
117+
bool success = crypto->generate_keys(
115118
make_ArrayView(X),
116119
make_ArrayView(Y),
117120
make_ArrayView(secret)
118-
)) {
119-
return BLE_ERROR_NONE;
120-
}
121+
);
122+
delete crypto;
121123

122-
return BLE_ERROR_INTERNAL_STACK_FAILURE;
124+
return success ? BLE_ERROR_NONE : BLE_ERROR_INTERNAL_STACK_FAILURE;
123125
#endif
124126
return BLE_ERROR_NONE;
125127
}
@@ -934,12 +936,16 @@ bool nRF5xSecurityManager::sm_handler(const ble_evt_t *evt)
934936
static const size_t key_size = public_key_coord_t::size_;
935937
ble_gap_lesc_dhkey_t shared_secret;
936938

937-
_crypto.generate_shared_secret(
939+
// Allocated on the heap to reduce stack pressure.
940+
// Risk stack overflows if allocated on stack.
941+
CryptoToolbox* crypto = new CryptoToolbox();
942+
crypto->generate_shared_secret(
938943
make_const_ArrayView<key_size>(dhkey_request.p_pk_peer->pk),
939944
make_const_ArrayView<key_size>(dhkey_request.p_pk_peer->pk + key_size),
940945
make_const_ArrayView(secret),
941946
shared_secret.key
942947
);
948+
delete crypto;
943949

944950
sd_ble_gap_lesc_dhkey_reply(connection, &shared_secret);
945951

features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NORDIC_SOFTDEVICE/TARGET_NRF52/source/nRF5xPalSecurityManager.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,6 @@ class nRF5xSecurityManager : public ::ble::pal::SecurityManager {
360360

361361
pairing_control_block_t* _control_blocks;
362362
#if defined(MBEDTLS_ECDH_C)
363-
CryptoToolbox _crypto;
364363
ble::public_key_coord_t X;
365364
ble::public_key_coord_t Y;
366365
ble::public_key_coord_t secret;

0 commit comments

Comments
 (0)