Merge "Enforce that callers to matchesCallFilter have either listener access or read_contacts permission."

This commit is contained in:
Yuri Lin
2021-10-28 14:27:12 +00:00
committed by Android (Google) Code Review
4 changed files with 108 additions and 3 deletions

View File

@@ -2579,6 +2579,9 @@ public class NotificationManager {
* for more information.
* </p>
* <p>
* Callers of this method must have notification listener access or permission to read contacts.
* </p>
* <p>
* NOTE: This method calls into Contacts, which may take some time, and should not be called
* on the main thread.
* </p>

View File

@@ -5083,6 +5083,27 @@ public class NotificationManagerService extends SystemService {
@Override
public boolean matchesCallFilter(Bundle extras) {
// Because matchesCallFilter may use contact data to filter calls, the callers of this
// method need to either have notification listener access or permission to read
// contacts.
boolean listenerAccess = false;
try {
String[] pkgNames = mPackageManager.getPackagesForUid(Binder.getCallingUid());
for (int i = 0; i < pkgNames.length; i++) {
// in most cases there should only be one package here
listenerAccess |= mListeners.hasAllowedListener(pkgNames[i],
Binder.getCallingUserHandle().getIdentifier());
}
} catch (RemoteException e) {
} finally {
if (!listenerAccess) {
getContext().enforceCallingPermission(
Manifest.permission.READ_CONTACTS,
"matchesCallFilter requires listener permissions or "
+ "contacts read access");
}
}
return mZenModeHelper.matchesCallFilter(
Binder.getCallingUserHandle(),
extras,
@@ -10954,6 +10975,23 @@ public class NotificationManagerService extends SystemService {
}
return false;
}
// Returns whether there is a component with listener access granted that is associated
// with the given package name / user ID.
boolean hasAllowedListener(String packageName, int userId) {
if (packageName == null) {
return false;
}
// Loop through allowed components to compare package names
List<ComponentName> allowedComponents = getAllowedComponents(userId);
for (int i = 0; i < allowedComponents.size(); i++) {
if (allowedComponents.get(i).getPackageName().equals(packageName)) {
return true;
}
}
return false;
}
}
// TODO (b/194833441): remove when we've fully migrated to a permission

View File

@@ -24,8 +24,9 @@ import static com.android.server.notification.NotificationManagerService.Notific
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;
@@ -41,7 +42,6 @@ import android.service.notification.NotificationListenerFilter;
import android.service.notification.NotificationListenerService;
import android.util.ArraySet;
import android.util.Pair;
import android.util.Slog;
import android.util.TypedXmlPullParser;
import android.util.TypedXmlSerializer;
import android.util.Xml;
@@ -355,4 +355,19 @@ public class NotificationListenersTest extends UiServiceTestCase {
.getDisallowedPackages()).isEmpty();
}
@Test
public void testHasAllowedListener() {
final int uid1 = 1, uid2 = 2;
// enable mCn1 but not mCn2 for uid1
mListeners.addApprovedList(mCn1.flattenToString(), uid1, true);
// verify that:
// the package for mCn1 has an allowed listener for uid1 and not uid2
assertTrue(mListeners.hasAllowedListener(mCn1.getPackageName(), uid1));
assertFalse(mListeners.hasAllowedListener(mCn1.getPackageName(), uid2));
// and that mCn2 has no allowed listeners for either user id
assertFalse(mListeners.hasAllowedListener(mCn2.getPackageName(), uid1));
assertFalse(mListeners.hasAllowedListener(mCn2.getPackageName(), uid2));
}
}

View File

@@ -8356,4 +8356,53 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
verify(mPermissionHelper, never()).hasPermission(anyInt());
verify(mPreferencesHelper, never()).getNotificationChannelsBypassingDnd(PKG, mUid);
}
@Test
public void testMatchesCallFilter_noPermissionShouldThrow() throws Exception {
// make sure a caller without listener access or read_contacts permission can't call
// matchesCallFilter.
when(mListeners.hasAllowedListener(anyString(), anyInt())).thenReturn(false);
doThrow(new SecurityException()).when(mContext).enforceCallingPermission(
eq("android.permission.READ_CONTACTS"), anyString());
try {
// shouldn't matter what we're passing in, if we get past this line fail immediately
((INotificationManager) mService.mService).matchesCallFilter(null);
fail("call to matchesCallFilter with no permissions should fail");
} catch (SecurityException e) {
// pass
}
}
@Test
public void testMatchesCallFilter_hasListenerPermission() throws Exception {
// make sure a caller with only listener access and not read_contacts permission can call
// matchesCallFilter.
when(mListeners.hasAllowedListener(anyString(), anyInt())).thenReturn(true);
doThrow(new SecurityException()).when(mContext).enforceCallingPermission(
eq("android.permission.READ_CONTACTS"), anyString());
try {
((INotificationManager) mService.mService).matchesCallFilter(null);
// pass, this is not a functionality test
} catch (SecurityException e) {
fail("call to matchesCallFilter with listener permissions should work");
}
}
@Test
public void testMatchesCallFilter_hasContactsPermission() throws Exception {
// make sure a caller with only read_contacts permission and not listener access can call
// matchesCallFilter.
when(mListeners.hasAllowedListener(anyString(), anyInt())).thenReturn(false);
doNothing().when(mContext).enforceCallingPermission(
eq("android.permission.READ_CONTACTS"), anyString());
try {
((INotificationManager) mService.mService).matchesCallFilter(null);
// pass, this is not a functionality test
} catch (SecurityException e) {
fail("call to matchesCallFilter with listener permissions should work");
}
}
}