Make GD LeSecurityTest pass on real devices

This patch contain couple fixes that stabilize LeSecurityTest
When running with --host flag, the virtual execution environment is much
more stable, but on real devices more guarantees around address handling
and disconnection are needed.

Bug: 155399771
Test: cert/run LeSecurityTest
Tag: #gd-refactor
Change-Id: I1f2d7e58600168140009458a2d31509edc19e8c4

diff --git a/gd/hci/le_advertising_manager.cc b/gd/hci/le_advertising_manager.cc
index 59bfd26..832831b 100644 (file)
--- a/gd/hci/le_advertising_manager.cc
+++ b/gd/hci/le_advertising_manager.cc
@@ -324,11 +324,16 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb
           module_handler_->BindOnce(impl::check_status<LeSetExtendedAdvertisingParametersCompleteView>));
     }
 
-    advertising_sets_[id].current_address = le_address_manager_->GetAnotherAddress();
-    le_advertising_interface_->EnqueueCommand(
-        hci::LeSetExtendedAdvertisingRandomAddressBuilder::Create(
-            id, advertising_sets_[id].current_address.GetAddress()),
-        module_handler_->BindOnce(impl::check_status<LeSetExtendedAdvertisingRandomAddressCompleteView>));
+    if (config.own_address_type == OwnAddressType::RANDOM_DEVICE_ADDRESS) {
+      advertising_sets_[id].current_address = le_address_manager_->GetAnotherAddress();
+      le_advertising_interface_->EnqueueCommand(
+          hci::LeSetExtendedAdvertisingRandomAddressBuilder::Create(
+              id, advertising_sets_[id].current_address.GetAddress()),
+          module_handler_->BindOnce(impl::check_status<LeSetExtendedAdvertisingRandomAddressCompleteView>));
+    } else {
+      advertising_sets_[id].current_address =
+          AddressWithType(controller_->GetMacAddress(), AddressType::PUBLIC_DEVICE_ADDRESS);
+    }
     if (!config.scan_response.empty()) {
       le_advertising_interface_->EnqueueCommand(
           hci::LeSetExtendedAdvertisingScanResponseBuilder::Create(id, config.operation, config.fragment_preference,
@@ -461,7 +466,7 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb
   }
 
   common::Callback<void(Address, AddressType)> scan_callback_;
-  common::ContextualCallback<void(ErrorCode, uint16_t, hci::AddressWithType)> set_terminated_callback_;
+  common::ContextualCallback<void(ErrorCode, uint16_t, hci::AddressWithType)> set_terminated_callback_{};
   os::Handler* registered_handler_{nullptr};
   Module* module_;
   os::Handler* module_handler_;

diff --git a/gd/security/cert/le_security_test.py b/gd/security/cert/le_security_test.py
index 43256ad..2ecedae 100644 (file)
--- a/gd/security/cert/le_security_test.py
+++ b/gd/security/cert/le_security_test.py
@@ -72,16 +72,20 @@ class LeSecurityTest(GdBaseTestClass):
         self.cert_security = PyLeSecurity(self.cert)
         self.dut_hci = PyHci(self.dut)
 
+        raw_addr = self.dut.hci_controller.GetMacAddress(empty_proto.Empty()).address
+
         self.dut_address = common.BluetoothAddressWithType(
-            address=common.BluetoothAddress(address=bytes(b'DD:05:04:03:02:01')), type=common.RANDOM_DEVICE_ADDRESS)
+            address=common.BluetoothAddress(address=raw_addr), type=common.PUBLIC_DEVICE_ADDRESS)
         privacy_policy = le_initiator_address_facade.PrivacyPolicy(
-            address_policy=le_initiator_address_facade.AddressPolicy.USE_STATIC_ADDRESS,
+            address_policy=le_initiator_address_facade.AddressPolicy.USE_PUBLIC_ADDRESS,
             address_with_type=self.dut_address)
         self.dut.security.SetLeInitiatorAddressPolicy(privacy_policy)
         self.cert_address = common.BluetoothAddressWithType(
-            address=common.BluetoothAddress(address=bytes(b'C5:11:FF:AA:33:22')), type=common.RANDOM_DEVICE_ADDRESS)
+            address=common.BluetoothAddress(
+                address=self.cert.hci_controller.GetMacAddress(empty_proto.Empty()).address),
+            type=common.PUBLIC_DEVICE_ADDRESS)
         cert_privacy_policy = le_initiator_address_facade.PrivacyPolicy(
-            address_policy=le_initiator_address_facade.AddressPolicy.USE_STATIC_ADDRESS,
+            address_policy=le_initiator_address_facade.AddressPolicy.USE_PUBLIC_ADDRESS,
             address_with_type=self.cert_address)
         self.cert.security.SetLeInitiatorAddressPolicy(cert_privacy_policy)
 
@@ -102,7 +106,7 @@ class LeSecurityTest(GdBaseTestClass):
             interval_min=512,
             interval_max=768,
             event_type=le_advertising_facade.AdvertisingEventType.ADV_IND,
-            address_type=common.RANDOM_DEVICE_ADDRESS,
+            address_type=self.cert_address.type,
             channel_map=7,
             filter_policy=le_advertising_facade.AdvertisingFilterPolicy.ALL_DEVICES)
         request = le_advertising_facade.CreateAdvertiserRequest(config=config)
@@ -119,7 +123,7 @@ class LeSecurityTest(GdBaseTestClass):
             interval_min=512,
             interval_max=768,
             event_type=le_advertising_facade.AdvertisingEventType.ADV_IND,
-            address_type=common.RANDOM_DEVICE_ADDRESS,
+            address_type=self.dut_address.type,
             channel_map=7,
             filter_policy=le_advertising_facade.AdvertisingFilterPolicy.ALL_DEVICES)
         request = le_advertising_facade.CreateAdvertiserRequest(config=config)
@@ -703,6 +707,7 @@ class LeSecurityTest(GdBaseTestClass):
             assertThat(self.dut_security.get_bond_stream()).emits(SecurityMatchers.BondMsg(BondMsgType.DEVICE_UNBONDED))
 
             self.dut_security.wait_device_disconnect(self.cert_address)
+            self.cert_security.wait_device_disconnect(self.dut_address)
 
     @metadata(
         pts_test_id="SM/SLA/SCOB/BV-02-C", pts_test_name="Out of Band, IUT Responder, Secure Connections – Success")
@@ -767,6 +772,7 @@ class LeSecurityTest(GdBaseTestClass):
 
             assertThat(self.dut_security.get_bond_stream()).emits(SecurityMatchers.BondMsg(BondMsgType.DEVICE_UNBONDED))
 
+            self.cert_security.wait_device_disconnect(self.dut_address)
             self.dut_security.wait_device_disconnect(self.cert_address)
 
     @metadata(
@@ -837,6 +843,7 @@ class LeSecurityTest(GdBaseTestClass):
             assertThat(self.dut_security.get_bond_stream()).emits(SecurityMatchers.BondMsg(BondMsgType.DEVICE_UNBONDED))
 
             self.dut_security.wait_device_disconnect(self.cert_address)
+            self.cert_security.wait_device_disconnect(self.dut_address)
 
     @metadata(
         pts_test_id="SM/MAS/SCOB/BV-04-C",
@@ -906,3 +913,4 @@ class LeSecurityTest(GdBaseTestClass):
             assertThat(self.dut_security.get_bond_stream()).emits(SecurityMatchers.BondMsg(BondMsgType.DEVICE_UNBONDED))
 
             self.dut_security.wait_device_disconnect(self.cert_address)
+            self.cert_security.wait_device_disconnect(self.dut_address)

diff --git a/gd/security/facade.cc b/gd/security/facade.cc
index e5541fe..41929ba 100644 (file)
--- a/gd/security/facade.cc
+++ b/gd/security/facade.cc
@@ -221,7 +221,8 @@ class SecurityModuleFacadeService : public SecurityModuleFacade::Service, public
     Address address = Address::kEmpty;
     hci::LeAddressManager::AddressPolicy address_policy =
         static_cast<hci::LeAddressManager::AddressPolicy>(request->address_policy());
-    if (address_policy == hci::LeAddressManager::AddressPolicy::USE_STATIC_ADDRESS) {
+    if (address_policy == hci::LeAddressManager::AddressPolicy::USE_STATIC_ADDRESS ||
+        address_policy == hci::LeAddressManager::AddressPolicy::USE_PUBLIC_ADDRESS) {
       ASSERT(Address::FromString(request->address_with_type().address().address(), address));
     }
     hci::AddressWithType address_with_type(address, static_cast<hci::AddressType>(request->address_with_type().type()));

diff --git a/gd/security/pairing_handler_le.h b/gd/security/pairing_handler_le.h
index 542187e..2c64922 100644 (file)
--- a/gd/security/pairing_handler_le.h
+++ b/gd/security/pairing_handler_le.h
@@ -336,6 +336,32 @@ class PairingHandlerLe {
 
   std::optional<PairingEvent> WaitUiPasskey() {
     PairingEvent e = WaitForEvent();
+
+    // It's possible to receive PAIRING_CONFIRM from remote device while waiting for the passkey.
+    // Store it until it's needed.
+    if (e.type == PairingEvent::L2CAP) {
+      auto l2cap_packet = e.l2cap_packet.value();
+      if (!l2cap_packet.IsValid()) {
+        LOG_WARN("Malformed L2CAP packet received!");
+        return std::nullopt;
+      }
+
+      const auto& received_code = l2cap_packet.GetCode();
+      if (received_code != Code::PAIRING_CONFIRM) {
+        LOG_WARN("Was waiting for passkey, received bad packet instead!");
+        return std::nullopt;
+      }
+
+      auto pkt = PairingConfirmView::Create(l2cap_packet);
+      if (!pkt.IsValid()) {
+        LOG_WARN("Malformed PAIRING_CONFIRM packet");
+        return std::nullopt;
+      }
+
+      cached_pariring_confirm_view = std::make_unique<PairingConfirmView>(pkt);
+      e = WaitForEvent();
+    }
+
     if (e.type == PairingEvent::UI & e.ui_action == PairingEvent::PASSKEY) {
       return e;
     } else {
@@ -455,7 +481,12 @@ class PairingHandlerLe {
     return WaitPacket<Code::PAIRING_RESPONSE>();
   }
 
-  auto WaitPairingConfirm() {
+  std::variant<bluetooth::security::PairingConfirmView, bluetooth::security::PairingFailure> WaitPairingConfirm() {
+    if (cached_pariring_confirm_view) {
+      PairingConfirmView pkt = *cached_pariring_confirm_view;
+      cached_pariring_confirm_view.release();
+      return pkt;
+    }
     return WaitPacket<Code::PAIRING_CONFIRM>();
   }
 
@@ -507,6 +538,9 @@ class PairingHandlerLe {
   std::queue<PairingEvent> queue;
 
   std::thread thread_;
+
+  // holds pairing_confirm, if received out of order
+  std::unique_ptr<PairingConfirmView> cached_pariring_confirm_view;
 };
 }  // namespace security
 }  // namespace bluetooth