From 7b4d1d59f1706d2dd8bc0d94a1f10a51181187f2 Mon Sep 17 00:00:00 2001 From: Paul Szczepanek Date: Wed, 23 Jun 2021 17:29:00 +0100 Subject: [PATCH 1/3] add new BLE API call to sync security db with persisten storage --- UNITTESTS/fakes/ble/SecurityManagerImpl_mock.h | 1 + .../ble/source/generic/SecurityManagerImpl.h | 2 ++ .../FEATURE_BLE/include/ble/SecurityManager.h | 18 ++++++++++++++++++ .../FEATURE_BLE/source/SecurityManager.cpp | 5 +++++ .../source/generic/FileSecurityDb.h | 2 +- .../source/generic/KVStoreSecurityDb.h | 2 +- .../FEATURE_BLE/source/generic/SecurityDb.h | 4 +++- .../source/generic/SecurityManagerImpl.cpp | 12 ++++++++++++ .../source/generic/SecurityManagerImpl.h | 2 ++ 9 files changed, 45 insertions(+), 3 deletions(-) diff --git a/UNITTESTS/fakes/ble/SecurityManagerImpl_mock.h b/UNITTESTS/fakes/ble/SecurityManagerImpl_mock.h index 3f1007e4fa5..014b077596c 100644 --- a/UNITTESTS/fakes/ble/SecurityManagerImpl_mock.h +++ b/UNITTESTS/fakes/ble/SecurityManagerImpl_mock.h @@ -34,6 +34,7 @@ class SecurityManagerMock : public ble::impl::SecurityManager { MOCK_METHOD(ble_error_t, init, (bool enableBonding, bool requireMITM, SecurityIOCapabilities_t iocaps, const Passkey_t passkey, bool signing, const char *dbFilepath), (override)); MOCK_METHOD(ble_error_t, setDatabaseFilepath, (const char *dbFilepath), (override)); MOCK_METHOD(ble_error_t, preserveBondingStateOnReset, (bool enable), (override)); + MOCK_METHOD(ble_error_t, writeBondingStateToPersistentStorage, (), (override)); MOCK_METHOD(ble_error_t, purgeAllBondingState, (), (override)); MOCK_METHOD(ble_error_t, generateWhitelistFromBondTable, (::ble::whitelist_t *whitelist), (const, override)); MOCK_METHOD(ble_error_t, requestPairing, (ble::connection_handle_t connectionHandle), (override)); diff --git a/UNITTESTS/fakes/ble/source/generic/SecurityManagerImpl.h b/UNITTESTS/fakes/ble/source/generic/SecurityManagerImpl.h index 35e4de51178..7f033f4a1bf 100644 --- a/UNITTESTS/fakes/ble/source/generic/SecurityManagerImpl.h +++ b/UNITTESTS/fakes/ble/source/generic/SecurityManagerImpl.h @@ -52,6 +52,8 @@ class SecurityManager { virtual ble_error_t preserveBondingStateOnReset(bool enable) { return BLE_ERROR_NONE; }; + virtual ble_error_t writeBondingStateToPersistentStorage() { return BLE_ERROR_NONE; }; + //////////////////////////////////////////////////////////////////////////// // List management // diff --git a/connectivity/FEATURE_BLE/include/ble/SecurityManager.h b/connectivity/FEATURE_BLE/include/ble/SecurityManager.h index c32d0c9c409..b1fe3ba114e 100644 --- a/connectivity/FEATURE_BLE/include/ble/SecurityManager.h +++ b/connectivity/FEATURE_BLE/include/ble/SecurityManager.h @@ -501,11 +501,29 @@ class SecurityManager * Normally all bonding information is lost when device is reset, this requests that the stack * attempts to save the information and reload it during initialisation. This is not guaranteed. * + * @note This option is itself saved together with bonding data. When data is read after reset, + * the state of this option decides if data should be restored. If this option has not been saved + * the data will not be restored even if partial data is present. + * * @param[in] enable if true the stack will attempt to preserve bonding information on reset. * @return BLE_ERROR_NONE or appropriate error code indicating the failure reason. */ ble_error_t preserveBondingStateOnReset(bool enable); + /** + * Some or all of bonding information may be stored in memory while in use. This will write + * bonding data to persistent storage. This will have no effect if no persistent storage is enabled. + * + * @note You still need to call preserveBondingStateOnReset(true) before reset happens for data to be + * loaded when it's read. + * + * @note Depending on the driver used to implement the storage solution used this may be a disruptive + * operation and may cause active connections to drop due to failed processing deadlines. + * + * @return BLE_ERROR_NONE or appropriate error code indicating the failure reason. + */ + ble_error_t writeBondingStateToPersistentStorage(); + //////////////////////////////////////////////////////////////////////////// // List management // diff --git a/connectivity/FEATURE_BLE/source/SecurityManager.cpp b/connectivity/FEATURE_BLE/source/SecurityManager.cpp index 602c9224bd5..b7b981933e7 100644 --- a/connectivity/FEATURE_BLE/source/SecurityManager.cpp +++ b/connectivity/FEATURE_BLE/source/SecurityManager.cpp @@ -47,6 +47,11 @@ ble_error_t SecurityManager::preserveBondingStateOnReset(bool enable) return impl->preserveBondingStateOnReset(enable); } +ble_error_t SecurityManager::writeBondingStateToPersistentStorage() +{ + return impl->writeBondingStateToPersistentStorage(); +} + ble_error_t SecurityManager::purgeAllBondingState() { return impl->purgeAllBondingState(); diff --git a/connectivity/FEATURE_BLE/source/generic/FileSecurityDb.h b/connectivity/FEATURE_BLE/source/generic/FileSecurityDb.h index 1ed0741e2f5..cf20c540326 100644 --- a/connectivity/FEATURE_BLE/source/generic/FileSecurityDb.h +++ b/connectivity/FEATURE_BLE/source/generic/FileSecurityDb.h @@ -136,7 +136,7 @@ class FileSecurityDb : public SecurityDb { virtual void restore(); - virtual void sync(entry_handle_t db_handle); + virtual void sync(entry_handle_t db_handle = invalid_entry_handle); virtual void set_restore(bool reload); diff --git a/connectivity/FEATURE_BLE/source/generic/KVStoreSecurityDb.h b/connectivity/FEATURE_BLE/source/generic/KVStoreSecurityDb.h index 8ce9ac57884..a34c023cdda 100644 --- a/connectivity/FEATURE_BLE/source/generic/KVStoreSecurityDb.h +++ b/connectivity/FEATURE_BLE/source/generic/KVStoreSecurityDb.h @@ -200,7 +200,7 @@ class KVStoreSecurityDb : public SecurityDb { virtual void restore(); - virtual void sync(entry_handle_t db_handle); + virtual void sync(entry_handle_t db_handle = invalid_entry_handle); virtual void set_restore(bool reload); diff --git a/connectivity/FEATURE_BLE/source/generic/SecurityDb.h b/connectivity/FEATURE_BLE/source/generic/SecurityDb.h index 8decd154c10..eec3b7d7b88 100644 --- a/connectivity/FEATURE_BLE/source/generic/SecurityDb.h +++ b/connectivity/FEATURE_BLE/source/generic/SecurityDb.h @@ -113,6 +113,8 @@ class SecurityDb { */ typedef void* entry_handle_t; + static constexpr entry_handle_t invalid_entry_handle = nullptr; + /* callbacks for asynchronous data retrieval from the security db */ typedef mbed::Callback @@ -522,7 +524,7 @@ class SecurityDb { /** * Flush all values which might be stored in memory into NVM. */ - virtual void sync(entry_handle_t db_handle); + virtual void sync(entry_handle_t db_handle = invalid_entry_handle); /** * Toggle whether values should be preserved across resets. diff --git a/connectivity/FEATURE_BLE/source/generic/SecurityManagerImpl.cpp b/connectivity/FEATURE_BLE/source/generic/SecurityManagerImpl.cpp index 584dd9212f7..e999375e22c 100644 --- a/connectivity/FEATURE_BLE/source/generic/SecurityManagerImpl.cpp +++ b/connectivity/FEATURE_BLE/source/generic/SecurityManagerImpl.cpp @@ -222,6 +222,18 @@ ble_error_t SecurityManager::preserveBondingStateOnReset(bool enabled) return BLE_ERROR_NONE; } + +ble_error_t SecurityManager::writeBondingStateToPersistentStorage() +{ + tr_info("Writing bonding to storage"); + if (!_db) { + tr_error("Failure, DB not initialized"); + return BLE_ERROR_INITIALIZATION_INCOMPLETE; + } + _db->sync(); + return BLE_ERROR_NONE; +} + //////////////////////////////////////////////////////////////////////////// // List management // diff --git a/connectivity/FEATURE_BLE/source/generic/SecurityManagerImpl.h b/connectivity/FEATURE_BLE/source/generic/SecurityManagerImpl.h index 737f4dccca9..22a92f736bb 100644 --- a/connectivity/FEATURE_BLE/source/generic/SecurityManagerImpl.h +++ b/connectivity/FEATURE_BLE/source/generic/SecurityManagerImpl.h @@ -85,6 +85,8 @@ class SecurityManager : ble_error_t preserveBondingStateOnReset(bool enable); + ble_error_t writeBondingStateToPersistentStorage(); + //////////////////////////////////////////////////////////////////////////// // List management // From c9d0ff86747ba9fd02cb47bf5f98346f5abcc48e Mon Sep 17 00:00:00 2001 From: Paul Szczepanek Date: Wed, 23 Jun 2021 22:05:20 +0100 Subject: [PATCH 2/3] ble security db complete sync implementation --- .../source/generic/FileSecurityDb.cpp | 16 ++++++++++++++++ .../source/generic/KVStoreSecurityDb.cpp | 16 ++++++++++++++++ .../FEATURE_BLE/source/generic/SecurityDb.h | 6 +++++- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/connectivity/FEATURE_BLE/source/generic/FileSecurityDb.cpp b/connectivity/FEATURE_BLE/source/generic/FileSecurityDb.cpp index 6ad4fd45b00..09f77638d09 100644 --- a/connectivity/FEATURE_BLE/source/generic/FileSecurityDb.cpp +++ b/connectivity/FEATURE_BLE/source/generic/FileSecurityDb.cpp @@ -361,6 +361,22 @@ void FileSecurityDb::restore() void FileSecurityDb::sync(entry_handle_t db_handle) { + /* if no entry is selected we will sync all entries */ + if (db_handle == invalid_entry_handle) { + /* only write the connected devices as others are already written */ + for (size_t i = 0; i < get_entry_count(); i++) { + entry_handle_t db_handle = get_entry_handle_by_index(i); + SecurityDistributionFlags_t* flags = get_distribution_flags(db_handle); + + if (flags && flags->connected) { + sync(db_handle); + } + } + /* global sync triggers a flush */ + fflush(_db_file); + return; + } + entry_t *entry = as_entry(db_handle); if (!entry) { return; diff --git a/connectivity/FEATURE_BLE/source/generic/KVStoreSecurityDb.cpp b/connectivity/FEATURE_BLE/source/generic/KVStoreSecurityDb.cpp index 7a186203425..011ecfbfb00 100644 --- a/connectivity/FEATURE_BLE/source/generic/KVStoreSecurityDb.cpp +++ b/connectivity/FEATURE_BLE/source/generic/KVStoreSecurityDb.cpp @@ -333,6 +333,22 @@ void KVStoreSecurityDb::restore() void KVStoreSecurityDb::sync(entry_handle_t db_handle) { + /* storage synchronisation is handled by KVStore itself, no explicit flushing */ + + /* if no entry is selected we will sync all entries */ + if (db_handle == invalid_entry_handle) { + /* only write the connected devices as others are already written */ + for (size_t i = 0; i < get_entry_count(); i++) { + entry_handle_t db_handle = get_entry_handle_by_index(i); + SecurityDistributionFlags_t* flags = get_distribution_flags(db_handle); + + if (flags && flags->connected) { + sync(db_handle); + } + } + return; + } + entry_t *entry = as_entry(db_handle); if (!entry) { return; diff --git a/connectivity/FEATURE_BLE/source/generic/SecurityDb.h b/connectivity/FEATURE_BLE/source/generic/SecurityDb.h index eec3b7d7b88..4f7ae5060b0 100644 --- a/connectivity/FEATURE_BLE/source/generic/SecurityDb.h +++ b/connectivity/FEATURE_BLE/source/generic/SecurityDb.h @@ -522,7 +522,11 @@ class SecurityDb { virtual void restore(); /** - * Flush all values which might be stored in memory into NVM. + * Write all values and attempt to sync persistent storage. Passing in an optional valid + * db_handle will only write the given entry and not attempt to flush buffers. + * + * @param db_handle database entry to write. If invalid all entries are written and + * persistent storage attempts to sync (flush buffers). */ virtual void sync(entry_handle_t db_handle = invalid_entry_handle); From 470917f9ab2d46b83c6256345ea00238b3a0f120 Mon Sep 17 00:00:00 2001 From: Paul Szczepanek Date: Mon, 28 Jun 2021 19:38:13 +0100 Subject: [PATCH 3/3] add implicit call to preserve state on reset --- connectivity/FEATURE_BLE/include/ble/SecurityManager.h | 3 +-- connectivity/FEATURE_BLE/source/SecurityManager.cpp | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/connectivity/FEATURE_BLE/include/ble/SecurityManager.h b/connectivity/FEATURE_BLE/include/ble/SecurityManager.h index b1fe3ba114e..b8dfe5de9e9 100644 --- a/connectivity/FEATURE_BLE/include/ble/SecurityManager.h +++ b/connectivity/FEATURE_BLE/include/ble/SecurityManager.h @@ -514,8 +514,7 @@ class SecurityManager * Some or all of bonding information may be stored in memory while in use. This will write * bonding data to persistent storage. This will have no effect if no persistent storage is enabled. * - * @note You still need to call preserveBondingStateOnReset(true) before reset happens for data to be - * loaded when it's read. + * @note This implicitly also calls preserveBondingStateOnReset(true) inside. * * @note Depending on the driver used to implement the storage solution used this may be a disruptive * operation and may cause active connections to drop due to failed processing deadlines. diff --git a/connectivity/FEATURE_BLE/source/SecurityManager.cpp b/connectivity/FEATURE_BLE/source/SecurityManager.cpp index b7b981933e7..0e3026a06e7 100644 --- a/connectivity/FEATURE_BLE/source/SecurityManager.cpp +++ b/connectivity/FEATURE_BLE/source/SecurityManager.cpp @@ -49,6 +49,10 @@ ble_error_t SecurityManager::preserveBondingStateOnReset(bool enable) ble_error_t SecurityManager::writeBondingStateToPersistentStorage() { + ble_error_t err = impl->preserveBondingStateOnReset(true); + if (err) { + return err; + } return impl->writeBondingStateToPersistentStorage(); }