From 3ffffe71c7d6c69f17c5cc6b48ab43e25d3b11b4 Mon Sep 17 00:00:00 2001 From: paulhu Date: Thu, 16 Sep 2021 10:15:22 +0800 Subject: [PATCH] Use common methods to check/enforece mutiple permissions Use PermissionUtils methods to check/enforece mutiple permissions to avoid inconsistent behavior and duplicated code. Bug: 177187957 Test: atest FrameworksNetTests CtsNetTestCases Change-Id: I0850a2c8b15e0dfc6d21298c5599ad36bb2056dc --- .../src/com/android/server/NsdService.java | 9 +-- .../server/ethernet/EthernetServiceImpl.java | 9 +-- .../server/net/NetworkStatsService.java | 21 +----- .../android/server/ConnectivityService.java | 64 +++++++------------ 4 files changed, 27 insertions(+), 76 deletions(-) diff --git a/service-t/src/com/android/server/NsdService.java b/service-t/src/com/android/server/NsdService.java index 7115720fd6..88184606c7 100644 --- a/service-t/src/com/android/server/NsdService.java +++ b/service-t/src/com/android/server/NsdService.java @@ -21,7 +21,6 @@ import static android.net.nsd.NsdManager.MDNS_SERVICE_EVENT; import android.content.Context; import android.content.Intent; -import android.content.pm.PackageManager; import android.net.ConnectivityManager; import android.net.INetd; import android.net.LinkProperties; @@ -51,6 +50,7 @@ import android.util.SparseIntArray; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.State; import com.android.internal.util.StateMachine; +import com.android.net.module.util.PermissionUtils; import java.io.FileDescriptor; import java.io.PrintWriter; @@ -861,12 +861,7 @@ public class NsdService extends INsdManager.Stub { @Override public void dump(FileDescriptor fd, PrintWriter pw, String[] args) { - if (mContext.checkCallingOrSelfPermission(android.Manifest.permission.DUMP) - != PackageManager.PERMISSION_GRANTED) { - pw.println("Permission Denial: can't dump " + TAG - + " due to missing android.permission.DUMP permission"); - return; - } + if (!PermissionUtils.checkDumpPermission(mContext, TAG, pw)) return; for (ClientInfo client : mClients.values()) { pw.println("Client Info"); diff --git a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java index f058f94da5..dae3d2a9d4 100644 --- a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java +++ b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java @@ -32,7 +32,6 @@ import android.net.ITetheredInterfaceCallback; import android.net.IpConfiguration; import android.net.NetworkCapabilities; import android.net.NetworkSpecifier; -import android.os.Binder; import android.os.Handler; import android.os.RemoteException; import android.util.Log; @@ -188,13 +187,7 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { @Override protected void dump(FileDescriptor fd, PrintWriter writer, String[] args) { final IndentingPrintWriter pw = new IndentingPrintWriter(writer, " "); - if (mContext.checkCallingOrSelfPermission(android.Manifest.permission.DUMP) - != PackageManager.PERMISSION_GRANTED) { - pw.println("Permission Denial: can't dump EthernetService from pid=" - + Binder.getCallingPid() - + ", uid=" + Binder.getCallingUid()); - return; - } + if (!PermissionUtils.checkDumpPermission(mContext, TAG, pw)) return; pw.println("Current Ethernet state: "); pw.increaseIndent(); diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index 424dcd999d..e5bd94b891 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -24,7 +24,6 @@ import static android.content.Intent.ACTION_SHUTDOWN; import static android.content.Intent.ACTION_UID_REMOVED; import static android.content.Intent.ACTION_USER_REMOVED; import static android.content.Intent.EXTRA_UID; -import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import static android.net.NetworkStats.DEFAULT_NETWORK_ALL; @@ -2816,24 +2815,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { return stats; } - // TODO: It is copied from ConnectivityService, consider refactor these check permission - // functions to a proper util. - private boolean checkAnyPermissionOf(String... permissions) { - for (String permission : permissions) { - if (mContext.checkCallingOrSelfPermission(permission) == PERMISSION_GRANTED) { - return true; - } - } - return false; - } - - private void enforceAnyPermissionOf(String... permissions) { - if (!checkAnyPermissionOf(permissions)) { - throw new SecurityException("Requires one of the following permissions: " - + String.join(", ", permissions) + "."); - } - } - /** * Registers a custom provider of {@link android.net.NetworkStats} to combine the network * statistics that cannot be seen by the kernel to system. To unregister, invoke the @@ -2848,7 +2829,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { */ public @NonNull INetworkStatsProviderCallback registerNetworkStatsProvider( @NonNull String tag, @NonNull INetworkStatsProvider provider) { - enforceAnyPermissionOf(NETWORK_STATS_PROVIDER, + PermissionUtils.enforceAnyPermissionOf(mContext, NETWORK_STATS_PROVIDER, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); Objects.requireNonNull(provider, "provider is null"); Objects.requireNonNull(tag, "tag is null"); diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index b210bb32be..5cfb98ffe9 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -98,6 +98,9 @@ import static android.system.OsConstants.IPPROTO_TCP; import static android.system.OsConstants.IPPROTO_UDP; import static com.android.net.module.util.DeviceConfigUtils.TETHERING_MODULE_NAME; +import static com.android.net.module.util.PermissionUtils.enforceAnyPermissionOf; +import static com.android.net.module.util.PermissionUtils.enforceNetworkStackPermission; +import static com.android.net.module.util.PermissionUtils.enforceNetworkStackPermissionOr; import static java.util.Map.Entry; @@ -1956,7 +1959,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public Network getActiveNetworkForUid(int uid, boolean ignoreBlocked) { - PermissionUtils.enforceNetworkStackPermission(mContext); + enforceNetworkStackPermission(mContext); return getActiveNetworkForUidInternal(uid, ignoreBlocked); } @@ -1979,7 +1982,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkInfo getActiveNetworkInfoForUid(int uid, boolean ignoreBlocked) { - PermissionUtils.enforceNetworkStackPermission(mContext); + enforceNetworkStackPermission(mContext); final NetworkAgentInfo nai = getNetworkAgentInfoForUid(uid); if (nai == null) return null; return getFilteredNetworkInfo(nai, uid, ignoreBlocked); @@ -2518,7 +2521,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkState[] getAllNetworkState() { // This contains IMSI details, so make sure the caller is privileged. - PermissionUtils.enforceNetworkStackPermission(mContext); + enforceNetworkStackPermission(mContext); final ArrayList result = new ArrayList<>(); for (NetworkStateSnapshot snapshot : getAllNetworkStateSnapshots()) { @@ -2783,15 +2786,6 @@ public class ConnectivityService extends IConnectivityManager.Stub setUidBlockedReasons(uid, blockedReasons); } - private boolean checkAnyPermissionOf(String... permissions) { - for (String permission : permissions) { - if (mContext.checkCallingOrSelfPermission(permission) == PERMISSION_GRANTED) { - return true; - } - } - return false; - } - private boolean checkAnyPermissionOf(int pid, int uid, String... permissions) { for (String permission : permissions) { if (mContext.checkPermission(permission, pid, uid) == PERMISSION_GRANTED) { @@ -2801,13 +2795,6 @@ public class ConnectivityService extends IConnectivityManager.Stub return false; } - private void enforceAnyPermissionOf(String... permissions) { - if (!checkAnyPermissionOf(permissions)) { - throw new SecurityException("Requires one of the following permissions: " - + String.join(", ", permissions) + "."); - } - } - private void enforceInternetPermission() { mContext.enforceCallingOrSelfPermission( android.Manifest.permission.INTERNET, @@ -2867,7 +2854,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } private void enforceSettingsPermission() { - enforceAnyPermissionOf( + enforceAnyPermissionOf(mContext, android.Manifest.permission.NETWORK_SETTINGS, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); } @@ -2875,7 +2862,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private void enforceNetworkFactoryPermission() { // TODO: Check for the BLUETOOTH_STACK permission once that is in the API surface. if (UserHandle.getAppId(getCallingUid()) == Process.BLUETOOTH_UID) return; - enforceAnyPermissionOf( + enforceAnyPermissionOf(mContext, android.Manifest.permission.NETWORK_FACTORY, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); } @@ -2883,7 +2870,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private void enforceNetworkFactoryOrSettingsPermission() { // TODO: Check for the BLUETOOTH_STACK permission once that is in the API surface. if (UserHandle.getAppId(getCallingUid()) == Process.BLUETOOTH_UID) return; - enforceAnyPermissionOf( + enforceAnyPermissionOf(mContext, android.Manifest.permission.NETWORK_SETTINGS, android.Manifest.permission.NETWORK_FACTORY, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); @@ -2892,7 +2879,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private void enforceNetworkFactoryOrTestNetworksPermission() { // TODO: Check for the BLUETOOTH_STACK permission once that is in the API surface. if (UserHandle.getAppId(getCallingUid()) == Process.BLUETOOTH_UID) return; - enforceAnyPermissionOf( + enforceAnyPermissionOf(mContext, android.Manifest.permission.MANAGE_TEST_NETWORKS, android.Manifest.permission.NETWORK_FACTORY, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); @@ -2909,7 +2896,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } private boolean checkSettingsPermission() { - return checkAnyPermissionOf( + return PermissionUtils.checkAnyPermissionOf(mContext, android.Manifest.permission.NETWORK_SETTINGS, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); } @@ -2922,27 +2909,21 @@ public class ConnectivityService extends IConnectivityManager.Stub } private void enforceNetworkStackOrSettingsPermission() { - enforceAnyPermissionOf( - android.Manifest.permission.NETWORK_SETTINGS, - android.Manifest.permission.NETWORK_STACK, - NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); + enforceNetworkStackPermissionOr(mContext, + android.Manifest.permission.NETWORK_SETTINGS); } private void enforceNetworkStackSettingsOrSetup() { - enforceAnyPermissionOf( + enforceNetworkStackPermissionOr(mContext, android.Manifest.permission.NETWORK_SETTINGS, - android.Manifest.permission.NETWORK_SETUP_WIZARD, - android.Manifest.permission.NETWORK_STACK, - NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); + android.Manifest.permission.NETWORK_SETUP_WIZARD); } private void enforceAirplaneModePermission() { - enforceAnyPermissionOf( + enforceNetworkStackPermissionOr(mContext, android.Manifest.permission.NETWORK_AIRPLANE_MODE, android.Manifest.permission.NETWORK_SETTINGS, - android.Manifest.permission.NETWORK_SETUP_WIZARD, - android.Manifest.permission.NETWORK_STACK, - NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); + android.Manifest.permission.NETWORK_SETUP_WIZARD); } private void enforceOemNetworkPreferencesPermission() { @@ -2958,7 +2939,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } private boolean checkNetworkStackPermission() { - return checkAnyPermissionOf( + return PermissionUtils.checkAnyPermissionOf(mContext, android.Manifest.permission.NETWORK_STACK, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); } @@ -5735,7 +5716,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public void setGlobalProxy(@Nullable final ProxyInfo proxyProperties) { - PermissionUtils.enforceNetworkStackPermission(mContext); + enforceNetworkStackPermission(mContext); mProxyTracker.setGlobalProxy(proxyProperties); } @@ -7283,7 +7264,7 @@ public class ConnectivityService extends IConnectivityManager.Stub Objects.requireNonNull(initialScore, "initialScore must not be null"); Objects.requireNonNull(networkAgentConfig, "networkAgentConfig must not be null"); if (networkCapabilities.hasTransport(TRANSPORT_TEST)) { - enforceAnyPermissionOf(Manifest.permission.MANAGE_TEST_NETWORKS); + enforceAnyPermissionOf(mContext, Manifest.permission.MANAGE_TEST_NETWORKS); } else { enforceNetworkFactoryPermission(); } @@ -10296,7 +10277,8 @@ public class ConnectivityService extends IConnectivityManager.Stub Objects.requireNonNull(network, "network must not be null"); Objects.requireNonNull(extras, "extras must not be null"); - enforceAnyPermissionOf(android.Manifest.permission.MANAGE_TEST_NETWORKS, + enforceAnyPermissionOf(mContext, + android.Manifest.permission.MANAGE_TEST_NETWORKS, android.Manifest.permission.NETWORK_STACK); final NetworkCapabilities nc = getNetworkCapabilitiesInternal(network); if (!nc.hasTransport(TRANSPORT_TEST)) { @@ -10704,7 +10686,7 @@ public class ConnectivityService extends IConnectivityManager.Stub preferences.add(pref); } - PermissionUtils.enforceNetworkStackPermission(mContext); + enforceNetworkStackPermission(mContext); if (DBG) { log("setProfileNetworkPreferences " + profile + " to " + preferences); }