WifiConfigManagerNew: Check UID belongs to foreground user
Add a utility method to check if the provided UID belongs to either the
current foreground user or if the SysUI. This is used to check the UID
of the app in all the methods invoked from public WifiManager API's.
This utility method is used to validate the following methods:
1. addOrUpdateNetwork
2. removeNetwork
3. enableNetwork
4. disableNetwork
5. checkAndUpdateLastConnectUid
While there,
a. Add the missing |uid| parameter in |removeNetwork| &
|removeNetworkInternal|. We need to check both if the the uid belongs to
the current user and that the uid has enough permission to delete the
network.
b. Setup the user profiles for the default user in the setup for each test.
BUG: 30891063
Change-Id: Ia03490c9dce7722b8d046a324004448e911257b6
TEST: Added a Unit test.
diff --git a/service/java/com/android/server/wifi/WifiConfigManagerNew.java b/service/java/com/android/server/wifi/WifiConfigManagerNew.java
index 512633c..59b66bd 100644
--- a/service/java/com/android/server/wifi/WifiConfigManagerNew.java
+++ b/service/java/com/android/server/wifi/WifiConfigManagerNew.java
@@ -90,6 +90,12 @@
@VisibleForTesting
public static final String PASSWORD_MASK = "*";
/**
+ * Package name for SysUI. This is used to lookup the UID of SysUI which is used to allow
+ * Quick settings to modify network configurations.
+ */
+ @VisibleForTesting
+ public static final String SYSUI_PACKAGE_NAME = "com.android.systemui";
+ /**
* Network Selection disable reason thresholds. These numbers are used to debounce network
* failures before we disable them.
* These are indexed using the disable reason constants defined in
@@ -164,6 +170,7 @@
* Log tag for this class.
*/
private static final String TAG = "WifiConfigManagerNew";
+
/**
* Disconnected/Connected PnoNetwork list sorting algorithm:
* Place the configurations in descending order of their |numAssociation| values. If networks
@@ -244,7 +251,12 @@
* This is keeping track of the last network ID assigned. Any new networks will be assigned
* |mLastNetworkId + 1| as network ID.
*/
- private int mLastNetworkId;
+ private int mLastNetworkId = 0;
+ /**
+ * UID of system UI. This uid is allowed to modify network configurations regardless of which
+ * user is logged in.
+ */
+ private int mSystemUiUid = -1;
/**
* Create new instance of WifiConfigManager.
@@ -268,6 +280,13 @@
R.bool.config_wifi_only_link_same_credential_configurations);
mMaxNumActiveChannelsForPartialScans = mContext.getResources().getInteger(
R.integer.config_wifi_framework_associated_partial_scan_max_num_active_channels);
+
+ try {
+ mSystemUiUid = mContext.getPackageManager().getPackageUidAsUser(SYSUI_PACKAGE_NAME,
+ PackageManager.MATCH_SYSTEM_ONLY, UserHandle.USER_SYSTEM);
+ } catch (PackageManager.NameNotFoundException e) {
+ Log.e(TAG, "Unable to resolve SystemUI's UID.");
+ }
}
/**
@@ -584,6 +603,19 @@
}
/**
+ * Method to check if the provided UID belongs to the current foreground user or some other
+ * app (only SysUI today) running on behalf of the user.
+ * This is used to prevent any background user apps from modifying network configurations.
+ *
+ * @param uid uid of the app.
+ * @return true if the UID belongs to the current foreground app or SystemUI, false otherwise.
+ */
+ private boolean doesUidBelongToCurrentUser(int uid) {
+ return (WifiConfigurationUtil.doesUidBelongToAnyProfile(
+ uid, mUserManager.getProfiles(mCurrentUserId)) || (uid == mSystemUiUid));
+ }
+
+ /**
* Copy over public elements from an external WifiConfiguration object to the internal
* configuration object if element has been set in the provided external WifiConfiguration.
* The only exception is the hidden |IpConfiguration| parameters, these need to be copied over
@@ -866,10 +898,14 @@
* network configuration. Otherwise, the networkId should refer to an existing configuration.
*
* @param config provided WifiConfiguration object.
- * @param uid UID of the app requesting the network addition/deletion.
+ * @param uid UID of the app requesting the network addition/modification.
* @return NetworkUpdateResult object representing status of the update.
*/
public NetworkUpdateResult addOrUpdateNetwork(WifiConfiguration config, int uid) {
+ if (!doesUidBelongToCurrentUser(uid)) {
+ Log.e(TAG, "UID " + uid + " not visible to the current user");
+ return new NetworkUpdateResult(WifiConfiguration.INVALID_NETWORK_ID);
+ }
if (config == null) {
Log.e(TAG, "Cannot add/update network with null config");
return new NetworkUpdateResult(WifiConfiguration.INVALID_NETWORK_ID);
@@ -921,13 +957,23 @@
* Removes the specified network configuration from our database.
*
* @param networkId network ID of the provided network.
+ * @param uid UID of the app requesting the network deletion.
* @return true if successful, false otherwise.
*/
- public boolean removeNetwork(int networkId) {
+ public boolean removeNetwork(int networkId, int uid) {
+ if (!doesUidBelongToCurrentUser(uid)) {
+ Log.e(TAG, "UID " + uid + " not visible to the current user");
+ return false;
+ }
WifiConfiguration config = getInternalConfiguredNetwork(networkId);
if (config == null) {
return false;
}
+ if (!canModifyNetwork(config, uid, DISALLOW_LOCKDOWN_CHECK_BYPASS)) {
+ Log.e(TAG, "UID " + uid + " does not have permission to delete configuration "
+ + config.configKey());
+ return false;
+ }
if (!removeNetworkInternal(config)) {
Log.e(TAG, "Failed to remove network " + config.getPrintableSsid());
return false;
@@ -1128,6 +1174,10 @@
* @return true if it succeeds, false otherwise
*/
public boolean enableNetwork(int networkId, int uid) {
+ if (!doesUidBelongToCurrentUser(uid)) {
+ Log.e(TAG, "UID " + uid + " not visible to the current user");
+ return false;
+ }
WifiConfiguration config = getInternalConfiguredNetwork(networkId);
if (config == null) {
return false;
@@ -1149,6 +1199,10 @@
* @return true if it succeeds, false otherwise
*/
public boolean disableNetwork(int networkId, int uid) {
+ if (!doesUidBelongToCurrentUser(uid)) {
+ Log.e(TAG, "UID " + uid + " not visible to the current user");
+ return false;
+ }
WifiConfiguration config = getInternalConfiguredNetwork(networkId);
if (config == null) {
return false;
@@ -1172,6 +1226,10 @@
* network, false otherwise.
*/
public boolean checkAndUpdateLastConnectUid(int networkId, int uid) {
+ if (!doesUidBelongToCurrentUser(uid)) {
+ Log.e(TAG, "UID " + uid + " not visible to the current user");
+ return false;
+ }
WifiConfiguration config = getInternalConfiguredNetwork(networkId);
if (config == null) {
return false;
@@ -1837,8 +1895,8 @@
// because all networks were previously stored in a central file. We cannot
// write these private networks to the user specific store until the corresponding
// user logs in.
- if (config.shared || !WifiConfigurationUtil.isVisibleToAnyProfile(
- config, mUserManager.getProfiles(mCurrentUserId))) {
+ if (config.shared || !WifiConfigurationUtil.doesUidBelongToAnyProfile(
+ config.creatorUid, mUserManager.getProfiles(mCurrentUserId))) {
sharedConfigurations.add(config);
} else {
userConfigurations.add(config);
diff --git a/service/java/com/android/server/wifi/WifiConfigurationUtil.java b/service/java/com/android/server/wifi/WifiConfigurationUtil.java
index 8a61eea..0981ac6 100644
--- a/service/java/com/android/server/wifi/WifiConfigurationUtil.java
+++ b/service/java/com/android/server/wifi/WifiConfigurationUtil.java
@@ -49,12 +49,21 @@
* profiles
*/
public static boolean isVisibleToAnyProfile(WifiConfiguration config, List<UserInfo> profiles) {
- if (config.shared) {
- return true;
- }
- final int creatorUserId = UserHandle.getUserId(config.creatorUid);
+ return (config.shared || doesUidBelongToAnyProfile(config.creatorUid, profiles));
+ }
+
+ /**
+ * Check whether a uid belong to a user or any of its managed profiles.
+ *
+ * @param uid uid of the app.
+ * @param profiles the user IDs of the user itself and all its managed profiles (can be obtained
+ * via {@link android.os.UserManager#getProfiles})
+ * @return whether the uid belongs to the user or any of its managed profiles.
+ */
+ public static boolean doesUidBelongToAnyProfile(int uid, List<UserInfo> profiles) {
+ final int userId = UserHandle.getUserId(uid);
for (UserInfo profile : profiles) {
- if (profile.id == creatorUserId) {
+ if (profile.id == userId) {
return true;
}
}
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerNewTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerNewTest.java
index 799f3d2..48fd054 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerNewTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerNewTest.java
@@ -63,6 +63,8 @@
private static final long TEST_ELAPSED_UPDATE_NETWORK_SELECTION_TIME_MILLIS = 29457631;
private static final int TEST_CREATOR_UID = 5;
private static final int TEST_UPDATE_UID = 4;
+ private static final int TEST_SYSUI_UID = 56;
+ private static final int TEST_DEFAULT_USER = UserHandle.USER_SYSTEM;
private static final int TEST_MAX_NUM_ACTIVE_CHANNELS_FOR_PARTIAL_SCAN = 5;
private static final Integer[] TEST_FREQ_LIST = {2400, 2450, 5150, 5175, 5650};
private static final String TEST_CREATOR_NAME = "com.wificonfigmanagerNew.creator";
@@ -102,17 +104,31 @@
TEST_MAX_NUM_ACTIVE_CHANNELS_FOR_PARTIAL_SCAN);
when(mContext.getResources()).thenReturn(mResources);
+ // Setup UserManager profiles for the default user.
+ setupUserProfiles(TEST_DEFAULT_USER);
+
doAnswer(new AnswerWithArguments() {
public String answer(int uid) throws Exception {
if (uid == TEST_CREATOR_UID) {
return TEST_CREATOR_NAME;
} else if (uid == TEST_UPDATE_UID) {
return TEST_UPDATE_NAME;
+ } else if (uid == TEST_SYSUI_UID) {
+ return WifiConfigManagerNew.SYSUI_PACKAGE_NAME;
}
fail("Unexpected UID: " + uid);
return "";
}
}).when(mPackageManager).getNameForUid(anyInt());
+ doAnswer(new AnswerWithArguments() {
+ public int answer(String packageName, int flags, int userId) throws Exception {
+ if (packageName.equals(WifiConfigManagerNew.SYSUI_PACKAGE_NAME)) {
+ return TEST_SYSUI_UID;
+ } else {
+ return 0;
+ }
+ }
+ }).when(mPackageManager).getPackageUidAsUser(anyString(), anyInt(), anyInt());
// Both the UID's in the test have the configuration override permission granted by
// default. This maybe modified for particular tests if needed.
@@ -647,7 +663,7 @@
// Change the networkID to an invalid one.
openNetwork.networkId++;
- assertFalse(mWifiConfigManager.removeNetwork(openNetwork.networkId));
+ assertFalse(mWifiConfigManager.removeNetwork(openNetwork.networkId, TEST_CREATOR_UID));
}
/**
@@ -1585,10 +1601,8 @@
*/
@Test
public void testHandleUserSwitchPushesOtherPrivateNetworksToSharedStore() throws Exception {
- // Set up the user profiles stuff. Needed for |WifiConfigurationUtil.isVisibleToAnyProfile|
- int user1 = UserHandle.USER_SYSTEM;
- int user2 = UserHandle.USER_SYSTEM + 1;
- setupUserProfiles(user1);
+ int user1 = TEST_DEFAULT_USER;
+ int user2 = TEST_DEFAULT_USER + 1;
setupUserProfiles(user2);
int appId = 674;
@@ -1655,10 +1669,8 @@
*/
@Test
public void testHandleUserSwitchWhenUnlocked() throws Exception {
- // Set up the user profiles stuff. Needed for |WifiConfigurationUtil.isVisibleToAnyProfile|
- int user1 = UserHandle.USER_SYSTEM;
- int user2 = UserHandle.USER_SYSTEM + 1;
- setupUserProfiles(user1);
+ int user1 = TEST_DEFAULT_USER;
+ int user2 = TEST_DEFAULT_USER + 1;
setupUserProfiles(user2);
when(mWifiConfigStore.read()).thenReturn(
@@ -1683,10 +1695,8 @@
* read until the user is unlocked.
*/
public void testHandleUserSwitchWhenLocked() throws Exception {
- // Set up the user profiles stuff. Needed for |WifiConfigurationUtil.isVisibleToAnyProfile|
- int user1 = UserHandle.USER_SYSTEM;
- int user2 = UserHandle.USER_SYSTEM + 1;
- setupUserProfiles(user1);
+ int user1 = TEST_DEFAULT_USER;
+ int user2 = TEST_DEFAULT_USER + 1;
setupUserProfiles(user2);
// user2 is locked and switched to foreground.
@@ -1716,10 +1726,8 @@
*/
@Test
public void testHandleUserStop() throws Exception {
- // Set up the user profiles stuff. Needed for |WifiConfigurationUtil.isVisibleToAnyProfile|
- int user1 = UserHandle.USER_SYSTEM;
- int user2 = UserHandle.USER_SYSTEM + 1;
- setupUserProfiles(user1);
+ int user1 = TEST_DEFAULT_USER;
+ int user2 = TEST_DEFAULT_USER + 1;
setupUserProfiles(user2);
// Try stopping background user2 first, this should not do anything.
@@ -1734,6 +1742,46 @@
anyBoolean(), any(WifiConfigStoreData.class));
}
+ /**
+ * Verifies the private network addition using
+ * {@link WifiConfigManagerNew#addOrUpdateNetwork(WifiConfiguration, int)}
+ * by a non foreground user is rejected.
+ */
+ @Test
+ public void testAddNetworkUsingBackgroundUserUId() throws Exception {
+ int user2 = TEST_DEFAULT_USER + 1;
+ setupUserProfiles(user2);
+
+ int creatorUid = UserHandle.getUid(user2, 674);
+
+ // Create a network for user2 try adding it. This should be rejected.
+ final WifiConfiguration user2Network = WifiConfigurationTestUtil.createPskNetwork();
+ NetworkUpdateResult result =
+ mWifiConfigManager.addOrUpdateNetwork(user2Network, creatorUid);
+ assertFalse(result.isSuccess());
+ }
+
+ /**
+ * Verifies the private network addition using
+ * {@link WifiConfigManagerNew#addOrUpdateNetwork(WifiConfiguration, int)}
+ * by SysUI is always accepted.
+ */
+ @Test
+ public void testAddNetworkUsingSysUiUid() throws Exception {
+ // Set up the user profiles stuff. Needed for |WifiConfigurationUtil.isVisibleToAnyProfile|
+ int user2 = TEST_DEFAULT_USER + 1;
+ setupUserProfiles(user2);
+
+ when(mUserManager.isUserUnlockingOrUnlocked(user2)).thenReturn(false);
+ mWifiConfigManager.handleUserSwitch(user2);
+
+ // Create a network for user2 try adding it. This should be rejected.
+ final WifiConfiguration user2Network = WifiConfigurationTestUtil.createPskNetwork();
+ NetworkUpdateResult result =
+ mWifiConfigManager.addOrUpdateNetwork(user2Network, TEST_SYSUI_UID);
+ assertTrue(result.isSuccess());
+ }
+
private void createWifiConfigManager() {
mWifiConfigManager =
new WifiConfigManagerNew(
@@ -2083,7 +2131,7 @@
*/
private void verifyRemoveNetworkFromWifiConfigManager(
WifiConfiguration configuration) {
- assertTrue(mWifiConfigManager.removeNetwork(configuration.networkId));
+ assertTrue(mWifiConfigManager.removeNetwork(configuration.networkId, TEST_CREATOR_UID));
verifyNetworkRemoveBroadcast(configuration);
// Verify if the config store write was triggered without this new configuration.