diff options
author | Roshan Pius <rpius@google.com> | 2020-04-09 16:41:30 -0700 |
---|---|---|
committer | Roshan Pius <rpius@google.com> | 2020-04-11 04:38:48 +0000 |
commit | 88624ac099ce8ecc04d05e59294a5900479e49e1 (patch) | |
tree | b0b30da6eccf6c2cd5611e0f010e6a7206340a38 /tests | |
parent | b69b89ff86db7b3e9f4263e7bb7fb48dba69832a (diff) |
NetworkListStoreData: Fix badly formed WifiConfiguration
The network config created by the app_XXX in the associated bug had
preSharedKey set, but did not have KeyMgmt.WPA_PSK bit set. It instead
had a KeyMgmt.OSEN bit set (which is not even public, so not sure why
the app is setting that). The user has a saved network with the same SSID in
the XML file. So it looks like the app tried to add the same network
back for some reason, but it had the wrong bits set in KeyMgmt. So, we
ended up creating a different network with configKey set to "SSID"NONE.
This was stored in the XML file, but when the user upgraded to a build
containing ag/10449860, the calculated configKey became "SSID"OSEN
which did not match the configKey stored in the XML file. Hence the
parsing logic errored out.
2 fixes added:
a) Add a generic correction logic in XML file parsing logic to reset
the KeyMgmt to WPA_PSK if it detects a preSharedKey, but KeyMgmt not
set appropriately.
b) Apply the same fix in validate() method. We could have error'ed
out here because it is an invalid WifiConfiguration. But, fixing
in-place will avoid a lot of app compatibility issues.
Bug: 153435438
Test: atest com.android.server.wifi
Change-Id: Idc864d3a9205337f0ebeb6f1057a723f2e6ea36f
Diffstat (limited to 'tests')
4 files changed, 122 insertions, 4 deletions
diff --git a/tests/wifitests/src/com/android/server/wifi/NetworkListStoreDataTest.java b/tests/wifitests/src/com/android/server/wifi/NetworkListStoreDataTest.java index 4e2eee4c5..d04076aa7 100644 --- a/tests/wifitests/src/com/android/server/wifi/NetworkListStoreDataTest.java +++ b/tests/wifitests/src/com/android/server/wifi/NetworkListStoreDataTest.java @@ -253,6 +253,69 @@ public class NetworkListStoreDataTest extends WifiBaseTest { + "</IpConfiguration>\n" + "</Network>\n"; + /** + * Repro'es the scenario in b/153435438. + * Network has + * - Valid preSharedKey + * - KeyMgmt set to KeyMgmt.OSEN + * - ConfigKey set to "SSID"NONE + */ + private static final String SINGLE_INVALID_NETWORK_DATA_XML_STRING_FORMAT = + "<Network>\n" + + "<WifiConfiguration>\n" + + "<string name=\"ConfigKey\">%s</string>\n" + + "<string name=\"SSID\">%s</string>\n" + + "<string name=\"PreSharedKey\">%s</string>\n" + + "<null name=\"WEPKeys\" />\n" + + "<int name=\"WEPTxKeyIndex\" value=\"0\" />\n" + + "<boolean name=\"HiddenSSID\" value=\"false\" />\n" + + "<boolean name=\"RequirePMF\" value=\"false\" />\n" + + "<byte-array name=\"AllowedKeyMgmt\" num=\"1\">20</byte-array>\n" + + "<byte-array name=\"AllowedProtocols\" num=\"0\"></byte-array>\n" + + "<byte-array name=\"AllowedAuthAlgos\" num=\"0\"></byte-array>\n" + + "<byte-array name=\"AllowedGroupCiphers\" num=\"0\"></byte-array>\n" + + "<byte-array name=\"AllowedPairwiseCiphers\" num=\"0\"></byte-array>\n" + + "<byte-array name=\"AllowedGroupMgmtCiphers\" num=\"0\"></byte-array>\n" + + "<byte-array name=\"AllowedSuiteBCiphers\" num=\"0\"></byte-array>\n" + + "<boolean name=\"Shared\" value=\"%s\" />\n" + + "<boolean name=\"AutoJoinEnabled\" value=\"true\" />\n" + + "<boolean name=\"Trusted\" value=\"true\" />\n" + + "<null name=\"BSSID\" />\n" + + "<int name=\"Status\" value=\"2\" />\n" + + "<null name=\"FQDN\" />\n" + + "<null name=\"ProviderFriendlyName\" />\n" + + "<null name=\"LinkedNetworksList\" />\n" + + "<null name=\"DefaultGwMacAddress\" />\n" + + "<boolean name=\"ValidatedInternetAccess\" value=\"false\" />\n" + + "<boolean name=\"NoInternetAccessExpected\" value=\"false\" />\n" + + "<boolean name=\"MeteredHint\" value=\"false\" />\n" + + "<int name=\"MeteredOverride\" value=\"0\" />\n" + + "<boolean name=\"UseExternalScores\" value=\"false\" />\n" + + "<int name=\"NumAssociation\" value=\"0\" />\n" + + "<int name=\"CreatorUid\" value=\"%d\" />\n" + + "<string name=\"CreatorName\">%s</string>\n" + + "<int name=\"LastUpdateUid\" value=\"-1\" />\n" + + "<null name=\"LastUpdateName\" />\n" + + "<int name=\"LastConnectUid\" value=\"0\" />\n" + + "<boolean name=\"IsLegacyPasspointConfig\" value=\"false\" />\n" + + "<long-array name=\"RoamingConsortiumOIs\" num=\"0\" />\n" + + "<string name=\"RandomizedMacAddress\">%s</string>\n" + + "<int name=\"MacRandomizationSetting\" value=\"1\" />\n" + + "<int name=\"CarrierId\" value=\"-1\" />\n" + + "<boolean name=\"IsMostRecentlyConnected\" value=\"false\" />\n" + + "</WifiConfiguration>\n" + + "<NetworkStatus>\n" + + "<string name=\"SelectionStatus\">NETWORK_SELECTION_ENABLED</string>\n" + + "<string name=\"DisableReason\">NETWORK_SELECTION_ENABLE</string>\n" + + "<null name=\"ConnectChoice\" />\n" + + "<boolean name=\"HasEverConnected\" value=\"false\" />\n" + + "</NetworkStatus>\n" + + "<IpConfiguration>\n" + + "<string name=\"IpAssignment\">UNASSIGNED</string>\n" + + "<string name=\"ProxySettings\">UNASSIGNED</string>\n" + + "</IpConfiguration>\n" + + "</Network>\n"; + // We use {@link NetworkListSharedStoreData} instance because {@link NetworkListStoreData} is // abstract. private NetworkListSharedStoreData mNetworkListSharedStoreData; @@ -714,4 +777,36 @@ public class NetworkListStoreDataTest extends WifiBaseTest { byte[] output2 = serializeData(); assertArrayEquals(output2, output1); } + + /** + * Verify that we parse out a badly formed WifiConfiguration saved on the device because + * our previous validation logic did not catch it. + * + * See b/153435438#comment26 for the exact problem. + */ + @Test + public void parseNetworkWithInvalidConfigKeyAndKeyMmt() throws Exception { + // Valid psk network (that we should recreate after deserialization) + WifiConfiguration pskNetwork = WifiConfigurationTestUtil.createPskNetwork(); + pskNetwork.setRandomizedMacAddress(TEST_RANDOMIZED_MAC); + pskNetwork.creatorName = TEST_CREATOR_NAME; + String invalidConfigKey = pskNetwork.getKey(); + invalidConfigKey.replace("WPA_PSK", "NONE"); + // XML data has 2 things that needs to be corrected: + // - ConfigKey is set to "SSID"NONE instead of "SSID"WPA_PSK + // - KeyMgmt has KeyMgmt.OSEN bit set instead of KeyMgmt.WPA_PSK + byte[] xmlData = String.format(SINGLE_INVALID_NETWORK_DATA_XML_STRING_FORMAT, + invalidConfigKey, + pskNetwork.SSID.replaceAll("\"", """), + pskNetwork.preSharedKey.replaceAll("\"", """), + pskNetwork.shared, pskNetwork.creatorUid, + pskNetwork.creatorName, pskNetwork.getRandomizedMacAddress()) + .getBytes(StandardCharsets.UTF_8); + List<WifiConfiguration> deserializedNetworks = deserializeData(xmlData); + assertEquals(1, deserializedNetworks.size()); + + WifiConfiguration deserializedPskNetwork = deserializedNetworks.get(0); + WifiConfigurationTestUtil.assertConfigurationEqualForConfigStore( + pskNetwork, deserializedPskNetwork); + } } diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java index d99381097..8e199ec00 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java @@ -1997,7 +1997,8 @@ public class WifiConfigManagerTest extends WifiBaseTest { assertFalse(pskNetwork.allowedKeyManagement.get(WifiConfiguration.KeyMgmt.IEEE8021X)); pskNetwork.allowedKeyManagement.clear(); - pskNetwork.allowedKeyManagement.set(WifiConfiguration.KeyMgmt.IEEE8021X); + pskNetwork.allowedKeyManagement.set(WifiConfiguration.KeyMgmt.SAE); + pskNetwork.requirePmf = true; verifyUpdateNetworkWithCredentialChangeHasEverConnectedFalse(pskNetwork); } @@ -2121,7 +2122,7 @@ public class WifiConfigManagerTest extends WifiBaseTest { @Test public void testUpdateIgnoresMaskedPasswords() { WifiConfiguration someRandomNetworkWithAllMaskedFields = - WifiConfigurationTestUtil.createEapNetwork(); + WifiConfigurationTestUtil.createPskNetwork(); someRandomNetworkWithAllMaskedFields.wepKeys = WifiConfigurationTestUtil.TEST_WEP_KEYS; someRandomNetworkWithAllMaskedFields.preSharedKey = WifiConfigurationTestUtil.TEST_PSK; someRandomNetworkWithAllMaskedFields.enterpriseConfig.setPassword( @@ -2457,7 +2458,7 @@ public class WifiConfigManagerTest extends WifiBaseTest { */ @Test public void testGetConfiguredNetworksMasksPasswords() { - WifiConfiguration networkWithPasswords = WifiConfigurationTestUtil.createEapNetwork(); + WifiConfiguration networkWithPasswords = WifiConfigurationTestUtil.createPskNetwork(); networkWithPasswords.wepKeys = WifiConfigurationTestUtil.TEST_WEP_KEYS; networkWithPasswords.preSharedKey = WifiConfigurationTestUtil.TEST_PSK; networkWithPasswords.enterpriseConfig.setPassword( diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigurationUtilTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigurationUtilTest.java index 61b79fc83..8d7f5c6e6 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigurationUtilTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigurationUtilTest.java @@ -473,6 +473,22 @@ public class WifiConfigurationUtilTest extends WifiBaseTest { } /** + * Verify that the validate method fails to validate WifiConfiguration with bad KeyMgmt value. + */ + @Test + public void testValidateNegativeCases_InvalidKeyMgmtWithPreSharedKey() { + WifiConfiguration config = WifiConfigurationTestUtil.createPskNetwork(); + assertTrue(WifiConfigurationUtil.validate(config, WifiConfigurationUtil.VALIDATE_FOR_ADD)); + + config.allowedKeyManagement.clear(); + config.allowedKeyManagement.set(WifiConfiguration.KeyMgmt.OSEN); + assertTrue(WifiConfigurationUtil.validate(config, WifiConfigurationUtil.VALIDATE_FOR_ADD)); + // Verify we reset the KeyMgmt + assertTrue(config.allowedKeyManagement.get(WifiConfiguration.KeyMgmt.WPA_PSK)); + assertFalse(config.allowedKeyManagement.get(WifiConfiguration.KeyMgmt.OSEN)); + } + + /** * Verify that the validate method fails to validate WifiConfiguration with bad Protocol value. */ @Test diff --git a/tests/wifitests/src/com/android/server/wifi/WifiNetworkSelectorTestUtil.java b/tests/wifitests/src/com/android/server/wifi/WifiNetworkSelectorTestUtil.java index 39316a411..6f98f6a7d 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiNetworkSelectorTestUtil.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiNetworkSelectorTestUtil.java @@ -16,6 +16,9 @@ package com.android.server.wifi; +import static com.android.server.wifi.WifiConfigurationTestUtil.SECURITY_PSK; +import static com.android.server.wifi.WifiConfigurationTestUtil.SECURITY_SAE; +import static com.android.server.wifi.WifiConfigurationTestUtil.SECURITY_WAPI_PSK; import static com.android.server.wifi.WifiConfigurationTestUtil.generateWifiConfig; import static org.junit.Assert.*; @@ -277,7 +280,10 @@ public class WifiNetworkSelectorTestUtil { configs[index] = generateWifiConfig(id.intValue(), 0, ssids[index], false, true, null, null, securities[index]); - configs[index].preSharedKey = "\"PA55W0RD\""; // needed to validate with PSK + if (securities[index] == SECURITY_PSK || securities[index] == SECURITY_SAE + || securities[index] == SECURITY_WAPI_PSK) { + configs[index].preSharedKey = "\"PA55W0RD\""; // needed to validate with PSK + } if (!WifiConfigurationUtil.validate(configs[index], true)) { throw new IllegalArgumentException("Invalid generated config: " + configs[index]); } |