Merge pull request #14824 from paul-szczepanek-arm/persistence-fix

BLE: Manual BLE security manager db synchronisation

Author: 0xc0170

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));

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
     //

connectivity/FEATURE_BLE/include/ble/SecurityManager.h

@@ -543,11 +543,28 @@ 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 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.
+     *
+     * @return BLE_ERROR_NONE or appropriate error code indicating the failure reason.
+     */
+    ble_error_t writeBondingStateToPersistentStorage();
+
     ////////////////////////////////////////////////////////////////////////////
     // List management
     //

connectivity/FEATURE_BLE/source/SecurityManager.cpp

@@ -47,6 +47,15 @@ ble_error_t SecurityManager::preserveBondingStateOnReset(bool enable)
     return impl->preserveBondingStateOnReset(enable);
 }
 
+ble_error_t SecurityManager::writeBondingStateToPersistentStorage()
+{
+    ble_error_t err = impl->preserveBondingStateOnReset(true);
+    if (err) {
+        return err;
+    }
+    return impl->writeBondingStateToPersistentStorage();
+}
+
 ble_error_t SecurityManager::purgeAllBondingState()
 {
     return impl->purgeAllBondingState();

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;

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);
 

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;

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);
 

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<void(entry_handle_t, const SecurityEntryKeys_t*)>
@@ -520,9 +522,13 @@ 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);
+    virtual void sync(entry_handle_t db_handle = invalid_entry_handle);
 
     /**
      * Toggle whether values should be preserved across resets.

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
 //