Merge "GD Security: Improve Enforce() workflow" (efc471c3) · Commits · e / os / android_packages_modules_Bluetooth

system/gd/security/cert/security_test.py

+2 −12

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

		# 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
		self.dut_security.set_io_capabilities(IoCapabilities.NO_INPUT_NO_OUTPUT)
		self.dut_security.set_authentication_requirements(AuthenticationRequirements.DEDICATED_BONDING)
		@@ -171,17 +171,7 @@ class SecurityTest(GdBaseTestClass):
		common.BluetoothAddressTypeEnum.PUBLIC_DEVICE_ADDRESS,
		ClassicSecurityPolicy.ENCRYPTED_TRANSPORT)

		self._verify_ssp_numeric_comparison(
		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)
		# TODO: We verify enforcement when we make sure EncryptionChange is received on DUT

		# 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):

system/gd/security/internal/security_manager_impl.cc

+84 −41

Original line number	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 record = security_database_.FindOrCreate(remote);
		record->SetIsEncrypted(encrypted);
		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);
		}
		UpdateLinkSecurityCondition(remote);
		}

		void SecurityManagerImpl::OnHciLeEvent(hci::LeMetaEventView event) {
		@@ -388,11 +384,6 @@ void SecurityManagerImpl::OnPairingHandlerComplete(hci::Address address, Pairing
		security_manager_channel_->Release(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)) {
		NotifyDeviceBonded(remote);
		} else {
		@@ -401,6 +392,7 @@ void SecurityManagerImpl::OnPairingHandlerComplete(hci::Address address, Pairing
		auto record = this->security_database_.FindOrCreate(remote);
		record->CancelPairing();
		security_database_.SaveRecordsToStorage();
		UpdateLinkSecurityCondition(remote);
		}

		void SecurityManagerImpl::OnL2capRegistrationCompleteLe(
		@@ -713,36 +705,57 @@ void SecurityManagerImpl::SetOobDataPresent(hci::OobDataPresent data_present) {
		void SecurityManagerImpl::InternalEnforceSecurityPolicy(
		hci::AddressWithType remote,
		l2cap::classic::SecurityPolicy policy,
		l2cap::classic::SecurityEnforcementInterface::ResultCallback result_callback,
		bool try_meet_requirements) {
		l2cap::classic::SecurityEnforcementInterface::ResultCallback result_callback) {
		if (IsSecurityRequirementSatisfied(remote, policy)) {
		// Notify client immediately if already satisfied
		result_callback.Invoke(true);
		return;
		}

		hci::AuthenticationRequirements authentication_requirements = kDefaultAuthenticationRequirements;

		bool result = false;
		auto record = this->security_database_.FindOrCreate(remote);
		bool need_to_pair = false;

		switch (policy) {
		case l2cap::classic::SecurityPolicy::BEST:
		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;
		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;
		case l2cap::classic::SecurityPolicy::ENCRYPTED_TRANSPORT:
		result = record->IsEncrypted();
		if (!record->IsPaired()) {
		need_to_pair = true;
		authentication_requirements = hci::AuthenticationRequirements::NO_BONDING;
		} else {
		// just need to wait for encryption change callback
		}
		break;
		case l2cap::classic::SecurityPolicy::_SDP_ONLY_NO_SECURITY_WHATSOEVER_PLAINTEXT_TRANSPORT_OK:
		result = true;
		break;
		default:
		return;
		}
		if (!result && try_meet_requirements) {

		auto entry = enforce_security_policy_callback_map_.find(remote);
		if (entry != enforce_security_policy_callback_map_.end()) {
		LOG_WARN("Callback already pending for remote: '%s' !", remote.ToString().c_str());
		} else {
		enforce_security_policy_callback_map_.emplace(
		remote,
		std::pair<l2cap::classic::SecurityPolicy, l2cap::classic::SecurityEnforcementInterface::ResultCallback>(
		policy, std::move(result_callback)));
		if (!record->IsPairing()) {
		enforce_security_policy_callback_map_[remote] = {policy, std::move(result_callback)};
		}

		if (need_to_pair && !record->IsPairing()) {
		LOG_WARN("Dispatch #3");
		DispatchPairingHandler(
		record,
		@@ -752,16 +765,46 @@ void SecurityManagerImpl::InternalEnforceSecurityPolicy(
		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;
		}
		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(
		hci::AddressWithType remote,
		l2cap::classic::SecurityPolicy policy,
		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(

system/gd/security/internal/security_manager_impl.h

+8 −6

Original line number	Diff line number	Diff line
		@@ -224,8 +224,9 @@ class SecurityManagerImpl : public channel::ISecurityManagerChannelListener, pub
		void InternalEnforceSecurityPolicy(
		hci::AddressWithType remote,
		l2cap::classic::SecurityPolicy policy,
		l2cap::classic::SecurityEnforcementInterface::ResultCallback result_callback,
		bool try_meet_requirements);
		l2cap::classic::SecurityEnforcementInterface::ResultCallback result_callback);
		void UpdateLinkSecurityCondition(hci::AddressWithType remote);
		bool IsSecurityRequirementSatisfied(hci::AddressWithType remote, l2cap::classic::SecurityPolicy policy);
		void ConnectionIsReadyStartPairing(LeFixedChannelEntry* stored_channel);
		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<FacadeDisconnectCallback> facade_disconnect_callback_;

		std::unordered_map<
		hci::AddressWithType,
		std::pair<l2cap::classic::SecurityPolicy, l2cap::classic::SecurityEnforcementInterface::ResultCallback>>
		enforce_security_policy_callback_map_;
		struct PendingSecurityEnforcementEntry {
		l2cap::classic::SecurityPolicy policy_;
		l2cap::classic::SecurityEnforcementInterface::ResultCallback callback_;
		};
		std::unordered_map<hci::AddressWithType, PendingSecurityEnforcementEntry> enforce_security_policy_callback_map_;

		struct {
		hci::AddressWithType address_;