From 1991f5723070d9464c06f2bb8dc6bf3a2432f9fd Mon Sep 17 00:00:00 2001 From: Alex Klyubin Date: Fri, 3 Mar 2017 14:08:36 -0800 Subject: [PATCH] Restrict access from apps to bluetooth_address setting BluetoothManagerService for some reason leaks the Android's Bluetooth MAC address via Settings.Secure which is normally readable by all apps. This lets apps bypass the restriction on access to Bluetooth MAC address from apps. This commit fixes the issue by restricting access to bluetooth_address secure setting (Settings.Secure). Only packages which hold the android.permission.LOCAL_MAC_ADDRESS permission retain access. This commit accordingly grants LOCAL_MAC_ADDRESS permission to the system Shell app because a number of scripts (including Android CTS) use "adb shell settings get secure bluetooth_address" as a convenient way to query the device's Bluetooth MAC address over ADB. This is acceptable because the user of the device can see the Bluetooth MAC address and thus it's fine for shell to be able to see the address as well. Test: See CTS test added in the cts project in this topic. Test: "adb shell settings get secure bluetooth_address" returns the Bluetooth MAC address of the Android. Test: "adb shell settings list secure | grep bluetooth_address" returns the Bluetooth MAC address of the Android. Test: Bluetooth works (toggling off/on, pairing, file transfer) Bug: 33701414 Change-Id: I17b110b96eb3794b25c1661e93d29a7a003e3c9a --- data/etc/privapp-permissions-platform.xml | 1 + .../providers/settings/SettingsProvider.java | 38 +++++++++++++++++-- packages/Shell/AndroidManifest.xml | 1 + 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/data/etc/privapp-permissions-platform.xml b/data/etc/privapp-permissions-platform.xml index bf3e793c0bdf3..a47386c49d4ea 100644 --- a/data/etc/privapp-permissions-platform.xml +++ b/data/etc/privapp-permissions-platform.xml @@ -263,6 +263,7 @@ applications that come with the platform + diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java index 5b4d2fd73655a..0916abebcc499 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java @@ -1010,8 +1010,9 @@ public class SettingsProvider extends ContentProvider { final int owningUserId = resolveOwningUserIdForSecureSettingLocked(callingUserId, name); - // Special case for location (sigh). - if (isLocationProvidersAllowedRestricted(name, callingUserId, owningUserId)) { + if (!isSecureSettingAccessible(name, callingUserId, owningUserId)) { + // This caller is not permitted to access this setting. Pretend the setting + // doesn't exist. continue; } @@ -1045,8 +1046,9 @@ public class SettingsProvider extends ContentProvider { // Determine the owning user as some profile settings are cloned from the parent. final int owningUserId = resolveOwningUserIdForSecureSettingLocked(callingUserId, name); - // Special case for location (sigh). - if (isLocationProvidersAllowedRestricted(name, callingUserId, owningUserId)) { + if (!isSecureSettingAccessible(name, callingUserId, owningUserId)) { + // This caller is not permitted to access this setting. Pretend the setting doesn't + // exist. SettingsState settings = mSettingsRegistry.getSettingsLocked(SETTINGS_TYPE_SECURE, owningUserId); return settings != null ? settings.getNullSetting() : null; @@ -1358,6 +1360,34 @@ public class SettingsProvider extends ContentProvider { } } + /** + * Returns {@code true} if the specified secure setting should be accessible to the caller. + */ + private boolean isSecureSettingAccessible(String name, int callingUserId, + int owningUserId) { + // Special case for location (sigh). + // This check is not inside the name-based checks below because this method performs checks + // only if the calling user ID is not the same as the owning user ID. + if (isLocationProvidersAllowedRestricted(name, callingUserId, owningUserId)) { + return false; + } + + switch (name) { + case "bluetooth_address": + // BluetoothManagerService for some reason stores the Android's Bluetooth MAC + // address in this secure setting. Secure settings can normally be read by any app, + // which thus enables them to bypass the recently introduced restrictions on access + // to device identifiers. + // To mitigate this we make this setting available only to callers privileged to see + // this device's MAC addresses, same as through public API + // BluetoothAdapter.getAddress() (see BluetoothManagerService for details). + return getContext().checkCallingOrSelfPermission( + Manifest.permission.LOCAL_MAC_ADDRESS) == PackageManager.PERMISSION_GRANTED; + default: + return true; + } + } + private boolean isLocationProvidersAllowedRestricted(String name, int callingUserId, int owningUserId) { // Optimization - location providers are restricted only for managed profiles. diff --git a/packages/Shell/AndroidManifest.xml b/packages/Shell/AndroidManifest.xml index 5b4dd48354e60..3cce2993fc4a1 100644 --- a/packages/Shell/AndroidManifest.xml +++ b/packages/Shell/AndroidManifest.xml @@ -38,6 +38,7 @@ +