fw/services/bluetooth: do not choke on gap_bondings of the wrong version

snowy uses v1 of bluetooth_persistent_storage, which NimBLE does not
support.  As it turns out, we are generally not very robust against
gap_bondings that we don't know how to deal with, and we can crash the
system if they happen!  Let's rather ignore them, in case a future PebbleOS
version extends the gap_bonding structure in a way that we don't know how to
read and someone tries to downgrade.

Signed-off-by: Joshua Wise <[email protected]>

Author: jwise

src/fw/services/normal/bluetooth/bluetooth_persistent_storage.c

@@ -50,18 +50,13 @@
 // Let the unittest define this using a header override:
 #  include "services/normal/bluetooth/bluetooth_persistent_storage_unittest_impl.h"
 #else
-// TODO: perhaps revert this back to v1 for cc2564x if we can figure out how to handle the old format
-// right now, you have to make sure you've erased all bondings before upgrading else you'll crash
-// because the v2 code chokes on the v1 format
 #  if BT_CONTROLLER_DA14681 || BT_CONTROLLER_QEMU || BT_CONTROLLER_NRF52 || BT_CONTROLLER_CC2564X
 #    include "services/normal/bluetooth/bluetooth_persistent_storage_v2_impl.h"
 #  else
 #    error "Unknown BT_CONTROLLER_... define?"
 #  endif
 #endif
 
-//! The BtPersistBonding*Data structs can never shrink, only grow
-
 //! Stores data about a remote BT classic device
 typedef struct PACKED {
   BTDeviceAddress addr;
@@ -126,6 +121,26 @@ static PebbleMutex *s_db_mutex = NULL;
 //! @note prv_lock() must be held when accessing this variable.
 static PebbleProtocolCapabilities s_cached_system_capabilities;
 
+// TODO: extend these to be able to load a legacy BtPersistBondingData from
+// v1 and migrate it, if we wish to.  The old format requires regenerating
+// CSRK / LTK from div/ediv directly, which seems not to be an API that's
+// readily exposed from NimBLE.  For now, we throw away old pairings.
+
+//! Is this val_len any kind of BtPersistBondingData that we support loading?
+static bool prv_val_len_is_bonding(size_t val_len) {
+  // In theory, it was permissible on old PebbleOS to have a smaller
+  // BtPersistBondingData v2 that needed to get zero-padded out to a larger
+  // one, and that was the "versioning"; this check does not check for that. 
+  // In practice, this never happened, so we are OK!
+  return val_len == sizeof(BtPersistBondingData);
+}
+
+//! Load whatever is in this SettingsRecordInfo into a modern BtPersistBondingData structure.
+static void prv_load_pairing_data_from_val(SettingsFile *file, SettingsRecordInfo *info, BtPersistBondingData *data) {
+  PBL_ASSERTN(info->val_len == sizeof(BtPersistBondingData));
+  info->get_val(file, (uint8_t *)data, info->val_len);
+}
+
 static void prv_lock(void) {
   mutex_lock(s_db_mutex);
 }
@@ -320,12 +335,12 @@ static bool prv_any_pinned_ble_pairings_itr(SettingsFile *file,
   if (info->key_len != sizeof(BTBondingID)) {
     return true;
   }
-  if (info->val_len == 0) {
+  if (!prv_val_len_is_bonding(info->val_len)) {
     return true;
   }
 
   BtPersistBondingData data;
-  info->get_val(file, &data, sizeof(data));
+  prv_load_pairing_data_from_val(file, info, &data);
   if (data.ble_data.requires_address_pinning) {
     bool *has_pinned_ble_pairings = context;
     *has_pinned_ble_pairings = true;
@@ -518,14 +533,14 @@ typedef struct {
 static bool prv_get_num_pairings_by_type_itr(SettingsFile *file,
                                              SettingsRecordInfo *info, void *context) {
   // check entry is valid
-  if (info->val_len == 0 || info->key_len != sizeof(BTBondingID)) {
+  if (!prv_val_len_is_bonding(info->val_len) || info->key_len != sizeof(BTBondingID)) {
     return true; // continue iterating
   }
 
   PairingCountItrData *itr_data = (PairingCountItrData *)context;
 
   BtPersistBondingData stored_data;
-  info->get_val(file, (uint8_t*) &stored_data, MIN((unsigned)info->val_len, sizeof(stored_data)));
+  prv_load_pairing_data_from_val(file, info, &stored_data);
 
   if (stored_data.type == itr_data->type) {
     itr_data->count++;
@@ -577,7 +592,7 @@ static bool prv_is_pairing_info_equal_identity(const BtPersistLEPairingInfo *a,
 static bool prv_get_key_for_sm_pairing_info_itr(SettingsFile *file,
                                                 SettingsRecordInfo *info, void *context) {
   // check entry is valid
-  if (info->val_len == 0 || info->key_len != sizeof(BTBondingID)) {
+  if (!prv_val_len_is_bonding(info->val_len) || info->key_len != sizeof(BTBondingID)) {
     return true; // continue iterating
   }
 
@@ -586,7 +601,7 @@ static bool prv_get_key_for_sm_pairing_info_itr(SettingsFile *file,
   BTBondingID key;
   BtPersistBondingData stored_data;
   info->get_key(file, (uint8_t*) &key, info->key_len);
-  info->get_val(file, (uint8_t*) &stored_data, MIN((unsigned)info->val_len, sizeof(stored_data)));
+  prv_load_pairing_data_from_val(file, info, &stored_data);
 
   if (stored_data.type == BtPersistBondingTypeBLE &&
       prv_is_pairing_info_equal_identity(&stored_data.ble_data.pairing_info,
@@ -752,7 +767,7 @@ typedef struct {
 static bool prv_find_by_addr_itr(SettingsFile *file,
                                  SettingsRecordInfo *info, void *context) {
   // check entry is valid
-  if (info->val_len == 0 || info->key_len != sizeof(BTBondingID)) {
+  if (!prv_val_len_is_bonding(info->val_len) || info->key_len != sizeof(BTBondingID)) {
     return true; // continue iterating
   }
 
@@ -761,7 +776,7 @@ static bool prv_find_by_addr_itr(SettingsFile *file,
   BTBondingID key;
   BtPersistBondingData stored_data;
   info->get_key(file, (uint8_t*) &key, info->key_len);
-  info->get_val(file, (uint8_t*) &stored_data, MIN((unsigned)info->val_len, sizeof(stored_data)));
+  prv_load_pairing_data_from_val(file, info, &stored_data);
 
   if (stored_data.type == BtPersistBondingTypeBLE &&
       bt_device_equal(&itr_data->device.opaque,
@@ -889,14 +904,14 @@ bool bt_persistent_storage_get_ble_pinned_address(BTDeviceAddress *address_out)
 static bool prv_get_first_ancs_bonding_itr(SettingsFile *file,
                                            SettingsRecordInfo *info, void *context) {
   // check entry is valid
-  if (info->val_len == 0 || info->key_len != sizeof(BTBondingID)) {
+  if (!prv_val_len_is_bonding(info->val_len) || info->key_len != sizeof(BTBondingID)) {
     return true; // continue iterating
   }
 
   BTBondingID *first_ancs_supported_bonding_found = (BTBondingID *) context;
 
   BtPersistBondingData stored_data;
-  info->get_val(file, (uint8_t*) &stored_data, MIN((unsigned)info->val_len, sizeof(stored_data)));
+  prv_load_pairing_data_from_val(file, info, &stored_data);
 
 
   if (stored_data.type == BtPersistBondingTypeBLE && stored_data.ble_data.supports_ancs) {
@@ -957,7 +972,7 @@ static void prv_public_for_each_ble_cb(BTBondingID key,
 static bool prv_ble_pairing_internal_for_each_itr(SettingsFile *file,
                                                   SettingsRecordInfo *info, void *context) {
   // check entry is valid
-  if (info->val_len == 0 || info->key_len != sizeof(BTBondingID)) {
+  if (!prv_val_len_is_bonding(info->val_len) || info->key_len != sizeof(BTBondingID)) {
     return true; // continue iterating
   }
 
@@ -966,7 +981,7 @@ static bool prv_ble_pairing_internal_for_each_itr(SettingsFile *file,
   BTBondingID key;
   BtPersistBondingData stored_data;
   info->get_key(file, (uint8_t*) &key, info->key_len);
-  info->get_val(file, (uint8_t*) &stored_data, MIN((unsigned)info->val_len, sizeof(stored_data)));
+  prv_load_pairing_data_from_val(file, info, &stored_data);
 
   if (stored_data.type == BtPersistBondingTypeBLE) {
     internal_itr_data->cb(key, &stored_data, internal_itr_data->cb_data);
@@ -1037,7 +1052,7 @@ typedef struct {
 static bool prv_get_key_for_bt_classic_addr_itr(SettingsFile *file,
                                                 SettingsRecordInfo *info, void *context) {
   // check entry is valid
-  if (info->val_len == 0 || info->key_len != sizeof(BTBondingID)) {
+  if (!prv_val_len_is_bonding(info->val_len) || info->key_len != sizeof(BTBondingID)) {
     return true; // continue iterating
   }
 
@@ -1046,7 +1061,7 @@ static bool prv_get_key_for_bt_classic_addr_itr(SettingsFile *file,
   BTBondingID key;
   BtPersistBondingData stored_data;
   info->get_key(file, (uint8_t*) &key, info->key_len);
-  info->get_val(file, (uint8_t*) &stored_data, MIN((unsigned)info->val_len, sizeof(stored_data)));
+  prv_load_pairing_data_from_val(file, info, &stored_data);
 
   if (stored_data.type == BtPersistBondingTypeBTClassic &&
       !memcmp(&itr_data->address, &stored_data.bt_classic_data.addr, sizeof(itr_data->address))) {
@@ -1222,7 +1237,7 @@ typedef struct {
 static bool bt_persistent_storage_bt_classic_pairing_for_each_itr(SettingsFile *file,
                                                         SettingsRecordInfo *info, void *context) {
   // check entry is valid
-  if (info->val_len == 0 || info->key_len != sizeof(BTBondingID)) {
+  if (!prv_val_len_is_bonding(info->val_len) || info->key_len != sizeof(BTBondingID)) {
     return true; // continue iterating
   }
 
@@ -1231,7 +1246,7 @@ static bool bt_persistent_storage_bt_classic_pairing_for_each_itr(SettingsFile *
   BTBondingID key;
   BtPersistBondingData stored_data;
   info->get_key(file, (uint8_t*) &key, info->key_len);
-  info->get_val(file, (uint8_t*) &stored_data, MIN((unsigned)info->val_len, sizeof(stored_data)));
+  prv_load_pairing_data_from_val(file, info, &stored_data);
 
   if (stored_data.type == BtPersistBondingTypeBTClassic) {
     itr_data->cb(&stored_data.bt_classic_data.addr, &stored_data.bt_classic_data.link_key,