From b73e0a91dcf3fa04cf07c2d096fb20c229d6cd28 Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Fri, 15 Sep 2017 14:45:40 -0700 Subject: NetworkListStoreData: Set creatorUid for all networks on load Some network configurations saved during N have the creatorUid set to -1. (Not sure what was the root cause in N which introduced this issue in saved networks) Ensure that we set |creatorUid| for such networks with invalid uid to SYSTEM_UID on loading. The networks with invalid creatorUid can anyway only be modified only by settings/sysui (because they possess NETWORK_SETTINGS permission). Bug: 65623732 Test: Unit tests Change-Id: Ib6f61b81e19877cdf0c42d29c9f5ae3869725517 --- .../server/wifi/NetworkListStoreDataTest.java | 119 +++++++++++++++++++-- .../android/server/wifi/WifiConfigStoreTest.java | 17 ++- 2 files changed, 125 insertions(+), 11 deletions(-) (limited to 'tests') diff --git a/tests/wifitests/src/com/android/server/wifi/NetworkListStoreDataTest.java b/tests/wifitests/src/com/android/server/wifi/NetworkListStoreDataTest.java index cbad3bbec..19a92b8e4 100644 --- a/tests/wifitests/src/com/android/server/wifi/NetworkListStoreDataTest.java +++ b/tests/wifitests/src/com/android/server/wifi/NetworkListStoreDataTest.java @@ -16,9 +16,13 @@ package com.android.server.wifi; +import static android.os.Process.SYSTEM_UID; + import static org.junit.Assert.*; import static org.mockito.Mockito.*; +import android.content.Context; +import android.content.pm.PackageManager; import android.net.wifi.WifiConfiguration; import android.net.wifi.WifiEnterpriseConfig; import android.test.suitebuilder.annotation.SmallTest; @@ -29,6 +33,8 @@ import com.android.server.wifi.util.XmlUtilTest; import org.junit.Before; import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; import org.xmlpull.v1.XmlSerializer; @@ -49,6 +55,7 @@ public class NetworkListStoreDataTest { private static final String TEST_SSID = "WifiConfigStoreDataSSID_"; private static final String TEST_CONNECT_CHOICE = "XmlUtilConnectChoice"; private static final long TEST_CONNECT_CHOICE_TIMESTAMP = 0x4566; + private static final String TEST_CREATOR_NAME = "CreatorName"; private static final String SINGLE_OPEN_NETWORK_DATA_XML_STRING_FORMAT = "\n" + "\n" @@ -79,7 +86,7 @@ public class NetworkListStoreDataTest { + "\n" + "\n" + "\n" - + "\n" + + "%s\n" + "\n" + "\n" + "\n" @@ -130,7 +137,7 @@ public class NetworkListStoreDataTest { + "\n" + "\n" + "\n" - + "\n" + + "%s\n" + "\n" + "\n" + "\n" @@ -170,10 +177,15 @@ public class NetworkListStoreDataTest { + "\n"; private NetworkListStoreData mNetworkListStoreData; + @Mock private Context mContext; + @Mock private PackageManager mPackageManager; @Before public void setUp() throws Exception { - mNetworkListStoreData = new NetworkListStoreData(); + MockitoAnnotations.initMocks(this); + when(mContext.getPackageManager()).thenReturn(mPackageManager); + when(mPackageManager.getNameForUid(anyInt())).thenReturn(TEST_CREATOR_NAME); + mNetworkListStoreData = new NetworkListStoreData(mContext); } /** @@ -221,11 +233,13 @@ public class NetworkListStoreDataTest { */ private List getTestNetworksConfig(boolean shared) { WifiConfiguration openNetwork = WifiConfigurationTestUtil.createOpenNetwork(); + openNetwork.creatorName = TEST_CREATOR_NAME; openNetwork.shared = shared; openNetwork.setIpConfiguration( WifiConfigurationTestUtil.createDHCPIpConfigurationWithNoProxy()); WifiConfiguration eapNetwork = WifiConfigurationTestUtil.createEapNetwork(); eapNetwork.shared = shared; + eapNetwork.creatorName = TEST_CREATOR_NAME; eapNetwork.setIpConfiguration( WifiConfigurationTestUtil.createDHCPIpConfigurationWithNoProxy()); List networkList = new ArrayList<>(); @@ -247,11 +261,11 @@ public class NetworkListStoreDataTest { String openNetworkXml = String.format(SINGLE_OPEN_NETWORK_DATA_XML_STRING_FORMAT, openNetwork.configKey().replaceAll("\"", """), openNetwork.SSID.replaceAll("\"", """), - openNetwork.shared, openNetwork.creatorUid); + openNetwork.shared, openNetwork.creatorUid, openNetwork.creatorName); String eapNetworkXml = String.format(SINGLE_EAP_NETWORK_DATA_XML_STRING_FORMAT, eapNetwork.configKey().replaceAll("\"", """), eapNetwork.SSID.replaceAll("\"", """), - eapNetwork.shared, eapNetwork.creatorUid); + eapNetwork.shared, eapNetwork.creatorUid, openNetwork.creatorName); return (openNetworkXml + eapNetworkXml).getBytes(StandardCharsets.UTF_8); } @@ -420,7 +434,8 @@ public class NetworkListStoreDataTest { byte[] xmlData = String.format(SINGLE_OPEN_NETWORK_DATA_XML_STRING_FORMAT, "InvalidConfigKey", openNetwork.SSID.replaceAll("\"", """), - openNetwork.shared, openNetwork.creatorUid).getBytes(StandardCharsets.UTF_8); + openNetwork.shared, openNetwork.creatorUid, openNetwork.creatorName) + .getBytes(StandardCharsets.UTF_8); deserializeData(xmlData, true); } @@ -448,4 +463,96 @@ public class NetworkListStoreDataTest { assertNotEquals(eapNetwork.SSID, network.SSID); } } + + /** + * Verify that a saved network config with invalid creatorUid resets it to + * {@link android.os.Process#SYSTEM_UID}. + */ + public void parseNetworkWithInvalidCreatorUidResetsToSystem() throws Exception { + WifiConfiguration openNetwork = WifiConfigurationTestUtil.createOpenNetwork(); + openNetwork.creatorUid = -1; + // Return null for invalid uid. + when(mPackageManager.getNameForUid(eq(openNetwork.creatorUid))).thenReturn(null); + + byte[] xmlData = String.format(SINGLE_OPEN_NETWORK_DATA_XML_STRING_FORMAT, + openNetwork.configKey().replaceAll("\"", """), + openNetwork.SSID.replaceAll("\"", """), + openNetwork.shared, openNetwork.creatorUid, openNetwork.creatorName) + .getBytes(StandardCharsets.UTF_8); + List deserializedNetworks = deserializeData(xmlData, true); + assertEquals(1, deserializedNetworks.size()); + assertEquals(openNetwork.configKey(), deserializedNetworks.get(0).configKey()); + assertEquals(SYSTEM_UID, deserializedNetworks.get(0).creatorUid); + assertEquals(TEST_CREATOR_NAME, deserializedNetworks.get(0).creatorName); + } + + /** + * Verify that a saved network config with invalid creatorName resets it to the package name + * provided {@link PackageManager} for the creatorUid. + */ + public void parseNetworkWithInvalidCreatorNameResetsToPackageNameForCreatorUid() + throws Exception { + String badCreatorName = "bad"; + String correctCreatorName = "correct"; + WifiConfiguration openNetwork = WifiConfigurationTestUtil.createOpenNetwork(); + openNetwork.creatorUid = 1324422; + openNetwork.creatorName = badCreatorName; + when(mPackageManager.getNameForUid(eq(openNetwork.creatorUid))) + .thenReturn(correctCreatorName); + + byte[] xmlData = String.format(SINGLE_OPEN_NETWORK_DATA_XML_STRING_FORMAT, + openNetwork.configKey().replaceAll("\"", """), + openNetwork.SSID.replaceAll("\"", """), + openNetwork.shared, openNetwork.creatorUid, openNetwork.creatorName) + .getBytes(StandardCharsets.UTF_8); + List deserializedNetworks = deserializeData(xmlData, true); + assertEquals(1, deserializedNetworks.size()); + assertEquals(openNetwork.configKey(), deserializedNetworks.get(0).configKey()); + assertEquals(openNetwork.creatorUid, deserializedNetworks.get(0).creatorUid); + assertEquals(correctCreatorName, deserializedNetworks.get(0).creatorName); + } + + /** + * Verify that a saved network config with invalid creatorName resets it to the package name + * provided {@link PackageManager} for the creatorUid. + */ + public void parseNetworkWithNullCreatorNameResetsToPackageNameForCreatorUid() + throws Exception { + String correctCreatorName = "correct"; + WifiConfiguration openNetwork = WifiConfigurationTestUtil.createOpenNetwork(); + openNetwork.creatorUid = 1324422; + openNetwork.creatorName = null; + when(mPackageManager.getNameForUid(eq(openNetwork.creatorUid))) + .thenReturn(correctCreatorName); + + byte[] xmlData = String.format(SINGLE_OPEN_NETWORK_DATA_XML_STRING_FORMAT, + openNetwork.configKey().replaceAll("\"", """), + openNetwork.SSID.replaceAll("\"", """), + openNetwork.shared, openNetwork.creatorUid, openNetwork.creatorName) + .getBytes(StandardCharsets.UTF_8); + List deserializedNetworks = deserializeData(xmlData, true); + assertEquals(1, deserializedNetworks.size()); + assertEquals(openNetwork.configKey(), deserializedNetworks.get(0).configKey()); + assertEquals(openNetwork.creatorUid, deserializedNetworks.get(0).creatorUid); + assertEquals(correctCreatorName, deserializedNetworks.get(0).creatorName); + } + + /** + * Verify that a saved network config with valid creatorUid is preserved. + */ + public void parseNetworkWithValidCreatorUid() throws Exception { + WifiConfiguration openNetwork = WifiConfigurationTestUtil.createOpenNetwork(); + openNetwork.creatorUid = 1324422; + + byte[] xmlData = String.format(SINGLE_OPEN_NETWORK_DATA_XML_STRING_FORMAT, + openNetwork.configKey().replaceAll("\"", """), + openNetwork.SSID.replaceAll("\"", """), + openNetwork.shared, openNetwork.creatorUid, openNetwork.creatorName) + .getBytes(StandardCharsets.UTF_8); + List deserializedNetworks = deserializeData(xmlData, true); + assertEquals(1, deserializedNetworks.size()); + assertEquals(openNetwork.configKey(), deserializedNetworks.get(0).configKey()); + assertEquals(openNetwork.creatorUid, deserializedNetworks.get(0).creatorUid); + assertEquals(TEST_CREATOR_NAME, deserializedNetworks.get(0).creatorName); + } } diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreTest.java index 86d6e11e7..0958fea61 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreTest.java @@ -21,6 +21,7 @@ import static org.mockito.Mockito.*; import android.app.test.TestAlarmManager; import android.content.Context; +import android.content.pm.PackageManager; import android.net.wifi.WifiConfiguration; import android.os.test.TestLooper; import android.test.suitebuilder.annotation.SmallTest; @@ -60,6 +61,7 @@ public class WifiConfigStoreTest { private static final String TEST_USER_DATA = "UserData"; private static final String TEST_SHARE_DATA = "ShareData"; + private static final String TEST_CREATOR_NAME = "CreatorName"; private static final String TEST_DATA_XML_STRING_FORMAT = "\n" @@ -95,7 +97,7 @@ public class WifiConfigStoreTest { + "\n" + "\n" + "\n" - + "\n" + + "%s\n" + "\n" + "\n" + "\n" @@ -125,6 +127,7 @@ public class WifiConfigStoreTest { // Test mocks @Mock private Context mContext; + @Mock private PackageManager mPackageManager; private TestAlarmManager mAlarmManager; private TestLooper mLooper; @Mock private Clock mClock; @@ -146,6 +149,8 @@ public class WifiConfigStoreTest { mLooper = new TestLooper(); when(mContext.getSystemService(Context.ALARM_SERVICE)) .thenReturn(mAlarmManager.getAlarmManager()); + when(mContext.getPackageManager()).thenReturn(mPackageManager); + when(mPackageManager.getNameForUid(anyInt())).thenReturn(TEST_CREATOR_NAME); mUserStore = new MockStoreFile(); mSharedStore = new MockStoreFile(); mStoreData = new MockStoreData(); @@ -364,9 +369,10 @@ public class WifiConfigStoreTest { @Test public void testReadWifiConfigStoreData() throws Exception { // Setup network list. - NetworkListStoreData networkList = new NetworkListStoreData(); + NetworkListStoreData networkList = new NetworkListStoreData(mContext); mWifiConfigStore.registerStoreData(networkList); WifiConfiguration openNetwork = WifiConfigurationTestUtil.createOpenNetwork(); + openNetwork.creatorName = TEST_CREATOR_NAME; openNetwork.setIpConfiguration( WifiConfigurationTestUtil.createDHCPIpConfigurationWithNoProxy()); List userConfigs = new ArrayList<>(); @@ -384,7 +390,7 @@ public class WifiConfigStoreTest { String xmlString = String.format(TEST_DATA_XML_STRING_FORMAT, openNetwork.configKey().replaceAll("\"", """), openNetwork.SSID.replaceAll("\"", """), - openNetwork.shared, openNetwork.creatorUid, testSsid); + openNetwork.shared, openNetwork.creatorUid, openNetwork.creatorName, testSsid); byte[] xmlBytes = xmlString.getBytes(StandardCharsets.UTF_8); mUserStore.storeRawDataToWrite(xmlBytes); @@ -406,9 +412,10 @@ public class WifiConfigStoreTest { mWifiConfigStore.switchUserStoreAndRead(mUserStore); // Setup network list store data. - NetworkListStoreData networkList = new NetworkListStoreData(); + NetworkListStoreData networkList = new NetworkListStoreData(mContext); mWifiConfigStore.registerStoreData(networkList); WifiConfiguration openNetwork = WifiConfigurationTestUtil.createOpenNetwork(); + openNetwork.creatorName = TEST_CREATOR_NAME; openNetwork.setIpConfiguration( WifiConfigurationTestUtil.createDHCPIpConfigurationWithNoProxy()); List userConfigs = new ArrayList<>(); @@ -428,7 +435,7 @@ public class WifiConfigStoreTest { String xmlString = String.format(TEST_DATA_XML_STRING_FORMAT, openNetwork.configKey().replaceAll("\"", """), openNetwork.SSID.replaceAll("\"", """), - openNetwork.shared, openNetwork.creatorUid, testSsid); + openNetwork.shared, openNetwork.creatorUid, openNetwork.creatorName, testSsid); byte[] xmlBytes = xmlString.getBytes(StandardCharsets.UTF_8); mWifiConfigStore.write(true); -- cgit v1.2.3 From 7cede48303b0faabb3b6861a1ac7229f76fca006 Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Mon, 25 Sep 2017 14:51:37 -0700 Subject: WifiStateMachine: Handle WifiManager.save() when wifi is off Bug: 66909738 Test: Verified that you can save networks even when wifi is off using ag/2930541. Test: Unit tests Change-Id: I38c316456e74ce5ad1ec43750b075f0999c28327 --- .../android/server/wifi/WifiStateMachineTest.java | 137 +++++++++++++++++---- 1 file changed, 116 insertions(+), 21 deletions(-) (limited to 'tests') diff --git a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java index 07840fdac..d54e8f877 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java @@ -139,6 +139,7 @@ public class WifiStateMachineTest { : WifiStateMachine.NUM_LOG_RECS_VERBOSE); private static final int FRAMEWORK_NETWORK_ID = 7; private static final int TEST_RSSI = -54; + private static final int TEST_NETWORK_ID = 54; private static final int WPS_SUPPLICANT_NETWORK_ID = 5; private static final int WPS_FRAMEWORK_NETWORK_ID = 10; private static final String DEFAULT_TEST_SSID = "\"GoogleGuest\""; @@ -750,18 +751,15 @@ public class WifiStateMachineTest { (Intent) argThat(new WifiEnablingStateIntentMatcher()), any()); } - /** - * Verifies that configs can be removed when in client mode. - */ - @Test - public void canRemoveNetworkConfigInClientMode() throws Exception { + private void canRemoveNetwork() { boolean result; when(mWifiConfigManager.removeNetwork(eq(0), anyInt())).thenReturn(true); - initializeAndAddNetworkAndVerifySuccess(); mLooper.startAutoDispatch(); result = mWsm.syncRemoveNetwork(mWsmAsyncChannel, 0); mLooper.stopAutoDispatch(); + assertTrue(result); + verify(mWifiConfigManager).removeNetwork(anyInt(), anyInt()); } /** @@ -769,23 +767,21 @@ public class WifiStateMachineTest { */ @Test public void canRemoveNetworkConfigWhenWifiDisabled() { - boolean result; - when(mWifiConfigManager.removeNetwork(eq(0), anyInt())).thenReturn(true); - mLooper.startAutoDispatch(); - result = mWsm.syncRemoveNetwork(mWsmAsyncChannel, 0); - mLooper.stopAutoDispatch(); - - assertTrue(result); - verify(mWifiConfigManager).removeNetwork(anyInt(), anyInt()); + canRemoveNetwork(); } + /** - * Verifies that configs can be forgotten when in client mode. + * Verifies that configs can be removed when in client mode. */ @Test - public void canForgetNetworkConfigInClientMode() throws Exception { - when(mWifiConfigManager.removeNetwork(eq(0), anyInt())).thenReturn(true); + public void canRemoveNetworkConfigInClientMode() throws Exception { initializeAndAddNetworkAndVerifySuccess(); + canRemoveNetwork(); + } + + private void canForgetNetwork() { + when(mWifiConfigManager.removeNetwork(eq(0), anyInt())).thenReturn(true); mWsm.sendMessage(WifiManager.FORGET_NETWORK, 0, MANAGED_PROFILE_UID); mLooper.dispatchAll(); verify(mWifiConfigManager).removeNetwork(anyInt(), anyInt()); @@ -796,10 +792,109 @@ public class WifiStateMachineTest { */ @Test public void canForgetNetworkConfigWhenWifiDisabled() throws Exception { - when(mWifiConfigManager.removeNetwork(eq(0), anyInt())).thenReturn(true); - mWsm.sendMessage(WifiManager.FORGET_NETWORK, 0, MANAGED_PROFILE_UID); - mLooper.dispatchAll(); - verify(mWifiConfigManager).removeNetwork(anyInt(), anyInt()); + canForgetNetwork(); + } + + /** + * Verifies that configs can be forgotten when in client mode. + */ + @Test + public void canForgetNetworkConfigInClientMode() throws Exception { + initializeAndAddNetworkAndVerifySuccess(); + canForgetNetwork(); + } + + private void canSaveNetworkConfig() { + WifiConfiguration config = WifiConfigurationTestUtil.createOpenNetwork(); + + int networkId = TEST_NETWORK_ID; + when(mWifiConfigManager.addOrUpdateNetwork(any(WifiConfiguration.class), anyInt())) + .thenReturn(new NetworkUpdateResult(networkId)); + when(mWifiConfigManager.enableNetwork(eq(networkId), eq(false), anyInt())) + .thenReturn(true); + + mLooper.startAutoDispatch(); + Message reply = mWsmAsyncChannel.sendMessageSynchronously(WifiManager.SAVE_NETWORK, config); + mLooper.stopAutoDispatch(); + assertEquals(WifiManager.SAVE_NETWORK_SUCCEEDED, reply.what); + + verify(mWifiConfigManager).addOrUpdateNetwork(any(WifiConfiguration.class), anyInt()); + verify(mWifiConfigManager).enableNetwork(eq(networkId), eq(false), anyInt()); + } + + /** + * Verifies that configs can be saved when not in client mode. + */ + @Test + public void canSaveNetworkConfigWhenWifiDisabled() throws Exception { + canSaveNetworkConfig(); + } + + /** + * Verifies that configs can be saved when in client mode. + */ + @Test + public void canSaveNetworkConfigInClientMode() throws Exception { + loadComponentsInStaMode(); + canSaveNetworkConfig(); + } + + /** + * Verifies that null configs are rejected in SAVE_NETWORK message. + */ + @Test + public void saveNetworkConfigFailsWithNullConfig() throws Exception { + mLooper.startAutoDispatch(); + Message reply = mWsmAsyncChannel.sendMessageSynchronously(WifiManager.SAVE_NETWORK, null); + mLooper.stopAutoDispatch(); + assertEquals(WifiManager.SAVE_NETWORK_FAILED, reply.what); + + verify(mWifiConfigManager, never()) + .addOrUpdateNetwork(any(WifiConfiguration.class), anyInt()); + verify(mWifiConfigManager, never()) + .enableNetwork(anyInt(), anyBoolean(), anyInt()); + } + + /** + * Verifies that configs save fails when the addition of network fails. + */ + @Test + public void saveNetworkConfigFailsWithConfigAddFailure() throws Exception { + WifiConfiguration config = WifiConfigurationTestUtil.createOpenNetwork(); + + when(mWifiConfigManager.addOrUpdateNetwork(any(WifiConfiguration.class), anyInt())) + .thenReturn(new NetworkUpdateResult(WifiConfiguration.INVALID_NETWORK_ID)); + + mLooper.startAutoDispatch(); + Message reply = mWsmAsyncChannel.sendMessageSynchronously(WifiManager.SAVE_NETWORK, config); + mLooper.stopAutoDispatch(); + assertEquals(WifiManager.SAVE_NETWORK_FAILED, reply.what); + + verify(mWifiConfigManager).addOrUpdateNetwork(any(WifiConfiguration.class), anyInt()); + verify(mWifiConfigManager, never()) + .enableNetwork(anyInt(), anyBoolean(), anyInt()); + } + + /** + * Verifies that configs save fails when the enable of network fails. + */ + @Test + public void saveNetworkConfigFailsWithConfigEnableFailure() throws Exception { + WifiConfiguration config = WifiConfigurationTestUtil.createOpenNetwork(); + + int networkId = 5; + when(mWifiConfigManager.addOrUpdateNetwork(any(WifiConfiguration.class), anyInt())) + .thenReturn(new NetworkUpdateResult(networkId)); + when(mWifiConfigManager.enableNetwork(eq(networkId), eq(false), anyInt())) + .thenReturn(false); + + mLooper.startAutoDispatch(); + Message reply = mWsmAsyncChannel.sendMessageSynchronously(WifiManager.SAVE_NETWORK, config); + mLooper.stopAutoDispatch(); + assertEquals(WifiManager.SAVE_NETWORK_FAILED, reply.what); + + verify(mWifiConfigManager).addOrUpdateNetwork(any(WifiConfiguration.class), anyInt()); + verify(mWifiConfigManager).enableNetwork(eq(networkId), eq(false), anyInt()); } /** -- cgit v1.2.3