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

Commit 61ef3734 authored by Hansong Zhang's avatar Hansong Zhang
Browse files

GD Security: Improve Enforce() workflow 

InternalEnforceSecurityPolicy establishes the requirement
- ENCRYPTED_TRANSPORT: If paired but not encrypted, just wait for
  encryption change; if unpaired, pair with NO_BOND_NO_MITM
- AUTHENTICATED_ENCRYPTED_TRANSPORT: Similar as above, but we need to
  pair again if existing LK is not authenticated. Exception: If no MITM
  is needed during pairing, we assume authenticated LK is not possible,
  so we allow connection. In the future, use IO cap to check.

When link is encrypted, or new pairing is complete, we invoke
UpdateLinkSecurityCondition.

Test: cert/run --host
Test: CtsVerifier Insecure RFCOMM client
Tag: #gd-refactor
Bug: 141555841
Change-Id: Ic5792c8e967cd068e08df4702393ae3188c6d4e8
parent f6c7b89f

system/gd/security/cert/security_test.py

+2 −12
Original line number Original line Diff line number Diff line
@@ -147,7 +147,7 @@ class SecurityTest(GdBaseTestClass): 
        pass

        pass





    # no_input_no_output + no_input_no_output is JustWorks no confirmation

    # no_input_no_output + no_input_no_output is JustWorks no confirmation

    def test_dut_initiated_no_input_no_output_no_input_no_output_twice_same_acl(self):

    def test_dut_initiated_no_input_no_output_no_input_no_output_twice_bond_and_enforce(self):

        # Arrange

        # Arrange

        self.dut_security.set_io_capabilities(IoCapabilities.NO_INPUT_NO_OUTPUT)

        self.dut_security.set_io_capabilities(IoCapabilities.NO_INPUT_NO_OUTPUT)

        self.dut_security.set_authentication_requirements(AuthenticationRequirements.DEDICATED_BONDING)

        self.dut_security.set_authentication_requirements(AuthenticationRequirements.DEDICATED_BONDING)
@@ -171,17 +171,7 @@ class SecurityTest(GdBaseTestClass): 
                                                  common.BluetoothAddressTypeEnum.PUBLIC_DEVICE_ADDRESS,

                                                  common.BluetoothAddressTypeEnum.PUBLIC_DEVICE_ADDRESS,

                                                  ClassicSecurityPolicy.ENCRYPTED_TRANSPORT)

                                                  ClassicSecurityPolicy.ENCRYPTED_TRANSPORT)





        self._verify_ssp_numeric_comparison(

        # TODO: We verify enforcement when we make sure EncryptionChange is received on DUT

            initiator=self.dut_security,

            responder=self.cert_security,

            init_ui_response=True,

            resp_ui_response=True,

            expected_init_ui_event=None,

            expected_resp_ui_event=None,

            expected_init_bond_event=BondMsgType.DEVICE_BONDED,

            expected_resp_bond_event=None)



        self.dut_security.wait_for_enforce_security_event(expected_enforce_security_event=False)





    # no_input_no_output + no_input_no_output is JustWorks no confirmation

    # no_input_no_output + no_input_no_output is JustWorks no confirmation

    def test_dut_initiated_no_input_no_output_no_input_no_output_twice_with_remove_bond(self):

    def test_dut_initiated_no_input_no_output_no_input_no_output_twice_with_remove_bond(self):

system/gd/security/internal/security_manager_impl.cc

+84 −41
Original line number Original line Diff line number Diff line
@@ -334,11 +334,7 @@ void SecurityManagerImpl::OnEncryptionChange(hci::Address address, bool encrypte 
  auto remote = hci::AddressWithType(address, hci::AddressType::PUBLIC_DEVICE_ADDRESS);

  auto remote = hci::AddressWithType(address, hci::AddressType::PUBLIC_DEVICE_ADDRESS);

  auto record = security_database_.FindOrCreate(remote);

  auto record = security_database_.FindOrCreate(remote);

  record->SetIsEncrypted(encrypted);

  record->SetIsEncrypted(encrypted);

  auto cb_entry = enforce_security_policy_callback_map_.find(remote);

  UpdateLinkSecurityCondition(remote);

  if (cb_entry != enforce_security_policy_callback_map_.end()) {

    this->InternalEnforceSecurityPolicy(remote, cb_entry->second.first, std::move(cb_entry->second.second), false);

    enforce_security_policy_callback_map_.erase(cb_entry);

  }

}

}





void SecurityManagerImpl::OnHciLeEvent(hci::LeMetaEventView event) {

void SecurityManagerImpl::OnHciLeEvent(hci::LeMetaEventView event) {
@@ -388,11 +384,6 @@ void SecurityManagerImpl::OnPairingHandlerComplete(hci::Address address, Pairing 
    security_manager_channel_->Release(address);

    security_manager_channel_->Release(address);

  }

  }

  auto remote = hci::AddressWithType(address, hci::AddressType::PUBLIC_DEVICE_ADDRESS);

  auto remote = hci::AddressWithType(address, hci::AddressType::PUBLIC_DEVICE_ADDRESS);

  auto cb_entry = enforce_security_policy_callback_map_.find(remote);

  if (cb_entry != enforce_security_policy_callback_map_.end()) {

    this->InternalEnforceSecurityPolicy(remote, cb_entry->second.first, std::move(cb_entry->second.second), false);

    enforce_security_policy_callback_map_.erase(cb_entry);

  }

  if (!std::holds_alternative<PairingFailure>(status)) {

  if (!std::holds_alternative<PairingFailure>(status)) {

    NotifyDeviceBonded(remote);

    NotifyDeviceBonded(remote);

  } else {

  } else {
@@ -401,6 +392,7 @@ void SecurityManagerImpl::OnPairingHandlerComplete(hci::Address address, Pairing 
  auto record = this->security_database_.FindOrCreate(remote);

  auto record = this->security_database_.FindOrCreate(remote);

  record->CancelPairing();

  record->CancelPairing();

  security_database_.SaveRecordsToStorage();

  security_database_.SaveRecordsToStorage();

  UpdateLinkSecurityCondition(remote);

}

}





void SecurityManagerImpl::OnL2capRegistrationCompleteLe(

void SecurityManagerImpl::OnL2capRegistrationCompleteLe(
@@ -713,36 +705,57 @@ void SecurityManagerImpl::SetOobDataPresent(hci::OobDataPresent data_present) { 
void SecurityManagerImpl::InternalEnforceSecurityPolicy(

void SecurityManagerImpl::InternalEnforceSecurityPolicy(

    hci::AddressWithType remote,

    hci::AddressWithType remote,

    l2cap::classic::SecurityPolicy policy,

    l2cap::classic::SecurityPolicy policy,

    l2cap::classic::SecurityEnforcementInterface::ResultCallback result_callback,

    l2cap::classic::SecurityEnforcementInterface::ResultCallback result_callback) {

    bool try_meet_requirements) {

  if (IsSecurityRequirementSatisfied(remote, policy)) {

    // Notify client immediately if already satisfied

    result_callback.Invoke(true);

    return;

  }



  hci::AuthenticationRequirements authentication_requirements = kDefaultAuthenticationRequirements;

  hci::AuthenticationRequirements authentication_requirements = kDefaultAuthenticationRequirements;





  bool result = false;

  auto record = this->security_database_.FindOrCreate(remote);

  auto record = this->security_database_.FindOrCreate(remote);

  bool need_to_pair = false;



  switch (policy) {

  switch (policy) {

    case l2cap::classic::SecurityPolicy::BEST:

    case l2cap::classic::SecurityPolicy::BEST:

    case l2cap::classic::SecurityPolicy::AUTHENTICATED_ENCRYPTED_TRANSPORT:

    case l2cap::classic::SecurityPolicy::AUTHENTICATED_ENCRYPTED_TRANSPORT:

      result = record->IsAuthenticated() && record->RequiresMitmProtection() && record->IsEncrypted();

      if (!record->IsPaired()) {

        need_to_pair = true;

      } else if (record->IsAuthenticated()) {

        // if paired with MITM, only encryption is missing, so we just need to wait for encryption change callback

      } else {

        // We have an unauthenticated link key, so we need to pair again with MITM

        // Need to pair again with MITM

        need_to_pair = true;

        authentication_requirements = hci::AuthenticationRequirements::GENERAL_BONDING_MITM_PROTECTION;

        authentication_requirements = hci::AuthenticationRequirements::GENERAL_BONDING_MITM_PROTECTION;

        if (record->RequiresMitmProtection()) {

          // Workaround for headset: If no MITM is needed during pairing, we don't mandate authenticated LK

          // TODO(b/165671060): Use IO cap to check whether we can waive authenticated LK requirement

          need_to_pair = false;

        }

      }

      break;

      break;

    case l2cap::classic::SecurityPolicy::ENCRYPTED_TRANSPORT:

    case l2cap::classic::SecurityPolicy::ENCRYPTED_TRANSPORT:

      result = record->IsEncrypted();

      if (!record->IsPaired()) {

        need_to_pair = true;

        authentication_requirements = hci::AuthenticationRequirements::NO_BONDING;

        authentication_requirements = hci::AuthenticationRequirements::NO_BONDING;

      } else {

        // just need to wait for encryption change callback

      }

      break;

      break;

    case l2cap::classic::SecurityPolicy::_SDP_ONLY_NO_SECURITY_WHATSOEVER_PLAINTEXT_TRANSPORT_OK:

    default:

      result = true;

      return;

      break;

  }

  }

  if (!result && try_meet_requirements) {



  auto entry = enforce_security_policy_callback_map_.find(remote);

  auto entry = enforce_security_policy_callback_map_.find(remote);

  if (entry != enforce_security_policy_callback_map_.end()) {

  if (entry != enforce_security_policy_callback_map_.end()) {

    LOG_WARN("Callback already pending for remote: '%s' !", remote.ToString().c_str());

    LOG_WARN("Callback already pending for remote: '%s' !", remote.ToString().c_str());

  } else {

  } else {

      enforce_security_policy_callback_map_.emplace(

    enforce_security_policy_callback_map_[remote] = {policy, std::move(result_callback)};

          remote,

  }

          std::pair<l2cap::classic::SecurityPolicy, l2cap::classic::SecurityEnforcementInterface::ResultCallback>(



              policy, std::move(result_callback)));

  if (need_to_pair && !record->IsPairing()) {

      if (!record->IsPairing()) {

    LOG_WARN("Dispatch #3");

    LOG_WARN("Dispatch #3");

    DispatchPairingHandler(

    DispatchPairingHandler(

        record,

        record,
@@ -752,16 +765,46 @@ void SecurityManagerImpl::InternalEnforceSecurityPolicy( 
        std::as_const(authentication_requirements));

        std::as_const(authentication_requirements));

  }

  }

}

}



void SecurityManagerImpl::UpdateLinkSecurityCondition(hci::AddressWithType remote) {

  auto record = this->security_database_.FindOrCreate(remote);

  auto entry = enforce_security_policy_callback_map_.find(remote);

  if (entry == enforce_security_policy_callback_map_.end()) {

    LOG_ERROR("No security reuest pending for %s", remote.ToString().c_str());

    return;

    return;

  }

  }

  result_callback.Invoke(result);



  auto policy = entry->second.policy_;



  if (IsSecurityRequirementSatisfied(remote, policy)) {

    entry->second.callback_.Invoke(true);

    enforce_security_policy_callback_map_.erase(entry);

  }

}



bool SecurityManagerImpl::IsSecurityRequirementSatisfied(

    hci::AddressWithType remote, l2cap::classic::SecurityPolicy policy) {

  auto record = security_database_.FindOrCreate(remote);



  switch (policy) {

    case l2cap::classic::SecurityPolicy::BEST:

    case l2cap::classic::SecurityPolicy::AUTHENTICATED_ENCRYPTED_TRANSPORT:

      // TODO(b/165671060): Use IO cap to check whether we can waive authenticated LK requirement

      return (!record->RequiresMitmProtection() || record->IsAuthenticated()) && record->IsEncrypted();

    case l2cap::classic::SecurityPolicy::ENCRYPTED_TRANSPORT:

      return record->IsEncrypted();

    case l2cap::classic::SecurityPolicy::_SDP_ONLY_NO_SECURITY_WHATSOEVER_PLAINTEXT_TRANSPORT_OK:

      return true;

    default:

      return false;

  }

}

}





void SecurityManagerImpl::EnforceSecurityPolicy(

void SecurityManagerImpl::EnforceSecurityPolicy(

    hci::AddressWithType remote,

    hci::AddressWithType remote,

    l2cap::classic::SecurityPolicy policy,

    l2cap::classic::SecurityPolicy policy,

    l2cap::classic::SecurityEnforcementInterface::ResultCallback result_callback) {

    l2cap::classic::SecurityEnforcementInterface::ResultCallback result_callback) {

  this->InternalEnforceSecurityPolicy(remote, policy, std::move(result_callback), true);

  this->InternalEnforceSecurityPolicy(remote, policy, std::move(result_callback));

}

}





void SecurityManagerImpl::EnforceLeSecurityPolicy(

void SecurityManagerImpl::EnforceLeSecurityPolicy(

system/gd/security/internal/security_manager_impl.h

+8 −6
Original line number Original line Diff line number Diff line
@@ -224,8 +224,9 @@ class SecurityManagerImpl : public channel::ISecurityManagerChannelListener, pub 
  void InternalEnforceSecurityPolicy(

  void InternalEnforceSecurityPolicy(

      hci::AddressWithType remote,

      hci::AddressWithType remote,

      l2cap::classic::SecurityPolicy policy,

      l2cap::classic::SecurityPolicy policy,

      l2cap::classic::SecurityEnforcementInterface::ResultCallback result_callback,

      l2cap::classic::SecurityEnforcementInterface::ResultCallback result_callback);

      bool try_meet_requirements);

  void UpdateLinkSecurityCondition(hci::AddressWithType remote);

  bool IsSecurityRequirementSatisfied(hci::AddressWithType remote, l2cap::classic::SecurityPolicy policy);

  void ConnectionIsReadyStartPairing(LeFixedChannelEntry* stored_channel);

  void ConnectionIsReadyStartPairing(LeFixedChannelEntry* stored_channel);

  void WipeLePairingHandler();

  void WipeLePairingHandler();
@@ -251,10 +252,11 @@ class SecurityManagerImpl : public channel::ISecurityManagerChannelListener, pub 
  std::optional<crypto_toolbox::Octet16> remote_oob_data_le_sc_r_;

  std::optional<crypto_toolbox::Octet16> remote_oob_data_le_sc_r_;

  std::optional<FacadeDisconnectCallback> facade_disconnect_callback_;

  std::optional<FacadeDisconnectCallback> facade_disconnect_callback_;





  std::unordered_map<

  struct PendingSecurityEnforcementEntry {

      hci::AddressWithType,

    l2cap::classic::SecurityPolicy policy_;

      std::pair<l2cap::classic::SecurityPolicy, l2cap::classic::SecurityEnforcementInterface::ResultCallback>>

    l2cap::classic::SecurityEnforcementInterface::ResultCallback callback_;

      enforce_security_policy_callback_map_;

  };

  std::unordered_map<hci::AddressWithType, PendingSecurityEnforcementEntry> enforce_security_policy_callback_map_;





  struct {

  struct {

    hci::AddressWithType address_;

    hci::AddressWithType address_;