[BLE] Bonding information is not saved with FileSecurityDb unless SecurityManager::reset() is called

[AGlass0fMilk]

Description of defect

Separating this issue conversion from #14654.

Existing discussion quoted below:

Again, I am encountering the issue where the FileSecurityDb never actually closes the file handle and thus the bonding information never gets flushed to flash...

I have tried both with an LFS instance and a FAT instance on an external SPIFBlockDevice.

This may need its own issue but I really think we need to refactor the FileSecurityDb to operate more like "open, modify, close". In some applications, there isn't always a nice shutdown procedure (eg: hard power off with a switch) and in this case you cannot leave file handles open. That's just asking for corruption.

@paul-szczepanek-arm I will work on a refactor that works for me in the short term and I can do a PR. Do you see any issue with opening and closing the file each time it needs to be modified? Was there a reason (eg: latency) that you chose to originally keep the file open until the database was destroyed?

Currently, the only way the database gets destroyed (and thus the file is closed) is when the Security Manager itself gets destroyed... which I think never ever happens since it is a static singleton (as far as I can see).

I guess it does get deleted in SecurityManager::reset... I'll see if this gets called during BLE shutdown.

EDIT: I cannot find anywhere that SecurityManager::reset gets called by the stack itself. This doesn't seem like a great thing to do during the course of normal BLE operation... I still think we should go to the "open, write, close" tactic with the FileSecurityDb.

EDIT 2:
I now realize why the one method is called preserveBondingStateOnReset but I'm not sure why you must reset the security manager to flush the file. Shouldn't we have some sort of flushBondingState or something that doesn't modify the state of the security manager?

Also, nowhere in the mbed-os-example-ble for security and privacy is SecurityManager::reset called, nor mentioned...

So I added the reset and reinitialization of the security manager upon disconnection and I can now pair with Android/iOS devices and have it persist between power cycles, great.

This definitely needs its own issue at this point, but I wanted to point out (after testing myself) that using the KVStoreSecurityDb, you don't need to call SecurityManager::reset for the bonding information to be saved... so this is an inconsistency in the API.

And here we are!

Sorry @pan- @paul-szczepanek-arm, I know this is probably sending you a bunch of emails after work... 👨‍💻

Target(s) affected by this defect ?

All BLE targets

Toolchain(s) (name and version) displaying this defect ?

arm-none-eabi-gcc (GNU Tools for Arm Embedded Processors 9-2019-q4-major) 9.2.1 20191025 (release) [ARM/arm-9-branch revision 277599]

What version of Mbed-os are you using (tag or sha) ?

mbed-os-99.99.99

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

Package             Version     Location
------------------- ----------- -------------------------------------------------------------------
aenum               3.0.0
appdirs             1.4.3
asn1ate             0.6.0
attrs               19.3.0
Automat             20.2.0
beautifulsoup4      4.6.3
ble-serial          2.0.0
bleak               0.11.0
bluepy              1.3.0
cbor                1.0.0
certifi             2019.11.28
cffi                1.14.1
chardet             3.0.4
click               7.1
cmsis-pack-manager  0.2.10
cobs                1.1.4
cogapp              3.0.0
colorama            0.3.9
coloredlogs         15.0
constantly          15.1.0
crc16               0.1.1
crccheck            0.6
cryptography        2.9.2
cycler              0.10.0
dataclasses         0.8
dbus-next           0.2.2
distlib             0.3.1
docopt              0.6.2
ecdsa               0.15
elftools            0.1.0.dev0
ep                  0.0.1       /home/gdbeckstein/Documents/embeddedplanet/ep-app-generator
fasteners           0.15
filelock            3.0.12
Flask               1.1.2
future              0.16.0
futures             3.1.1
fuzzywuzzy          0.18.0
gitdb               4.0.5
GitPython           3.1.13
grip                4.5.2
hidapi              0.9.0.post2
humanfriendly       9.1
hyperlink           20.0.1
icetea              1.2.4
idna                2.7
imgtool             1.7.0rc1
importlib-metadata  1.6.0
importlib-resources 5.1.2
incremental         17.5.0
inflection          0.5.1
iniconfig           1.1.1
intelhex            2.2.1
itsdangerous        1.1.0
Jinja2              2.10.3
jsonmerge           1.7.0
jsonschema          2.6.0
junit-xml           1.8
keyrings.alt        4.0.2
kiwisolver          1.2.0
lockfile            0.12.2
Logbook             1.5.3
Mako                1.1.4
manifest-tool       1.5.2
Markdown            3.3.2
MarkupSafe          1.1.1
matplotlib          3.3.0
mbed-ble-test-suite 0.0.1       /home/gdbeckstein/Documents/mbed-os-bluetooth-integration-testsuite
mbed-cli            1.10.4
mbed-cloud-sdk      2.0.8
mbed-flasher        0.10.1
mbed-greentea       1.7.4
mbed-host-tests     1.5.10
mbed-ls             1.7.12
mbed-os-tools       0.0.15
mbed-tools          7.1.2
milksnake           0.1.5
monotonic           1.5
numpy               1.19.1
packaging           20.4
path-and-address    2.0.1
pc-ble-driver-py    0.14.2
pdoc3               0.9.2
Pillow              7.2.0
pip                 20.2
pkg-resources       0.0.0
pluggy              0.13.1
prettytable         0.7.2
protobuf            3.5.2.post1
psutil              5.6.6
py                  1.9.0
pyasn1              0.2.3
pycparser           2.20
pycryptodome        3.9.8
pyelftools          0.25
Pygments            2.7.1
PyHamcrest          2.0.2
pyparsing           2.4.7
pyrsistent          0.16.0
pyserial            3.4
pytest              6.1.1
python-can          3.3.4
python-dateutil     2.8.1
python-dotenv       0.14.0
pyudev              0.22.0
pyusb               1.0.2
PyYAML              4.2b1
requests            2.20.1
semver              2.10.2
setuptools          46.1.3
six                 1.12.0
smmap               3.0.5
soupsieve           2.0
tabulate            0.8.9
toml                0.10.1
tqdm                4.57.0
trollius            2.1.post2
Twisted             20.3.0
txdbus              1.1.2
typing-extensions   3.7.4.3
urllib3             1.24.2
virtualenv          20.4.3
Werkzeug            1.0.1
wheel               0.34.2
wrapt               1.12.1
yattag              1.13.2
zipp                3.1.0
zope.interface      5.2.0

How is this defect reproduced ?

Attempt to use the mbed-os-ble-example for security and privacy with the filesystem support enabled. The pairing credentials will not be saved into flash because the example does not call SecurityManager::reset. This is misleading, but more importantly this operation is not desireable. See discussion above.

[ciarmcom]

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/IOTOSM-3983

[paul-szczepanek-arm]

#14824

[paul-szczepanek-arm]

This is now merged, I'm closing the issue, if there's still an problem please reopen.