Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8c128dae authored by Yuri Lin's avatar Yuri Lin
Browse files

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

This constraint is because matchesCallFilter potentially exposes contact information for users whose DND settings involve only allowing calls from contacts or starred contacts.

Bug: 192592755
Test: NotificationManagerServiceTest, NotificationManagerTest#testMatchesCallFilter_*
Change-Id: I8e7c8b6994cb9668e22c3a7fc017d4401338369d
parent 4c0857c9

core/java/android/app/NotificationManager.java

+3 −0
Original line number Diff line number Diff line
@@ -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>

services/core/java/com/android/server/notification/NotificationManagerService.java

+38 −0
Original line number Diff line number Diff line
@@ -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

services/tests/uiservicestests/src/com/android/server/notification/NotificationListenersTest.java

+18 −3
Original line number Diff line number Diff line
@@ -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));

    }

}

services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java

+49 −0
Original line number Diff line number Diff line
@@ -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");

        }

    }

}