From fe3912fd156bc82e501dac14db9a07aa18a82acc Mon Sep 17 00:00:00 2001 From: xshu Date: Mon, 4 Nov 2019 15:36:22 -0800 Subject: [MAC rand] Fix unit test slowness Removing usage use spyStatic to speed up unit test. Bug: 143712485 Test: atest WifiConfigManagerTest is back to normal speed now Change-Id: I39bb0916bf3a88f73a1ca869af347e3442f2384b Merged-In: I39bb0916bf3a88f73a1ca869af347e3442f2384b (cherry picked from: 316dd9f75f489c8e4c26efaae56f1cdefe9a56b9) --- .../com/android/server/wifi/MacAddressUtil.java | 133 +++++++++++++++++++++ .../com/android/server/wifi/WifiConfigManager.java | 6 +- .../android/server/wifi/WifiConfigurationUtil.java | 102 ---------------- .../java/com/android/server/wifi/WifiInjector.java | 6 + .../android/server/wifi/MacAddressUtilTest.java | 72 +++++++++++ .../android/server/wifi/WifiConfigManagerTest.java | 8 +- .../server/wifi/WifiConfigurationUtilTest.java | 30 ----- 7 files changed, 220 insertions(+), 137 deletions(-) create mode 100644 service/java/com/android/server/wifi/MacAddressUtil.java create mode 100644 tests/wifitests/src/com/android/server/wifi/MacAddressUtilTest.java diff --git a/service/java/com/android/server/wifi/MacAddressUtil.java b/service/java/com/android/server/wifi/MacAddressUtil.java new file mode 100644 index 000000000..effa931a5 --- /dev/null +++ b/service/java/com/android/server/wifi/MacAddressUtil.java @@ -0,0 +1,133 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.wifi; + +import android.net.MacAddress; +import android.net.wifi.WifiConfiguration; +import android.security.keystore.AndroidKeyStoreProvider; +import android.security.keystore.KeyGenParameterSpec; +import android.security.keystore.KeyProperties; +import android.util.Log; + +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.ProviderException; +import java.security.UnrecoverableKeyException; +import java.util.Arrays; + +import javax.crypto.KeyGenerator; +import javax.crypto.Mac; +import javax.crypto.SecretKey; + +/** + * Contains helper methods to support MAC randomization. + */ +public class MacAddressUtil { + private static final String TAG = "MacAddressUtil"; + private static final String MAC_RANDOMIZATION_ALIAS = "MacRandSecret"; + private static final long MAC_ADDRESS_VALID_LONG_MASK = (1L << 48) - 1; + private static final long MAC_ADDRESS_LOCALLY_ASSIGNED_MASK = 1L << 41; + private static final long MAC_ADDRESS_MULTICAST_MASK = 1L << 40; + + /** + * Computes the persistent randomized MAC of the given configuration using the given + * hash function. + * @param config the WifiConfiguration to compute MAC address for + * @param hashFunction the hash function that will perform the MAC address computation. + * @return The persistent randomized MAC address or null if inputs are invalid. + */ + public MacAddress calculatePersistentMacForConfiguration(WifiConfiguration config, + Mac hashFunction) { + if (config == null || hashFunction == null) { + return null; + } + byte[] hashedBytes = hashFunction.doFinal( + config.getSsidAndSecurityTypeString().getBytes(StandardCharsets.UTF_8)); + ByteBuffer bf = ByteBuffer.wrap(hashedBytes); + long longFromSsid = bf.getLong(); + /** + * Masks the generated long so that it represents a valid randomized MAC address. + * Specifically, this sets the locally assigned bit to 1, multicast bit to 0 + */ + longFromSsid &= MAC_ADDRESS_VALID_LONG_MASK; + longFromSsid |= MAC_ADDRESS_LOCALLY_ASSIGNED_MASK; + longFromSsid &= ~MAC_ADDRESS_MULTICAST_MASK; + bf.clear(); + bf.putLong(0, longFromSsid); + + // MacAddress.fromBytes requires input of length 6, which is obtained from the + // last 6 bytes from the generated long. + MacAddress macAddress = MacAddress.fromBytes(Arrays.copyOfRange(bf.array(), 2, 8)); + return macAddress; + } + + /** + * Retrieves a Hash function that could be used to calculate the persistent randomized MAC + * for a WifiConfiguration. + * @param uid the UID of the KeyStore to get the secret of the hash function from. + */ + public Mac obtainMacRandHashFunction(int uid) { + try { + KeyStore keyStore = AndroidKeyStoreProvider.getKeyStoreForUid(uid); + // tries to retrieve the secret, and generate a new one if it's unavailable. + Key key = keyStore.getKey(MAC_RANDOMIZATION_ALIAS, null); + if (key == null) { + key = generateAndPersistNewMacRandomizationSecret(uid); + } + if (key == null) { + Log.e(TAG, "Failed to generate secret for " + MAC_RANDOMIZATION_ALIAS); + return null; + } + Mac result = Mac.getInstance("HmacSHA256"); + result.init(key); + return result; + } catch (KeyStoreException | NoSuchAlgorithmException | InvalidKeyException + | UnrecoverableKeyException | NoSuchProviderException e) { + Log.e(TAG, "Failure in obtainMacRandHashFunction", e); + return null; + } + } + + /** + * Generates and returns a secret key to use for Mac randomization. + * Will also persist the generated secret inside KeyStore, accessible in the + * future with KeyGenerator#getKey. + */ + private SecretKey generateAndPersistNewMacRandomizationSecret(int uid) { + try { + KeyGenerator keyGenerator = KeyGenerator.getInstance( + KeyProperties.KEY_ALGORITHM_HMAC_SHA256, "AndroidKeyStore"); + keyGenerator.init( + new KeyGenParameterSpec.Builder(MAC_RANDOMIZATION_ALIAS, + KeyProperties.PURPOSE_SIGN) + .setUid(uid) + .build()); + return keyGenerator.generateKey(); + } catch (NoSuchAlgorithmException | InvalidAlgorithmParameterException + | NoSuchProviderException | ProviderException e) { + Log.e(TAG, "Failure in generateMacRandomizationSecret", e); + return null; + } + } +} diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java index c027675ce..b5d06ff05 100644 --- a/service/java/com/android/server/wifi/WifiConfigManager.java +++ b/service/java/com/android/server/wifi/WifiConfigManager.java @@ -276,6 +276,7 @@ public class WifiConfigManager { private final WifiPermissionsUtil mWifiPermissionsUtil; private final WifiPermissionsWrapper mWifiPermissionsWrapper; private final WifiInjector mWifiInjector; + private final MacAddressUtil mMacAddressUtil; private boolean mConnectedMacRandomzationSupported; private final Mac mMac; @@ -452,7 +453,8 @@ public class WifiConfigManager { } catch (PackageManager.NameNotFoundException e) { Log.e(TAG, "Unable to resolve SystemUI's UID."); } - mMac = WifiConfigurationUtil.obtainMacRandHashFunction(Process.WIFI_UID); + mMacAddressUtil = mWifiInjector.getMacAddressUtil(); + 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!"); @@ -508,7 +510,7 @@ public class WifiConfigManager { mRandomizedMacAddressMapping.remove(config.getSsidAndSecurityTypeString()); } } - return WifiConfigurationUtil.calculatePersistentMacForConfiguration(config, mMac); + return mMacAddressUtil.calculatePersistentMacForConfiguration(config, mMac); } /** diff --git a/service/java/com/android/server/wifi/WifiConfigurationUtil.java b/service/java/com/android/server/wifi/WifiConfigurationUtil.java index b8992a011..59d3eb3f4 100644 --- a/service/java/com/android/server/wifi/WifiConfigurationUtil.java +++ b/service/java/com/android/server/wifi/WifiConfigurationUtil.java @@ -28,9 +28,6 @@ import android.net.wifi.WifiNetworkSpecifier; import android.net.wifi.WifiScanner; import android.os.PatternMatcher; import android.os.UserHandle; -import android.security.keystore.AndroidKeyStoreProvider; -import android.security.keystore.KeyGenParameterSpec; -import android.security.keystore.KeyProperties; import android.text.TextUtils; import android.util.Log; import android.util.Pair; @@ -39,17 +36,7 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.server.wifi.util.NativeUtil; import com.android.server.wifi.util.TelephonyUtil; -import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; -import java.security.InvalidAlgorithmParameterException; -import java.security.InvalidKeyException; -import java.security.Key; -import java.security.KeyStore; -import java.security.KeyStoreException; -import java.security.NoSuchAlgorithmException; -import java.security.NoSuchProviderException; -import java.security.ProviderException; -import java.security.UnrecoverableKeyException; import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.BitSet; @@ -57,10 +44,6 @@ import java.util.Comparator; import java.util.List; import java.util.Objects; -import javax.crypto.KeyGenerator; -import javax.crypto.Mac; -import javax.crypto.SecretKey; - /** * WifiConfiguration utility for any {@link android.net.wifi.WifiConfiguration} related operations. * Currently contains: @@ -89,10 +72,6 @@ public class WifiConfigurationUtil { new Pair(MacAddress.BROADCAST_ADDRESS, MacAddress.BROADCAST_ADDRESS); private static final Pair MATCH_ALL_BSSID_PATTERN = new Pair(MacAddress.ALL_ZEROS_ADDRESS, MacAddress.ALL_ZEROS_ADDRESS); - private static final String MAC_RANDOMIZATION_ALIAS = "MacRandSecret"; - private static final long MAC_ADDRESS_VALID_LONG_MASK = (1L << 48) - 1; - private static final long MAC_ADDRESS_LOCALLY_ASSIGNED_MASK = 1L << 41; - private static final long MAC_ADDRESS_MULTICAST_MASK = 1L << 40; /** * Check whether a network configuration is visible to a user or any of its managed profiles. @@ -247,87 +226,6 @@ public class WifiConfigurationUtil { return newConfig.macRandomizationSetting != existingConfig.macRandomizationSetting; } - /** - * Computes the persistent randomized MAC of the given configuration using the given - * hash function. - * @param config the WifiConfiguration to compute MAC address for - * @param hashFunction the hash function that will perform the MAC address computation. - * @return The persistent randomized MAC address or null if inputs are invalid. - */ - public static MacAddress calculatePersistentMacForConfiguration(WifiConfiguration config, - Mac hashFunction) { - if (config == null || hashFunction == null) { - return null; - } - byte[] hashedBytes = hashFunction.doFinal( - config.getSsidAndSecurityTypeString().getBytes(StandardCharsets.UTF_8)); - ByteBuffer bf = ByteBuffer.wrap(hashedBytes); - long longFromSsid = bf.getLong(); - /** - * Masks the generated long so that it represents a valid randomized MAC address. - * Specifically, this sets the locally assigned bit to 1, multicast bit to 0 - */ - longFromSsid &= MAC_ADDRESS_VALID_LONG_MASK; - longFromSsid |= MAC_ADDRESS_LOCALLY_ASSIGNED_MASK; - longFromSsid &= ~MAC_ADDRESS_MULTICAST_MASK; - bf.clear(); - bf.putLong(0, longFromSsid); - - // MacAddress.fromBytes requires input of length 6, which is obtained from the - // last 6 bytes from the generated long. - MacAddress macAddress = MacAddress.fromBytes(Arrays.copyOfRange(bf.array(), 2, 8)); - return macAddress; - } - - /** - * Retrieves a Hash function that could be used to calculate the persistent randomized MAC - * for a WifiConfiguration. - * @param uid the UID of the KeyStore to get the secret of the hash function from. - */ - public static Mac obtainMacRandHashFunction(int uid) { - try { - KeyStore keyStore = AndroidKeyStoreProvider.getKeyStoreForUid(uid); - // tries to retrieve the secret, and generate a new one if it's unavailable. - Key key = keyStore.getKey(MAC_RANDOMIZATION_ALIAS, null); - if (key == null) { - key = generateAndPersistNewMacRandomizationSecret(uid); - } - if (key == null) { - Log.e(TAG, "Failed to generate secret for " + MAC_RANDOMIZATION_ALIAS); - return null; - } - Mac result = Mac.getInstance("HmacSHA256"); - result.init(key); - return result; - } catch (KeyStoreException | NoSuchAlgorithmException | InvalidKeyException - | UnrecoverableKeyException | NoSuchProviderException e) { - Log.e(TAG, "Failure in obtainMacRandHashFunction", e); - return null; - } - } - - /** - * Generates and returns a secret key to use for Mac randomization. - * Will also persist the generated secret inside KeyStore, accessible in the - * future with KeyGenerator#getKey. - */ - private static SecretKey generateAndPersistNewMacRandomizationSecret(int uid) { - try { - KeyGenerator keyGenerator = KeyGenerator.getInstance( - KeyProperties.KEY_ALGORITHM_HMAC_SHA256, "AndroidKeyStore"); - keyGenerator.init( - new KeyGenParameterSpec.Builder(MAC_RANDOMIZATION_ALIAS, - KeyProperties.PURPOSE_SIGN) - .setUid(uid) - .build()); - return keyGenerator.generateKey(); - } catch (NoSuchAlgorithmException | InvalidAlgorithmParameterException - | NoSuchProviderException | ProviderException e) { - Log.e(TAG, "Failure in generateMacRandomizationSecret", e); - return null; - } - } - /** * Compare existing and new WifiEnterpriseConfig objects after a network update and return if * credential parameters have changed or not. diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java index f7212ddfc..83ae89cdc 100644 --- a/service/java/com/android/server/wifi/WifiInjector.java +++ b/service/java/com/android/server/wifi/WifiInjector.java @@ -154,6 +154,7 @@ public class WifiInjector { private final LinkProbeManager mLinkProbeManager; private final IpMemoryStore mIpMemoryStore; private final CellularLinkLayerStatsCollector mCellularLinkLayerStatsCollector; + private final MacAddressUtil mMacAddressUtil; public WifiInjector(Context context) { if (context == null) { @@ -168,6 +169,7 @@ public class WifiInjector { sWifiInjector = this; + mMacAddressUtil = new MacAddressUtil(); mContext = context; mDeviceConfigFacade = new DeviceConfigFacade(); mWifiScoreCard = new WifiScoreCard(mClock, @@ -695,6 +697,10 @@ public class WifiInjector { return mRttHandlerThread; } + public MacAddressUtil getMacAddressUtil() { + return mMacAddressUtil; + } + /** * Returns a single instance of HalDeviceManager for injection. */ diff --git a/tests/wifitests/src/com/android/server/wifi/MacAddressUtilTest.java b/tests/wifitests/src/com/android/server/wifi/MacAddressUtilTest.java new file mode 100644 index 000000000..253310840 --- /dev/null +++ b/tests/wifitests/src/com/android/server/wifi/MacAddressUtilTest.java @@ -0,0 +1,72 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.wifi; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +import android.net.MacAddress; +import android.net.wifi.WifiConfiguration; + +import androidx.test.filters.SmallTest; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.util.Random; + +import javax.crypto.Mac; + +/** + * Unit tests for {@link com.android.server.wifi.MacAddressUtil}. + */ +@SmallTest +public class MacAddressUtilTest { + private MacAddressUtil mMacAddressUtil; + + @Mock private Mac mMac; + + @Before + public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); + mMacAddressUtil = new MacAddressUtil(); + } + + /** + * Verifies that calculatePersistentMacForConfiguration valid randomized MACs. + */ + @Test + public void testCalculatePersistentMacForConfiguration() { + // verify null inputs + assertNull(mMacAddressUtil.calculatePersistentMacForConfiguration(null, null)); + + Random rand = new Random(); + // Verify that a the MAC address calculated is valid + for (int i = 0; i < 10; i++) { + WifiConfiguration config = WifiConfigurationTestUtil.createOpenNetwork(); + + byte[] bytes = new byte[32]; + rand.nextBytes(bytes); + when(mMac.doFinal(any())).thenReturn(bytes); + MacAddress macAddress = mMacAddressUtil.calculatePersistentMacForConfiguration( + config, mMac); + assertTrue(WifiConfiguration.isValidMacAddressForRandomization(macAddress)); + } + } +} diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java index c3b90d36c..95a1e84e0 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java @@ -135,6 +135,7 @@ public class WifiConfigManagerTest { @Mock private WifiConfigManager.OnSavedNetworkUpdateListener mWcmListener; @Mock private FrameworkFacade mFrameworkFacade; @Mock private CarrierNetworkConfig mCarrierNetworkConfig; + @Mock private MacAddressUtil mMacAddressUtil; private MockResources mResources; private InOrder mContextConfigStoreMockOrder; @@ -218,6 +219,10 @@ public class WifiConfigManagerTest { when(mWifiInjector.getWifiLastResortWatchdog().shouldIgnoreSsidUpdate()) .thenReturn(false); when(mWifiInjector.getCarrierNetworkConfig()).thenReturn(mCarrierNetworkConfig); + when(mWifiInjector.getMacAddressUtil()).thenReturn(mMacAddressUtil); + when(mMacAddressUtil.calculatePersistentMacForConfiguration(any(), any())) + .thenReturn(TEST_RANDOMIZED_MAC); + createWifiConfigManager(); mWifiConfigManager.setOnSavedNetworkUpdateListener(mWcmListener); ArgumentCaptor observerCaptor = @@ -233,13 +238,10 @@ public class WifiConfigManagerTest { // static mocking mSession = ExtendedMockito.mockitoSession() .mockStatic(WifiConfigStore.class, withSettings().lenient()) - .spyStatic(WifiConfigurationUtil.class) .strictness(Strictness.LENIENT) .startMocking(); when(WifiConfigStore.createUserFiles(anyInt(), anyBoolean())).thenReturn(mock(List.class)); when(mTelephonyManager.createForSubscriptionId(anyInt())).thenReturn(mDataTelephonyManager); - when(WifiConfigurationUtil.calculatePersistentMacForConfiguration(any(), any())) - .thenReturn(TEST_RANDOMIZED_MAC); } /** diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigurationUtilTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigurationUtilTest.java index 7173dae5b..c1640ce91 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigurationUtilTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigurationUtilTest.java @@ -25,7 +25,6 @@ import android.net.wifi.WifiConfiguration; import android.net.wifi.WifiEnterpriseConfig; import android.net.wifi.WifiNetworkSpecifier; import android.net.wifi.WifiScanner; -import android.os.Binder; import android.os.PatternMatcher; import android.os.UserHandle; import android.util.Pair; @@ -40,8 +39,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; -import javax.crypto.Mac; - /** * Unit tests for {@link com.android.server.wifi.WifiConfigurationUtil}. */ @@ -964,33 +961,6 @@ public class WifiConfigurationUtilTest { existingConfig, newConfig)); } - /** - * Verifies that calculatePersistentMacForConfiguration produces persistent, locally generated - * MAC addresses that are valid for MAC randomization. - */ - @Test - public void testCalculatePersistentMacForConfiguration() { - // verify null inputs - assertNull(WifiConfigurationUtil.calculatePersistentMacForConfiguration(null, null)); - - // test multiple times since there is some randomness involved with hashing - int uid = Binder.getCallingUid(); - for (int i = 0; i < 10; i++) { - // Verify that a the MAC address calculated is valid - WifiConfiguration config = WifiConfigurationTestUtil.createOpenNetwork(); - Mac hashFunction = WifiConfigurationUtil.obtainMacRandHashFunction(uid); - MacAddress macAddress = WifiConfigurationUtil.calculatePersistentMacForConfiguration( - config, hashFunction); - assertTrue(WifiConfiguration.isValidMacAddressForRandomization(macAddress)); - - // Verify that the secret used to generate MAC address is persistent - Mac hashFunction2 = WifiConfigurationUtil.obtainMacRandHashFunction(uid); - MacAddress macAddress2 = WifiConfigurationUtil.calculatePersistentMacForConfiguration( - config, hashFunction2); - assertEquals(macAddress, macAddress2); - } - } - private static class EnterpriseConfig { public String eap; public String phase2; -- cgit v1.2.3