From 8e6fbc8c1acff08c0f69811d50af6b57ca1d146e Mon Sep 17 00:00:00 2001 From: Yuyang Huang Date: Mon, 7 Aug 2023 17:46:19 +0900 Subject: [PATCH] Update permission check for offloadEngine registration For an app to register itself as an offloadEngine, it must have either of the following permissions: NETWORK_STACK, PERMISSION_MAINLINE_NETWORK_STACK, NETWORK_SETTINGS, REGISTER_NSD_OFFLOAD_ENGINE. Bug: 294777050 Test: atest CtsNetTestCases FrameworksNetTests Change-Id: I19fe9b996a02b1ae23116c02a1b8406d93b3ecf1 --- .../src/com/android/server/NsdService.java | 32 ++++++++++++--- .../com/android/server/NsdServiceTest.java | 39 +++++++++++++++++++ 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/service-t/src/com/android/server/NsdService.java b/service-t/src/com/android/server/NsdService.java index eb73ee5bea..415159dcf1 100644 --- a/service-t/src/com/android/server/NsdService.java +++ b/service-t/src/com/android/server/NsdService.java @@ -17,15 +17,18 @@ package com.android.server; import static android.Manifest.permission.NETWORK_SETTINGS; +import static android.Manifest.permission.NETWORK_STACK; import static android.net.ConnectivityManager.NETID_UNSET; import static android.net.NetworkCapabilities.TRANSPORT_VPN; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; +import static android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK; import static android.net.nsd.NsdManager.MDNS_DISCOVERY_MANAGER_EVENT; import static android.net.nsd.NsdManager.MDNS_SERVICE_EVENT; import static android.net.nsd.NsdManager.RESOLVE_SERVICE_SUCCEEDED; import static android.provider.DeviceConfig.NAMESPACE_TETHERING; import static com.android.modules.utils.build.SdkLevel.isAtLeastU; +import static com.android.networkstack.apishim.ConstantsShim.REGISTER_NSD_OFFLOAD_ENGINE; import static com.android.server.connectivity.mdns.MdnsRecord.MAX_LABEL_LENGTH; import static com.android.server.connectivity.mdns.util.MdnsUtils.Clock; @@ -75,6 +78,7 @@ import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.State; import com.android.internal.util.StateMachine; import com.android.metrics.NetworkNsdReportedMetrics; +import com.android.modules.utils.build.SdkLevel; import com.android.net.module.util.CollectionUtils; import com.android.net.module.util.DeviceConfigUtils; import com.android.net.module.util.InetAddressUtils; @@ -2040,9 +2044,7 @@ public class NsdService extends INsdManager.Stub { public void registerOffloadEngine(String ifaceName, IOffloadEngine cb, @OffloadEngine.OffloadCapability long offloadCapabilities, @OffloadEngine.OffloadType long offloadTypes) { - // TODO: Relax the permission because NETWORK_SETTINGS is a signature permission, and - // it may not be possible for all the callers of this API to have it. - PermissionUtils.enforceNetworkStackPermissionOr(mContext, NETWORK_SETTINGS); + checkOffloadEnginePermission(mContext); Objects.requireNonNull(ifaceName); Objects.requireNonNull(cb); mNsdStateMachine.sendMessage( @@ -2053,13 +2055,31 @@ public class NsdService extends INsdManager.Stub { @Override public void unregisterOffloadEngine(IOffloadEngine cb) { - // TODO: Relax the permission because NETWORK_SETTINGS is a signature permission, and - // it may not be possible for all the callers of this API to have it. - PermissionUtils.enforceNetworkStackPermissionOr(mContext, NETWORK_SETTINGS); + checkOffloadEnginePermission(mContext); Objects.requireNonNull(cb); mNsdStateMachine.sendMessage( mNsdStateMachine.obtainMessage(NsdManager.UNREGISTER_OFFLOAD_ENGINE, cb)); } + + private static void checkOffloadEnginePermission(Context context) { + if (!SdkLevel.isAtLeastT()) { + throw new SecurityException("API is not available in before API level 33"); + } + // REGISTER_NSD_OFFLOAD_ENGINE was only added to the SDK in V, but may + // be back ported to older builds: accept it as long as it's signature-protected + if (PermissionUtils.checkAnyPermissionOf(context, REGISTER_NSD_OFFLOAD_ENGINE) + && (SdkLevel.isAtLeastV() || PermissionUtils.isSystemSignaturePermission( + context, REGISTER_NSD_OFFLOAD_ENGINE))) { + return; + } + if (PermissionUtils.checkAnyPermissionOf(context, NETWORK_STACK, + PERMISSION_MAINLINE_NETWORK_STACK, NETWORK_SETTINGS)) { + return; + } + throw new SecurityException("Requires one of the following permissions: " + + String.join(", ", List.of(REGISTER_NSD_OFFLOAD_ENGINE, NETWORK_STACK, + PERMISSION_MAINLINE_NETWORK_STACK, NETWORK_SETTINGS)) + "."); + } } private void sendNsdStateChangeBroadcast(boolean isEnabled) { diff --git a/tests/unit/java/com/android/server/NsdServiceTest.java b/tests/unit/java/com/android/server/NsdServiceTest.java index dbd4e4edca..46ddeeee71 100644 --- a/tests/unit/java/com/android/server/NsdServiceTest.java +++ b/tests/unit/java/com/android/server/NsdServiceTest.java @@ -16,20 +16,27 @@ package com.android.server; +import static android.Manifest.permission.NETWORK_SETTINGS; +import static android.Manifest.permission.NETWORK_STACK; import static android.app.ActivityManager.RunningAppProcessInfo.IMPORTANCE_CACHED; import static android.app.ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND; import static android.app.ActivityManager.RunningAppProcessInfo.IMPORTANCE_GONE; import static android.app.ActivityManager.RunningAppProcessInfo.IMPORTANCE_VISIBLE; +import static android.content.pm.PackageManager.PERMISSION_DENIED; +import static android.content.pm.PackageManager.PERMISSION_GRANTED; +import static android.content.pm.PermissionInfo.PROTECTION_SIGNATURE; import static android.net.InetAddresses.parseNumericAddress; import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET; import static android.net.NetworkCapabilities.TRANSPORT_VPN; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; +import static android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK; import static android.net.connectivity.ConnectivityCompatChanges.ENABLE_PLATFORM_MDNS_BACKEND; import static android.net.connectivity.ConnectivityCompatChanges.RUN_NATIVE_NSD_ONLY_IF_LEGACY_APPS_T_AND_LATER; import static android.net.nsd.NsdManager.FAILURE_BAD_PARAMETERS; import static android.net.nsd.NsdManager.FAILURE_INTERNAL_ERROR; import static android.net.nsd.NsdManager.FAILURE_OPERATION_NOT_RUNNING; +import static com.android.networkstack.apishim.api33.ConstantsShim.REGISTER_NSD_OFFLOAD_ENGINE; import static com.android.server.NsdService.DEFAULT_RUNNING_APP_ACTIVE_IMPORTANCE_CUTOFF; import static com.android.server.NsdService.MdnsListener; import static com.android.server.NsdService.NO_TRANSACTION; @@ -68,6 +75,8 @@ import android.app.ActivityManager.OnUidImportanceListener; import android.compat.testing.PlatformCompatChangeRule; import android.content.ContentResolver; import android.content.Context; +import android.content.pm.PackageManager; +import android.content.pm.PermissionInfo; import android.net.INetd; import android.net.Network; import android.net.mdns.aidl.DiscoveryInfo; @@ -84,6 +93,7 @@ import android.net.nsd.NsdManager.RegistrationListener; import android.net.nsd.NsdManager.ResolveListener; import android.net.nsd.NsdManager.ServiceInfoCallback; import android.net.nsd.NsdServiceInfo; +import android.net.nsd.OffloadEngine; import android.net.wifi.WifiManager; import android.os.Binder; import android.os.Build; @@ -161,6 +171,7 @@ public class NsdServiceTest { @Rule public TestRule compatChangeRule = new PlatformCompatChangeRule(); @Mock Context mContext; + @Mock PackageManager mPackageManager; @Mock ContentResolver mResolver; @Mock MDnsManager mMockMDnsM; @Mock Dependencies mDeps; @@ -198,6 +209,7 @@ public class NsdServiceTest { mockService(mContext, MDnsManager.class, MDnsManager.MDNS_SERVICE, mMockMDnsM); mockService(mContext, WifiManager.class, Context.WIFI_SERVICE, mWifiManager); mockService(mContext, ActivityManager.class, Context.ACTIVITY_SERVICE, mActivityManager); + doReturn(mPackageManager).when(mContext).getPackageManager(); if (mContext.getSystemService(MDnsManager.class) == null) { // Test is using mockito-extended doCallRealMethod().when(mContext).getSystemService(MDnsManager.class); @@ -1642,6 +1654,33 @@ public class NsdServiceTest { assertThrows(IllegalArgumentException.class, () -> new NsdManager(mContext, service)); } + @Test + @EnableCompatChanges(ENABLE_PLATFORM_MDNS_BACKEND) + public void testRegisterOffloadEngine_checkPermission() + throws PackageManager.NameNotFoundException { + final NsdManager client = connectClient(mService); + final OffloadEngine offloadEngine = mock(OffloadEngine.class); + doReturn(PERMISSION_DENIED).when(mContext).checkCallingOrSelfPermission(NETWORK_STACK); + doReturn(PERMISSION_DENIED).when(mContext).checkCallingOrSelfPermission( + PERMISSION_MAINLINE_NETWORK_STACK); + doReturn(PERMISSION_DENIED).when(mContext).checkCallingOrSelfPermission(NETWORK_SETTINGS); + doReturn(PERMISSION_GRANTED).when(mContext).checkCallingOrSelfPermission( + REGISTER_NSD_OFFLOAD_ENGINE); + + PermissionInfo permissionInfo = new PermissionInfo(""); + permissionInfo.packageName = "android"; + permissionInfo.protectionLevel = PROTECTION_SIGNATURE; + doReturn(permissionInfo).when(mPackageManager).getPermissionInfo( + REGISTER_NSD_OFFLOAD_ENGINE, 0); + client.registerOffloadEngine("iface1", OffloadEngine.OFFLOAD_TYPE_REPLY, + OffloadEngine.OFFLOAD_CAPABILITY_BYPASS_MULTICAST_LOCK, + Runnable::run, offloadEngine); + client.unregisterOffloadEngine(offloadEngine); + + // TODO: add checks to test the packageName other than android + } + + private void waitForIdle() { HandlerUtils.waitForIdle(mHandler, TIMEOUT_MS); }