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

Commit 22e66597 authored by Yuri Lin's avatar Yuri Lin
Browse files

Revert^2 for adding permission enforcement for matchesCallFilter. 

This change limits callers of matchesCallFilter to those who have listener access, contacts read permission, or are system/SysUI/phone.

The previous attempt at this change broke phone functionality because the original permissions of this method (system or systemUI or phone) were not necessarily covered by "listener or contacts access".

Revert submission 16150992-revert-16077951-yl-mcfperm-TORWDGEVSS

Reason for revert: Trying to re-submit changes but with fixing the tests/functionality that they broke
Reverted Changes:
Ib5c6814de:Revert "Enforce that callers to matchesCallFilter ...
Ie93693688:Revert "Test permissions for matchesCallFilter in ...

Bug: 204523343
Bug: 192592755
Test: atest IncomingCallTest#testRingOnIncomingCall; NotificationManagerTest

Change-Id: Ic250d912d7f9b3351442927d822e9cd2ec632346
parent c025e147

core/java/android/app/NotificationManager.java

+4 −0
Original line number Diff line number Diff line
@@ -2579,6 +2579,10 @@ public class NotificationManager { 
     * for more information.

     * </p>

     * <p>

     * Callers of this method must have notification listener access, permission to read contacts,

     * or have system permissions.

     * </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

+44 −0
Original line number Diff line number Diff line
@@ -5083,6 +5083,33 @@ 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 systemAccess = false;

            try {

                enforceSystemOrSystemUI("INotificationManager.matchesCallFilter");

                systemAccess = true;

            } catch (SecurityException e) {

            }



            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 (!systemAccess && !listenerAccess) {

                    getContext().enforceCallingPermission(Manifest.permission.READ_CONTACTS,

                            "matchesCallFilter requires listener permission, contacts read access,"

                            + " or system level access");

                }

            }



            return mZenModeHelper.matchesCallFilter(

                    Binder.getCallingUserHandle(),

                    extras,
@@ -10954,6 +10981,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

+75 −0
Original line number Diff line number Diff line
@@ -8356,4 +8356,79 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
        verify(mPermissionHelper, never()).hasPermission(anyInt());

        verify(mPreferencesHelper, never()).getNotificationChannelsBypassingDnd(PKG, mUid);

    }



    @Test

    public void testMatchesCallFilter_noPermissionShouldThrow() throws Exception {

        // set the testable NMS to not system uid

        mService.isSystemUid = false;



        // 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_hasSystemPermission() throws Exception {

        // set the testable NMS to system uid

        mService.isSystemUid = true;



        // make sure caller doesn't have listener access or read_contacts permission

        when(mListeners.hasAllowedListener(anyString(), anyInt())).thenReturn(false);

        doThrow(new SecurityException()).when(mContext).enforceCallingPermission(

                eq("android.permission.READ_CONTACTS"), anyString());



        try {

            ((INotificationManager) mService.mService).matchesCallFilter(null);

            // pass, but check that we actually checked for system permissions

            assertTrue(mService.countSystemChecks > 0);

        } catch (SecurityException e) {

            fail("call to matchesCallFilter with just system permissions should work");

        }

    }



    @Test

    public void testMatchesCallFilter_hasListenerPermission() throws Exception {

        mService.isSystemUid = false;



        // 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 {

        mService.isSystemUid = false;



        // 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");

        }

    }

}