Revert "Revert "Revert^2 "Audio policy: anonymize Bluetooth MAC ..."
Revert submission 27578620
Reason for revert: 343776563
investigating
Reverted changes: /q/submissionid:27578620
Change-Id: I4e72dbcfa3e03c604ba9fa5d49ebeda01584790a
diff --git a/media/audioaidlconversion/AidlConversionCppNdk.cpp b/media/audioaidlconversion/AidlConversionCppNdk.cpp
index 9eaddce..77418eb 100644
--- a/media/audioaidlconversion/AidlConversionCppNdk.cpp
+++ b/media/audioaidlconversion/AidlConversionCppNdk.cpp
@@ -1069,6 +1069,13 @@
if (mac.size() != 6) return BAD_VALUE;
snprintf(addressBuffer, AUDIO_DEVICE_MAX_ADDRESS_LEN, "%02X:%02X:%02X:%02X:%02X:%02X",
mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]);
+ // special case for anonymized mac address:
+ // change anonymized bytes back from FD:FF:FF:FF to XX:XX:XX:XX
+ std::string address(addressBuffer);
+ if (address.compare(0, strlen("FD:FF:FF:FF"), "FD:FF:FF:FF") == 0) {
+ address.replace(0, strlen("FD:FF:FF:FF"), "XX:XX:XX:XX");
+ }
+ strcpy(addressBuffer, address.c_str());
} break;
case Tag::ipv4: {
const std::vector<uint8_t>& ipv4 = aidl.address.get<AudioDeviceAddress::ipv4>();
@@ -1129,11 +1136,20 @@
if (!legacyAddress.empty()) {
switch (suggestDeviceAddressTag(aidl.type)) {
case Tag::mac: {
+ // special case for anonymized mac address:
+ // change anonymized bytes so that they can be scanned as HEX bytes
+ // Use '01' for LSB bits 0 and 1 as Bluetooth MAC addresses are never multicast
+ // and universaly administered
+ std::string address = legacyAddress;
+ if (address.compare(0, strlen("XX:XX:XX:XX"), "XX:XX:XX:XX") == 0) {
+ address.replace(0, strlen("XX:XX:XX:XX"), "FD:FF:FF:FF");
+ }
+
std::vector<uint8_t> mac(6);
- int status = sscanf(legacyAddress.c_str(), "%hhX:%hhX:%hhX:%hhX:%hhX:%hhX",
+ int status = sscanf(address.c_str(), "%hhX:%hhX:%hhX:%hhX:%hhX:%hhX",
&mac[0], &mac[1], &mac[2], &mac[3], &mac[4], &mac[5]);
if (status != mac.size()) {
- ALOGE("%s: malformed MAC address: \"%s\"", __func__, legacyAddress.c_str());
+ ALOGE("%s: malformed MAC address: \"%s\"", __func__, address.c_str());
return unexpected(BAD_VALUE);
}
aidl.address = AudioDeviceAddress::make<AudioDeviceAddress::mac>(std::move(mac));
diff --git a/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp b/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp
index 7f55e48..0be1d7e 100644
--- a/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp
+++ b/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp
@@ -483,8 +483,28 @@
AudioDeviceAddress::make<AudioDeviceAddress::Tag::alsa>(
std::vector<int32_t>{1, 2}))));
+TEST(AnonymizedBluetoothAddressRoundTripTest, Legacy2Aidl2Legacy) {
+ const std::vector<uint8_t> sAnonymizedAidlAddress =
+ std::vector<uint8_t>{0xFD, 0xFF, 0xFF, 0xFF, 0xAB, 0xCD};
+ const std::string sAnonymizedLegacyAddress = std::string("XX:XX:XX:XX:AB:CD");
+ auto device = legacy2aidl_audio_device_AudioDevice(AUDIO_DEVICE_OUT_BLUETOOTH_A2DP,
+ sAnonymizedLegacyAddress);
+ ASSERT_TRUE(device.ok());
+ ASSERT_EQ(AudioDeviceAddress::Tag::mac, device.value().address.getTag());
+ ASSERT_EQ(sAnonymizedAidlAddress, device.value().address.get<AudioDeviceAddress::mac>());
+
+ audio_devices_t legacyType;
+ std::string legacyAddress;
+ status_t status =
+ aidl2legacy_AudioDevice_audio_device(device.value(), &legacyType, &legacyAddress);
+ ASSERT_EQ(OK, status);
+ EXPECT_EQ(legacyType, AUDIO_DEVICE_OUT_BLUETOOTH_A2DP);
+ EXPECT_EQ(sAnonymizedLegacyAddress, legacyAddress);
+}
+
class AudioFormatDescriptionRoundTripTest : public testing::TestWithParam<AudioFormatDescription> {
};
+
TEST_P(AudioFormatDescriptionRoundTripTest, Aidl2Legacy2Aidl) {
const auto initial = GetParam();
auto conv = aidl2legacy_AudioFormatDescription_audio_format_t(initial);
diff --git a/media/utils/ServiceUtilities.cpp b/media/utils/ServiceUtilities.cpp
index 7bec8cf..4b0192a 100644
--- a/media/utils/ServiceUtilities.cpp
+++ b/media/utils/ServiceUtilities.cpp
@@ -47,6 +47,7 @@
static const String16 sModifyPhoneState("android.permission.MODIFY_PHONE_STATE");
static const String16 sModifyAudioRouting("android.permission.MODIFY_AUDIO_ROUTING");
static const String16 sCallAudioInterception("android.permission.CALL_AUDIO_INTERCEPTION");
+static const String16 sAndroidPermissionBluetoothConnect("android.permission.BLUETOOTH_CONNECT");
static String16 resolveCallingPackage(PermissionController& permissionController,
const std::optional<String16> opPackageName, uid_t uid) {
@@ -392,6 +393,48 @@
return NO_ERROR;
}
+/**
+ * Determines if the MAC address in Bluetooth device descriptors returned by APIs of
+ * a native audio service (audio flinger, audio policy) must be anonymized.
+ * MAC addresses returned to system server or apps with BLUETOOTH_CONNECT permission
+ * are not anonymized.
+ *
+ * @param attributionSource The attribution source of the calling app.
+ * @param caller string identifying the caller for logging.
+ * @return true if the MAC addresses must be anonymized, false otherwise.
+ */
+bool mustAnonymizeBluetoothAddress(
+ const AttributionSourceState& attributionSource, const String16& caller) {
+ uid_t uid = VALUE_OR_FATAL(aidl2legacy_int32_t_uid_t(attributionSource.uid));
+ if (isAudioServerOrSystemServerUid(uid)) {
+ return false;
+ }
+ const std::optional<AttributionSourceState> resolvedAttributionSource =
+ resolveAttributionSource(attributionSource, DEVICE_ID_DEFAULT);
+ if (!resolvedAttributionSource.has_value()) {
+ return true;
+ }
+ permission::PermissionChecker permissionChecker;
+ return permissionChecker.checkPermissionForPreflightFromDatasource(
+ sAndroidPermissionBluetoothConnect, resolvedAttributionSource.value(), caller,
+ AppOpsManager::OP_BLUETOOTH_CONNECT)
+ != permission::PermissionChecker::PERMISSION_GRANTED;
+}
+
+/**
+ * Modifies the passed MAC address string in place for consumption by unprivileged clients.
+ * the string is assumed to have a valid MAC address format.
+ * the anonymzation must be kept in sync with toAnonymizedAddress() in BluetoothUtils.java
+ *
+ * @param address input/output the char string contining the MAC address to anonymize.
+ */
+void anonymizeBluetoothAddress(char *address) {
+ if (address == nullptr || strlen(address) != strlen("AA:BB:CC:DD:EE:FF")) {
+ return;
+ }
+ memcpy(address, "XX:XX:XX:XX", strlen("XX:XX:XX:XX"));
+}
+
sp<content::pm::IPackageManagerNative> MediaPackageManager::retrievePackageManager() {
const sp<IServiceManager> sm = defaultServiceManager();
if (sm == nullptr) {
diff --git a/media/utils/include/mediautils/ServiceUtilities.h b/media/utils/include/mediautils/ServiceUtilities.h
index e0fabfd..9c02cd4 100644
--- a/media/utils/include/mediautils/ServiceUtilities.h
+++ b/media/utils/include/mediautils/ServiceUtilities.h
@@ -113,6 +113,10 @@
bool bypassInterruptionPolicyAllowed(const AttributionSourceState& attributionSource);
bool callAudioInterceptionAllowed(const AttributionSourceState& attributionSource);
void purgePermissionCache();
+bool mustAnonymizeBluetoothAddress(
+ const AttributionSourceState& attributionSource, const String16& caller);
+void anonymizeBluetoothAddress(char *address);
+
int32_t getOpForSource(audio_source_t source);
AttributionSourceState getCallingAttributionSource();
diff --git a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
index 615c390..a862037 100644
--- a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
+++ b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
@@ -1548,6 +1548,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,
@@ -1566,14 +1579,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)));
@@ -1596,12 +1622,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();
}
@@ -1659,14 +1697,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)));