Anonymize bt addresses in listAudioPorts
Fix a security bypass where listAudioPorts would provide BT MAC
addresses without the required permission.
If a client doesn't have BLUETOOTH_CONNECT, partially redact the
address. To avoid performance issues, this change:
- Caches uids which hold the permission, invalidating them on the
package manager cache invalidation sysprop
- Ensures we only call the check outside of any locks in audioserver.
Soaking for backport on main, a better solution to follow.
Test: Manual repro using the POC app
Bug: 285588444
Flag: EXEMPT security
Change-Id: Ide27226237236380814e6f19a74d1ce1e72828fd
diff --git a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
index 24ab6a1..60b155da 100644
--- a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
+++ b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
@@ -1687,6 +1687,19 @@
return Status::ok();
}
+template <typename Port>
+void anonymizePortBluetoothAddress(Port& port) {
+ if (port.type != AUDIO_PORT_TYPE_DEVICE) {
+ return;
+ }
+ if (!(audio_is_a2dp_device(port.ext.device.type)
+ || audio_is_ble_device(port.ext.device.type)
+ || audio_is_bluetooth_sco_device(port.ext.device.type)
+ || audio_is_hearing_aid_out_device(port.ext.device.type))) {
+ return;
+ }
+ anonymizeBluetoothAddress(port.ext.device.address);
+}
Status AudioPolicyService::listAudioPorts(media::AudioPortRole roleAidl,
media::AudioPortType typeAidl, Int* count,
@@ -1705,14 +1718,27 @@
std::unique_ptr<audio_port_v7[]> ports(new audio_port_v7[num_ports]);
unsigned int generation;
- audio_utils::lock_guard _l(mMutex);
- if (mAudioPolicyManager == NULL) {
- return binderStatusFromStatusT(NO_INIT);
- }
+ const AttributionSourceState attributionSource = getCallingAttributionSource();
AutoCallerClear acc;
- RETURN_IF_BINDER_ERROR(binderStatusFromStatusT(
- mAudioPolicyManager->listAudioPorts(role, type, &num_ports, ports.get(), &generation)));
- numPortsReq = std::min(numPortsReq, num_ports);
+ {
+ audio_utils::lock_guard _l(mMutex);
+ if (mAudioPolicyManager == NULL) {
+ return binderStatusFromStatusT(NO_INIT);
+ }
+ // AudioPolicyManager->listAudioPorts makes a deep copy of port structs into ports
+ // so it is safe to access after releasing the mutex
+ RETURN_IF_BINDER_ERROR(binderStatusFromStatusT(
+ mAudioPolicyManager->listAudioPorts(
+ role, type, &num_ports, ports.get(), &generation)));
+ numPortsReq = std::min(numPortsReq, num_ports);
+ }
+
+ if (mustAnonymizeBluetoothAddress(attributionSource, String16(__func__))) {
+ for (size_t i = 0; i < numPortsReq; ++i) {
+ anonymizePortBluetoothAddress(ports[i]);
+ }
+ }
+
RETURN_IF_BINDER_ERROR(binderStatusFromStatusT(
convertRange(ports.get(), ports.get() + numPortsReq, std::back_inserter(*portsAidl),
legacy2aidl_audio_port_v7_AudioPortFw)));
@@ -1735,12 +1761,24 @@
Status AudioPolicyService::getAudioPort(int portId,
media::AudioPortFw* _aidl_return) {
audio_port_v7 port{ .id = portId };
- audio_utils::lock_guard _l(mMutex);
- if (mAudioPolicyManager == NULL) {
- return binderStatusFromStatusT(NO_INIT);
- }
+
+ const AttributionSourceState attributionSource = getCallingAttributionSource();
AutoCallerClear acc;
- RETURN_IF_BINDER_ERROR(binderStatusFromStatusT(mAudioPolicyManager->getAudioPort(&port)));
+
+ {
+ audio_utils::lock_guard _l(mMutex);
+ if (mAudioPolicyManager == NULL) {
+ return binderStatusFromStatusT(NO_INIT);
+ }
+ // AudioPolicyManager->getAudioPort makes a deep copy of the port struct into port
+ // so it is safe to access after releasing the mutex
+ RETURN_IF_BINDER_ERROR(binderStatusFromStatusT(mAudioPolicyManager->getAudioPort(&port)));
+ }
+
+ if (mustAnonymizeBluetoothAddress(attributionSource, String16(__func__))) {
+ anonymizePortBluetoothAddress(port);
+ }
+
*_aidl_return = VALUE_OR_RETURN_BINDER_STATUS(legacy2aidl_audio_port_v7_AudioPortFw(port));
return Status::ok();
}
@@ -1802,14 +1840,32 @@
std::unique_ptr<audio_patch[]> patches(new audio_patch[num_patches]);
unsigned int generation;
- audio_utils::lock_guard _l(mMutex);
- if (mAudioPolicyManager == NULL) {
- return binderStatusFromStatusT(NO_INIT);
- }
+ const AttributionSourceState attributionSource = getCallingAttributionSource();
AutoCallerClear acc;
- RETURN_IF_BINDER_ERROR(binderStatusFromStatusT(
- mAudioPolicyManager->listAudioPatches(&num_patches, patches.get(), &generation)));
- numPatchesReq = std::min(numPatchesReq, num_patches);
+
+ {
+ audio_utils::lock_guard _l(mMutex);
+ if (mAudioPolicyManager == NULL) {
+ return binderStatusFromStatusT(NO_INIT);
+ }
+ // AudioPolicyManager->listAudioPatches makes a deep copy of patches structs into patches
+ // so it is safe to access after releasing the mutex
+ RETURN_IF_BINDER_ERROR(binderStatusFromStatusT(
+ mAudioPolicyManager->listAudioPatches(&num_patches, patches.get(), &generation)));
+ numPatchesReq = std::min(numPatchesReq, num_patches);
+ }
+
+ if (mustAnonymizeBluetoothAddress(attributionSource, String16(__func__))) {
+ for (size_t i = 0; i < numPatchesReq; ++i) {
+ for (size_t j = 0; j < patches[i].num_sources; ++j) {
+ anonymizePortBluetoothAddress(patches[i].sources[j]);
+ }
+ for (size_t j = 0; j < patches[i].num_sinks; ++j) {
+ anonymizePortBluetoothAddress(patches[i].sinks[j]);
+ }
+ }
+ }
+
RETURN_IF_BINDER_ERROR(binderStatusFromStatusT(
convertRange(patches.get(), patches.get() + numPatchesReq,
std::back_inserter(*patchesAidl), legacy2aidl_audio_patch_AudioPatchFw)));