Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b1d8f69a authored by Łukasz Rymanowski's avatar Łukasz Rymanowski
Browse files

csis: Fix clearing temporary device for group verification 

During set member pairing, there is additional verification of the SIRK
which happens after pairing.
This patch fixes possible issue which could lead to situation that
temporary device was not cleared properly

Bug: 294604632
Test: atest bluetooth_csis_test
Tag: #feature
Change-Id: I293d0a5b9c04001f974d76ee2140d52233b8e367
parent 7e0b6227

system/bta/csis/csis_client.cc

+64 −34
Original line number Diff line number Diff line
@@ -780,9 +780,25 @@ class CsisClientImpl : public CsisClient { 
      }



      csis_group->RemoveDevice(device->addr);



      if (csis_group->IsEmpty()) {

        RemoveCsisGroup(group_id);



        /* Remove cached candidate devices for group */

        devices_.erase(

            std::remove_if(devices_.begin(), devices_.end(),

                           [group_id](auto& dev) {

                             if (dev->GetNumberOfCsisInstances() == 0 &&

                                 dev->GetExpectedGroupIdMember() == group_id &&

                                 dev->GetPairingSirkReadFlag() == false) {

                               return true;

                             }



                             return false;

                           }),

            devices_.end());

      }



      device->RemoveCsisInstance(group_id);

    }
@@ -1259,8 +1275,6 @@ class CsisClientImpl : public CsisClient { 
     * truly is member of group.

     */

    device.get()->SetExpectedGroupIdMember(group_id);

    devices_.push_back(device);



    callbacks_->OnSetMemberAvailable(address,

                                     device.get()->GetExpectedGroupIdMember());

  }
@@ -2119,22 +2133,39 @@ class CsisClientImpl : public CsisClient { 
    }

  }



  void ReadSirkValue(tGATT_STATUS status, const RawAddress& address,

                     uint8_t sirk_type, Octet16& received_sirk) {

    if (status != GATT_SUCCESS) {

      LOG_INFO("Invalid member, can't read SIRK (status: %02x)", status);

  void SirkValueReadCompleteDuringPairing(tGATT_STATUS status,

                                          const RawAddress& address,

                                          uint8_t sirk_type,

                                          Octet16& received_sirk) {

    LOG_INFO("%s, status: 0x%02x", ADDRESS_TO_LOGGABLE_CSTR(address), status);



    auto device = FindDeviceByAddress(address);

    if (device == nullptr) {

      LOG_ERROR("Unknown device %s", ADDRESS_TO_LOGGABLE_CSTR(address));

      BTA_DmSirkConfirmDeviceReply(address, false);

      return;

    }



    auto device = FindDeviceByAddress(address);

    if (device == nullptr) {

      LOG_ERROR("Invalid SIRK value read for unknown device");

    auto group_id_to_join = device->GetExpectedGroupIdMember();

    device->SetPairingSirkReadFlag(false);



    /* Verify group still exist, if not it means user forget the group and

     * paring should be rejected.

     */

    auto csis_group = FindCsisGroup(group_id_to_join);

    if (!csis_group) {

      LOG_ERROR("Group %d removed during paring a set member",

                group_id_to_join);

      RemoveDevice(address);

      BTA_DmSirkConfirmDeviceReply(address, false);

      return;

    }



    LOG_DEBUG("%s, status: 0x%02x", ADDRESS_TO_LOGGABLE_CSTR(address), status);

    if (status != GATT_SUCCESS) {

      LOG_INFO("Invalid member, can't read SIRK (status: %02x)", status);

      BTA_DmSirkConfirmDeviceReply(address, false);

      return;

    }



    /* Verify if sirk is not all zeros */

    Octet16 zero{};
@@ -2148,30 +2179,11 @@ class CsisClientImpl : public CsisClient { 
    if (sirk_type == bluetooth::csis::kCsisSirkTypeEncrypted) {

      /* Decrypt encrypted SIRK */

      Octet16 sirk;

      sdf(device->addr, received_sirk, sirk);

      sdf(address, received_sirk, sirk);

      received_sirk = sirk;

    }



    /* SIRK is ready. Add device to the group */



    /* Now having SIRK we can decide if the device belongs to some group we

     * know or this is a new group

     */

    auto csis_group = FindCsisGroup(device->GetExpectedGroupIdMember());

    if (!csis_group) {

      LOG_ERROR("Expected group with ID: %d, isn't cached",

                device->GetExpectedGroupIdMember());

      return;

    }



    /* Device will be added to group when upper layer decides */

    RemoveDevice(device->addr);



    if (csis_group->IsSirkBelongsToGroup(received_sirk)) {

      LOG_INFO("Device %s, verified successfully by SIRK",

               ADDRESS_TO_LOGGABLE_CSTR(address));

      BTA_DmSirkConfirmDeviceReply(address, true);

    } else {

    if (!csis_group->IsSirkBelongsToGroup(received_sirk)) {

      /*

       * Joining member must join already existing group otherwise it means

       * that its SIRK is different. Device connection was triggered by RSI
@@ -2180,7 +2192,17 @@ class CsisClientImpl : public CsisClient { 
      LOG_ERROR("Joining device %s, does not match any existig group",

                ADDRESS_TO_LOGGABLE_CSTR(address));

      BTA_DmSirkConfirmDeviceReply(address, false);

      return;

    }



    LOG_INFO("Device %s, verified successfully by SIRK",

             ADDRESS_TO_LOGGABLE_CSTR(address));

    BTA_DmSirkConfirmDeviceReply(address, true);



    /* It was temporary device and we can remove it. When upper layer

     * decides to connect CSIS it will be added then

     */

    RemoveDevice(address);

  }



  void VerifySetMember(const RawAddress& address) {
@@ -2195,9 +2217,15 @@ class CsisClientImpl : public CsisClient { 
      return;

    }



    gatt_cl_read_sirk_req(address,

                          base::BindOnce(&CsisClientImpl::ReadSirkValue,

                                         base::Unretained(instance)));

    if (!gatt_cl_read_sirk_req(

            address,

            base::BindOnce(&CsisClientImpl::SirkValueReadCompleteDuringPairing,

                           weak_factory_.GetWeakPtr()))) {

      LOG_ERROR("Could not read SIKR of %s", ADDRESS_TO_LOGGABLE_CSTR(address));

      BTA_DmSirkConfirmDeviceReply(address, false);

      return;

    }

    device->SetPairingSirkReadFlag(true);

  }



  uint8_t gatt_if_;
@@ -2206,6 +2234,8 @@ class CsisClientImpl : public CsisClient { 
  std::list<std::shared_ptr<CsisGroup>> csis_groups_;

  DeviceGroups* dev_groups_;

  int discovering_group_ = bluetooth::groups::kGroupUnknown;



  base::WeakPtrFactory<CsisClientImpl> weak_factory_{this};

};



class DeviceGroupsCallbacksImpl : public DeviceGroupsCallbacks {

system/bta/csis/csis_types.h

+13 −5
Original line number Diff line number Diff line
@@ -272,17 +272,25 @@ class CsisDevice : public GattServiceDevice { 
  }



  void SetExpectedGroupIdMember(int group_id) {

    LOG_DEBUG("Expected Group ID: %d, for member: %s is set", group_id,

    LOG_INFO("Expected Group ID: %d, for member: %s is set", group_id,

             ADDRESS_TO_LOGGABLE_CSTR(addr));

    expected_group_id_member = group_id;

    expected_group_id_member_ = group_id;

  }



  inline int GetExpectedGroupIdMember() { return expected_group_id_member; }

  void SetPairingSirkReadFlag(bool flag) {

    LOG_INFO("Pairing flag for Group ID: %d, member: %s is set to %d",

             expected_group_id_member_, ADDRESS_TO_LOGGABLE_CSTR(addr), flag);

    pairing_sirk_read_flag_ = flag;

  }



  inline int GetExpectedGroupIdMember() { return expected_group_id_member_; }

  inline bool GetPairingSirkReadFlag() { return pairing_sirk_read_flag_; }



 private:

  /* Instances per start handle  */

  std::map<uint16_t, std::shared_ptr<CsisInstance>> csis_instances_;

  int expected_group_id_member = bluetooth::groups::kGroupUnknown;

  int expected_group_id_member_ = bluetooth::groups::kGroupUnknown;

  bool pairing_sirk_read_flag_ = false;

};



/*