No blame for location usage if no location sensitive field is queried
ServiceStateProvide calls LocationAccessPolicy#checkLocationPermission
to check if the calling app has location permission. But this
API blames a location usage when the app has location granted.
To avoid caller get blamed if no location protected filed is queried,
ServiceStateProvide will perform location permission check only
app explicitly queries loction protected fields.
Bug: 179730843
Test: atest ServiceStateProvideTest
Merged-In: Idcb04cf3b981db8e704c485993225e366dbc8db4
Change-Id: Idcb04cf3b981db8e704c485993225e366dbc8db4
(cherry picked from commit 353652c095b138e9629de5a929f19d683db20602)
diff --git a/src/com/android/phone/ServiceStateProvider.java b/src/com/android/phone/ServiceStateProvider.java
index 56786f9..7df5b7c 100644
--- a/src/com/android/phone/ServiceStateProvider.java
+++ b/src/com/android/phone/ServiceStateProvider.java
@@ -413,29 +413,37 @@
ss = unredactedServiceState;
} else {
availableColumns = ALL_COLUMNS;
-
- final boolean hasLocationPermission = hasLocationPermission();
- if (hasLocationPermission) {
- // No matter the targetSdkVersion, return unredacted ServiceState if caller does
- // have location permission.
- ss = unredactedServiceState;
- } else {
- // The caller has targetSdkVersion S+ but no location permission. It explicitly
- // requires location protected columns. Throw SecurityException to fail loudly.
- if (targetingAtLeastS && projection != null) {
- for (String requiredColumn : projection) {
- if (LOCATION_PROTECTED_COLUMNS_SET.contains(requiredColumn)) {
- throw new SecurityException("Column " + requiredColumn
- + "requires location permissions to access.");
- }
+ boolean implicitlyQueryLocation = projection == null;
+ boolean explicitlyQueryLocation = false;
+ if (projection != null) {
+ for (String requiredColumn : projection) {
+ if (LOCATION_PROTECTED_COLUMNS_SET.contains(requiredColumn)) {
+ explicitlyQueryLocation = true;
+ break;
}
}
+ }
- // In all other cases, return the redacted ServiceState.
- // The caller has no location permission but only requires columns without
- // location sensitive info or "all" columns, return result that scrub out all
- // sensitive info. In later case, we will not know which columns will be fetched
- // from the returned cursor until the result has been returned.
+ // Check location permission only when location sensitive info are queried
+ // (either explicitly or implicitly) to avoid caller get blamed with location
+ // permission when query non sensitive info.
+ if (implicitlyQueryLocation || explicitlyQueryLocation) {
+ if (hasLocationPermission()) {
+ ss = unredactedServiceState;
+ } else {
+ if (targetingAtLeastS) {
+ // Throw SecurityException to fail loudly if caller is targetSDK S+
+ throw new SecurityException(
+ "Querying location sensitive info requires location "
+ + "permissions");
+ } else {
+ // For backward compatibility, return redacted value for old SDK
+ ss = getLocationRedactedServiceState(unredactedServiceState);
+ }
+ }
+ } else {
+ // The caller is not interested in location sensitive info, return result
+ // that scrub out all sensitive info. And no permission check is needed.
ss = getLocationRedactedServiceState(unredactedServiceState);
}
}
diff --git a/tests/src/com/android/phone/ServiceStateProviderTest.java b/tests/src/com/android/phone/ServiceStateProviderTest.java
index cde584f..4ace460 100644
--- a/tests/src/com/android/phone/ServiceStateProviderTest.java
+++ b/tests/src/com/android/phone/ServiceStateProviderTest.java
@@ -27,6 +27,8 @@
import static android.provider.Telephony.ServiceStateTable.getUriForSubscriptionId;
import static android.telephony.NetworkRegistrationInfo.REGISTRATION_STATE_HOME;
+import static com.android.phone.ServiceStateProvider.NETWORK_ID;
+
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
@@ -38,6 +40,8 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.nullable;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import android.Manifest;
@@ -288,11 +292,8 @@
setCanReadPrivilegedPhoneState(true);
setLocationPermissions(false);
- // NETWORK_ID is a location-sensitive column
- String[] projection = new String[]{"network_id"};
-
assertThrows(SecurityException.class,
- () -> verifyServiceStateWithLocationColumns(mTestServiceState, projection));
+ () -> verifyServiceStateWithLocationColumns(mTestServiceState));
}
/**
@@ -324,9 +325,22 @@
true /*hasPermission*/);
}
- private void verifyServiceStateWithLocationColumns(ServiceState ss, String[] projection) {
- try (Cursor cursor = mContentResolver.query(ServiceStateTable.CONTENT_URI, projection, null,
- null)) {
+ /**
+ * Verify that when caller with targetSDK S+ has location permission and try to query
+ * location non-sensitive info, it should not get blamed.
+ */
+ @Test
+ public void testQuery_noLocationBlamed_whenQueryNonLocationInfo_withPermission() {
+ setTargetSdkVersion(Build.VERSION_CODES.S);
+ setLocationPermissions(true);
+
+ verifyServiceStateWithPublicColumns(mTestServiceState, null /*projection*/);
+ verify(mAppOpsManager, never()).noteOpNoThrow(any(), anyInt(), any(), any(), any());
+ }
+
+ private void verifyServiceStateWithLocationColumns(ServiceState ss) {
+ try (Cursor cursor = mContentResolver.query(ServiceStateTable.CONTENT_URI,
+ new String[]{NETWORK_ID}, null, null)) {
assertNotNull(cursor);
}
}