diff options
4 files changed, 54 insertions, 17 deletions
diff --git a/service/java/com/android/server/wifi/MacAddressUtil.java b/service/java/com/android/server/wifi/MacAddressUtil.java index effa931a5..4739b6141 100644 --- a/service/java/com/android/server/wifi/MacAddressUtil.java +++ b/service/java/com/android/server/wifi/MacAddressUtil.java @@ -62,8 +62,14 @@ public class MacAddressUtil { if (config == null || hashFunction == null) { return null; } - byte[] hashedBytes = hashFunction.doFinal( - config.getSsidAndSecurityTypeString().getBytes(StandardCharsets.UTF_8)); + byte[] hashedBytes; + try { + hashedBytes = hashFunction.doFinal(config.getSsidAndSecurityTypeString() + .getBytes(StandardCharsets.UTF_8)); + } catch (ProviderException | IllegalStateException e) { + Log.e(TAG, "Failure in calculatePersistentMac", e); + return null; + } ByteBuffer bf = ByteBuffer.wrap(hashedBytes); long longFromSsid = bf.getLong(); /** diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java index 80b583feb..c617b9e1f 100644 --- a/service/java/com/android/server/wifi/WifiConfigManager.java +++ b/service/java/com/android/server/wifi/WifiConfigManager.java @@ -76,8 +76,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import javax.crypto.Mac; - /** * This class provides the APIs to manage configured Wi-Fi networks. * It deals with the following: @@ -278,7 +276,6 @@ public class WifiConfigManager { private final WifiInjector mWifiInjector; private final MacAddressUtil mMacAddressUtil; private boolean mConnectedMacRandomzationSupported; - private Mac mMac; /** * Local log used for debugging any WifiConfigManager issues. @@ -516,7 +513,18 @@ public class WifiConfigManager { mRandomizedMacAddressMapping.remove(config.getSsidAndSecurityTypeString()); } } - return mMacAddressUtil.calculatePersistentMacForConfiguration(config, mMac); + MacAddress result = mMacAddressUtil.calculatePersistentMacForConfiguration( + config, mMacAddressUtil.obtainMacRandHashFunction(Process.WIFI_UID)); + if (result == null) { + result = mMacAddressUtil.calculatePersistentMacForConfiguration( + config, mMacAddressUtil.obtainMacRandHashFunction(Process.WIFI_UID)); + } + if (result == null) { + Log.wtf(TAG, "Failed to generate MAC address from KeyStore even after retrying. " + + "Using locally generated MAC address instead."); + result = MacAddress.createRandomUnicastAddress(); + } + return result; } /** @@ -3146,12 +3154,6 @@ public class WifiConfigManager { * @return true on success or not needed (fresh install), false otherwise. */ public boolean loadFromStore() { - // Get the hashfunction that is used to generate randomized MACs from the KeyStore - mMac = mMacAddressUtil.obtainMacRandHashFunction(Process.WIFI_UID); - if (mMac == null) { - Log.wtf(TAG, "Failed to obtain secret for MAC randomization." - + " All randomized MAC addresses are lost!"); - } // If the user unlock comes in before we load from store, which means the user store have // not been setup yet for the current user. Setup the user store before the read so that // configurations for the current user will also being loaded. diff --git a/tests/wifitests/src/com/android/server/wifi/MacAddressUtilTest.java b/tests/wifitests/src/com/android/server/wifi/MacAddressUtilTest.java index 253310840..7e598db31 100644 --- a/tests/wifitests/src/com/android/server/wifi/MacAddressUtilTest.java +++ b/tests/wifitests/src/com/android/server/wifi/MacAddressUtilTest.java @@ -29,6 +29,7 @@ import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.security.ProviderException; import java.util.Random; import javax.crypto.Mac; @@ -69,4 +70,18 @@ public class MacAddressUtilTest { assertTrue(WifiConfiguration.isValidMacAddressForRandomization(macAddress)); } } + + /** + * Verify the java.security.ProviderException is caught. + */ + @Test + public void testCalculatePersistentMacCatchesException() { + when(mMac.doFinal(any())).thenThrow(new ProviderException("error occurred")); + try { + WifiConfiguration config = WifiConfigurationTestUtil.createOpenNetwork(); + assertNull(mMacAddressUtil.calculatePersistentMacForConfiguration(config, mMac)); + } catch (Exception e) { + fail("Exception not caught."); + } + } } diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java index 23eea328f..6fa1868cb 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java @@ -303,13 +303,27 @@ public class WifiConfigManagerTest { } /** - * Verifies that the Mac randomization secret hashfunction is obtained after |loadFromStore|. + * Verify that a randomized MAC address is generated even if the KeyStore operation fails. */ @Test - public void testMacHashIsObtainedAfterLoadFromStore() { - verify(mMacAddressUtil, never()).obtainMacRandHashFunction(anyInt()); - assertTrue(mWifiConfigManager.loadFromStore()); - verify(mMacAddressUtil).obtainMacRandHashFunction(anyInt()); + public void testRandomizedMacIsGeneratedEvenIfKeyStoreFails() { + when(mMacAddressUtil.calculatePersistentMacForConfiguration(any(), any())).thenReturn(null); + + // Try adding a network. + WifiConfiguration openNetwork = WifiConfigurationTestUtil.createOpenNetwork(); + List<WifiConfiguration> networks = new ArrayList<>(); + networks.add(openNetwork); + verifyAddNetworkToWifiConfigManager(openNetwork); + List<WifiConfiguration> retrievedNetworks = + mWifiConfigManager.getConfiguredNetworksWithPasswords(); + + // Verify that we have attempted to generate the MAC address twice (1 retry) + verify(mMacAddressUtil, times(2)).calculatePersistentMacForConfiguration(any(), any()); + assertEquals(1, retrievedNetworks.size()); + + // Verify that despite KeyStore returning null, we are still getting a valid MAC address. + assertNotEquals(WifiInfo.DEFAULT_MAC_ADDRESS, + retrievedNetworks.get(0).getRandomizedMacAddress().toString()); } /** |