From c2f38ad4f9d67a16484085bdfa758f01b418044c Mon Sep 17 00:00:00 2001 From: Bartosz Cichocki Date: Fri, 6 May 2022 12:32:54 +0200 Subject: [PATCH] [MOS-452] Fix BT keys duplicates Due to possibility to have duplicated entries per BT address BT stack was requesting pairing when wrong key has been provided. Now, all previous key entries are deleted prior adding. --- module-bluetooth/Bluetooth/BtKeysStorage.cpp | 54 +++++--- module-bluetooth/Bluetooth/BtKeysStorage.hpp | 5 +- module-bluetooth/tests/CMakeLists.txt | 4 + .../tests/tests-BTKeysStorage.cpp | 127 ++++++++++++++++++ .../service-bluetooth/SettingsHolder.hpp | 8 +- 5 files changed, 177 insertions(+), 21 deletions(-) create mode 100644 module-bluetooth/tests/tests-BTKeysStorage.cpp diff --git a/module-bluetooth/Bluetooth/BtKeysStorage.cpp b/module-bluetooth/Bluetooth/BtKeysStorage.cpp index d7276d27272cb755360ddc795fdafbd4db5b581d..6603840a63ce754b702f47ed2a2505893e0c945c 100644 --- a/module-bluetooth/Bluetooth/BtKeysStorage.cpp +++ b/module-bluetooth/Bluetooth/BtKeysStorage.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #include @@ -8,7 +8,6 @@ json11::Json bluetooth::KeyStorage::keysJson = json11::Json(); btstack_link_key_db_t bluetooth::KeyStorage::keyStorage; json11::Json::array bluetooth::KeyStorage::keys; -std::string bluetooth::KeyStorage::keysEntry; std::shared_ptr bluetooth::KeyStorage::settings = nullptr; namespace bluetooth @@ -39,6 +38,7 @@ namespace bluetooth void KeyStorage::openStorage() { LOG_INFO("opening storage from API"); + std::string keysEntry; if (settings) { keysEntry = std::visit(bluetooth::StringVisitor(), settings->getValue(bluetooth::Settings::BtKeys)); } @@ -71,33 +71,45 @@ namespace bluetooth { if (type != nullptr && bd_addr != nullptr) { LOG_INFO("getting key from API"); + + json11::Json finalJson = json11::Json::object{{strings::keys, keys}}; + auto keysEntryDump = finalJson.dump(); + LOG_INFO("Current keys state: %s", keysEntryDump.c_str()); + if (keys.empty()) { LOG_ERROR("Keys empty!"); return 0; } - for (auto key : keys) { - bd_addr_t addr; - sscanf_bd_addr(key[strings::bd_addr].string_value().c_str(), addr); - - if (bd_addr_cmp(addr, bd_addr) == 0) { - auto foundLinkKey = key[strings::link_key].string_value().c_str(); - memcpy(link_key, reinterpret_cast(foundLinkKey), sizeof(link_key_t)); - LOG_INFO("Getting key: %s", foundLinkKey); - *type = static_cast(key[strings::type].int_value()); - - return 1; - } + + auto ret = processOnFoundKey(bd_addr, [&type, &link_key](const json11::Json &key, bd_addr_t) { + auto foundLinkKey = key[strings::link_key].string_value().c_str(); + memcpy(link_key, reinterpret_cast(foundLinkKey), sizeof(link_key_t)); + LOG_INFO("Getting key: %s", foundLinkKey); + *type = static_cast(key[strings::type].int_value()); + + return 1; + }); + if (ret == 0) { LOG_ERROR("Can't find key for this address!"); } + return ret; } return 0; } void KeyStorage::putLinkKey(uint8_t *bd_addr, uint8_t *link_key, link_key_type_t type) { + + processOnFoundKey(bd_addr, [](const json11::Json &, bd_addr_t bd_addr) { + LOG_ERROR("Key already found in the key storage - deleting!"); + deleteLinkKey(bd_addr); + return 1; + }); + auto keyEntry = json11::Json::object{{strings::bd_addr, bd_addr_to_str(bd_addr)}, {strings::link_key, std::string(reinterpret_cast(link_key))}, {strings::type, type}}; keys.emplace_back(keyEntry); + if (settings->onLinkKeyAdded) { settings->onLinkKeyAdded(bd_addr_to_str(bd_addr)); } @@ -140,7 +152,7 @@ namespace bluetooth void KeyStorage::writeSettings() { json11::Json finalJson = json11::Json::object{{strings::keys, keys}}; - keysEntry = finalJson.dump(); + auto keysEntry = finalJson.dump(); if (settings) { settings->setValue(bluetooth::Settings::BtKeys, keysEntry); } @@ -149,5 +161,17 @@ namespace bluetooth return; } } + auto KeyStorage::processOnFoundKey(bd_addr_t bd_addr, const std::function &fun) -> int + { + for (const auto &key : keys) { + bd_addr_t addr; + sscanf_bd_addr(key[strings::bd_addr].string_value().c_str(), addr); + + if (bd_addr_cmp(addr, bd_addr) == 0 && fun) { + return fun(key, bd_addr); + } + } + return 0; + } } // namespace bluetooth diff --git a/module-bluetooth/Bluetooth/BtKeysStorage.hpp b/module-bluetooth/Bluetooth/BtKeysStorage.hpp index 089222ac0260a195dd997f93be3b94c6ade438d2..c2e6c728fd1388c55243429d277bd7eafb22e427 100644 --- a/module-bluetooth/Bluetooth/BtKeysStorage.hpp +++ b/module-bluetooth/Bluetooth/BtKeysStorage.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #pragma once @@ -17,7 +17,6 @@ namespace bluetooth public: static auto getKeyStorage() -> btstack_link_key_db_t *; static std::shared_ptr settings; - private: static void openStorage(); static void closeStorage(); @@ -31,11 +30,11 @@ namespace bluetooth link_key_type_t *type) -> int; static void iterator_done(btstack_link_key_iterator_t *it); static void set_local_bd_addr(bd_addr_t bd_addr); + static auto processOnFoundKey(bd_addr_t bd_addr, const std::function &fun) -> int; static void writeSettings(); static json11::Json keysJson; static btstack_link_key_db_t keyStorage; static json11::Json::array keys; - static std::string keysEntry; }; } // namespace Bt diff --git a/module-bluetooth/tests/CMakeLists.txt b/module-bluetooth/tests/CMakeLists.txt index e8f77e5bb3a79595659bf202fed448a518e35872..959a281253179455b0ca6b0568c2382f7490858a 100644 --- a/module-bluetooth/tests/CMakeLists.txt +++ b/module-bluetooth/tests/CMakeLists.txt @@ -6,7 +6,11 @@ add_catch2_executable( tests-BluetoothDevicesModel.cpp tests-Devicei.cpp tests-command.cpp + tests-BTKeysStorage.cpp LIBS module-sys module-bluetooth + service-bluetooth ) + + diff --git a/module-bluetooth/tests/tests-BTKeysStorage.cpp b/module-bluetooth/tests/tests-BTKeysStorage.cpp new file mode 100644 index 0000000000000000000000000000000000000000..2bcca0279d56e662b9797c8b5b6d416106c0087a --- /dev/null +++ b/module-bluetooth/tests/tests-BTKeysStorage.cpp @@ -0,0 +1,127 @@ +// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. +// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md + +#include +#include +#include + +namespace bluetooth +{ + class TestSettingsHolder : public SettingsHolder + { + public: + std::string entryValue; + Settings settingType; + auto getValue(const Settings setting) -> SettingEntry override + { + if (setting == settingType) { + return entryValue; + } + else { + return std::string{}; + } + } + void setValue(const Settings &newSetting, const SettingEntry &value) override + { + settingType = newSetting; + entryValue = std::get(value); + } + }; + +} // namespace bluetooth + +using namespace bluetooth; + +TEST_CASE("BT Keys storage") +{ + KeyStorage storage; + auto settingHolder = std::make_shared(); + + storage.settings = settingHolder; + + auto btKeyStorage = storage.getKeyStorage(); + SECTION("put key to the storage") + { + bd_addr_t addr; + sscanf_bd_addr("94:B2:CC:F1:92:B1", addr); + link_key_t key = "12345"; + link_key_type_t type = COMBINATION_KEY; + btKeyStorage->put_link_key(addr, key, type); + + REQUIRE(settingHolder->settingType == Settings::BtKeys); + REQUIRE(settingHolder->entryValue == + "{\"keys\": [{\"bd_addr\": \"94:B2:CC:F1:92:B1\", \"link_key\": \"12345\", \"type\": 0}]}"); + } + SECTION("get key from the storage") + { + bd_addr_t addr; + sscanf_bd_addr("94:B2:CC:F1:92:B1", addr); + link_key_t key; + link_key_type_t type; + settingHolder->entryValue = + "{\"keys\": [{\"bd_addr\": \"94:B2:CC:F1:92:B1\", \"link_key\": \"12345\", \"type\": 0}]}"; + + btKeyStorage->open(); + auto ret = btKeyStorage->get_link_key(addr, key, &type); + + REQUIRE(std::string(reinterpret_cast(key)) == "12345"); + REQUIRE(type == COMBINATION_KEY); + REQUIRE(ret == 1); + } + SECTION("get key from the storage when there's no key") + { + bd_addr_t addr; + sscanf_bd_addr("94:B2:CC:F1:92:B1", addr); + link_key_t key; + link_key_type_t type; + settingHolder->entryValue = ""; + btKeyStorage->open(); + btKeyStorage->delete_link_key(addr); + auto ret = btKeyStorage->get_link_key(addr, key, &type); + + REQUIRE(ret != 1); + } + SECTION("put key to the storage few times") + { + bd_addr_t addr; + sscanf_bd_addr("94:B2:CC:F1:92:B1", addr); + link_key_t key = "12345"; + link_key_type_t type = COMBINATION_KEY; + + btKeyStorage->open(); + btKeyStorage->put_link_key(addr, key, type); + btKeyStorage->put_link_key(addr, key, type); + btKeyStorage->put_link_key(addr, key, type); + + REQUIRE(settingHolder->settingType == Settings::BtKeys); + REQUIRE(settingHolder->entryValue == + "{\"keys\": [{\"bd_addr\": \"94:B2:CC:F1:92:B1\", \"link_key\": \"12345\", \"type\": 0}]}"); + } + SECTION("delete key from the storage") + { + bd_addr_t addr; + sscanf_bd_addr("94:B2:CC:F1:92:B1", addr); + settingHolder->entryValue = + "{\"keys\": [{\"bd_addr\": \"94:B2:CC:F1:92:B1\", \"link_key\": \"12345\", \"type\": 0}]}"; + + btKeyStorage->open(); + btKeyStorage->delete_link_key(addr); + + REQUIRE(settingHolder->settingType == Settings::BtKeys); + REQUIRE(settingHolder->entryValue == "{\"keys\": []}"); + } + SECTION("double-delete key from the storage") + { + bd_addr_t addr; + sscanf_bd_addr("94:B2:CC:F1:92:B1", addr); + settingHolder->entryValue = + "{\"keys\": [{\"bd_addr\": \"94:B2:CC:F1:92:B1\", \"link_key\": \"12345\", \"type\": 0}]}"; + + btKeyStorage->open(); + btKeyStorage->delete_link_key(addr); + btKeyStorage->delete_link_key(addr); + + REQUIRE(settingHolder->settingType == Settings::BtKeys); + REQUIRE(settingHolder->entryValue == "{\"keys\": []}"); + } +} diff --git a/module-services/service-bluetooth/service-bluetooth/SettingsHolder.hpp b/module-services/service-bluetooth/service-bluetooth/SettingsHolder.hpp index 1666570fb80ac2c330cdc3cded977e7989e03324..8e134473d8b54c114df43bdd9ea11cda6f5fd523 100644 --- a/module-services/service-bluetooth/service-bluetooth/SettingsHolder.hpp +++ b/module-services/service-bluetooth/service-bluetooth/SettingsHolder.hpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2022, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #pragma once #include @@ -78,9 +78,11 @@ namespace bluetooth class SettingsHolder { public: + SettingsHolder() = default; + virtual ~SettingsHolder() = default; explicit SettingsHolder(std::unique_ptr settingsPtr); - auto getValue(const Settings setting) -> SettingEntry; - void setValue(const Settings &newSetting, const SettingEntry &value); + virtual auto getValue(const Settings setting) -> SettingEntry; + virtual void setValue(const Settings &newSetting, const SettingEntry &value); std::function onStateChange; std::function onLinkKeyAdded; void deinit();