WifiServiceImpl: ap state change response
When responding to getWifiApState API calls, do not send to
WifiStateMachine. Instead, use the locally stored state updated via
AP_STATE_CHANGE broadcasts. Also add test to verify SecurityException
when the API is called and the permission check fails.
Bug: 31830541
Test: frameworks/opt/net/wifi/tests/wifitests/runtests.sh
Test: manual verification from Settings and QuickSettings
Test: wifi integration tests
Test: wifi CTS tests
Change-Id: Id7e2ca73894c3acecfe4ce1ae0e2efa69dc5b07c
diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java
index 0fb4bfb..2724cb5 100644
--- a/service/java/com/android/server/wifi/WifiServiceImpl.java
+++ b/service/java/com/android/server/wifi/WifiServiceImpl.java
@@ -97,6 +97,7 @@
import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.Log;
+import android.util.MutableInt;
import android.util.Slog;
import com.android.internal.annotations.GuardedBy;
@@ -199,6 +200,18 @@
private final ConcurrentHashMap<String, Integer> mIfaceIpModes;
/**
+ * One of: {@link WifiManager#WIFI_AP_STATE_DISABLED},
+ * {@link WifiManager#WIFI_AP_STATE_DISABLING},
+ * {@link WifiManager#WIFI_AP_STATE_ENABLED},
+ * {@link WifiManager#WIFI_AP_STATE_ENABLING},
+ * {@link WifiManager#WIFI_AP_STATE_FAILED}
+ *
+ * Access/maintenance MUST be done on the wifi service thread
+ */
+ private int mWifiApState = WifiManager.WIFI_AP_STATE_DISABLED;
+
+
+ /**
* Callback for use with LocalOnlyHotspot to unregister requesting applications upon death.
*
* @hide
@@ -787,8 +800,7 @@
}
// If SoftAp is enabled, only Settings is allowed to toggle wifi
- boolean apEnabled =
- mWifiStateMachine.syncGetWifiApState() != WifiManager.WIFI_AP_STATE_DISABLED;
+ boolean apEnabled = mWifiApState != WifiManager.WIFI_AP_STATE_DISABLED;
if (apEnabled && !isFromSettings) {
mLog.info("setWifiEnabled SoftAp not disabled: only Settings can enable wifi").flush();
@@ -860,7 +872,13 @@
public int getWifiApEnabledState() {
enforceAccessPermission();
mLog.info("getWifiApEnabledState uid=%").c(Binder.getCallingUid()).flush();
- return mWifiStateMachine.syncGetWifiApState();
+
+ // hand off work to our handler thread
+ MutableInt apState = new MutableInt(WifiManager.WIFI_AP_STATE_DISABLED);
+ mClientHandler.runWithScissors(() -> {
+ apState.value = mWifiApState;
+ }, 0);
+ return apState.value;
}
/**
@@ -1021,6 +1039,8 @@
/**
* Private method to handle SoftAp state changes
+ *
+ * <p> MUST be called from the WifiStateMachine thread.
*/
private void handleWifiApStateChange(
int currentState, int previousState, int errorCode, String ifaceName, int mode) {
@@ -1029,6 +1049,9 @@
+ " previousState=" + previousState + " errorCode= " + errorCode
+ " ifaceName=" + ifaceName + " mode=" + mode);
+ // update the tracking ap state variable
+ mWifiApState = currentState;
+
// check if we have a failure - since it is possible (worst case scenario where
// WifiController and WifiStateMachine are out of sync wrt modes) to get two FAILED
// notifications in a row, we need to handle this first.
diff --git a/service/java/com/android/server/wifi/WifiStateMachine.java b/service/java/com/android/server/wifi/WifiStateMachine.java
index 1521bb9..e5014dc 100644
--- a/service/java/com/android/server/wifi/WifiStateMachine.java
+++ b/service/java/com/android/server/wifi/WifiStateMachine.java
@@ -1676,9 +1676,9 @@
/**
* TODO: doc
*/
- public int syncGetWifiApState() {
- return mWifiApState.get();
- }
+ //public int syncGetWifiApState() {
+ // return mWifiApState.get();
+ //}
/**
* TODO: doc
@@ -2920,6 +2920,7 @@
// Update state
mWifiApState.set(wifiApState);
+ // TODO: when this code is removed, also remove syncGetWifiApStateByName()
if (mVerboseLoggingEnabled) log("setWifiApState: " + syncGetWifiApStateByName());
}
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java
index ea81d53..d0a482e 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java
@@ -31,6 +31,7 @@
import static android.net.wifi.WifiManager.WIFI_AP_STATE_DISABLED;
import static android.net.wifi.WifiManager.WIFI_AP_STATE_DISABLING;
import static android.net.wifi.WifiManager.WIFI_AP_STATE_ENABLED;
+import static android.net.wifi.WifiManager.WIFI_AP_STATE_ENABLING;
import static android.net.wifi.WifiManager.WIFI_AP_STATE_FAILED;
import static android.net.wifi.WifiManager.WIFI_STATE_DISABLED;
import static android.provider.Settings.Secure.LOCATION_MODE_HIGH_ACCURACY;
@@ -45,6 +46,7 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
@@ -370,10 +372,11 @@
/**
* Verify that wifi can be enabled by a caller with WIFI_STATE_CHANGE permission when wifi is
* off (no hotspot, no airplane mode).
+ *
+ * Note: hotspot is disabled by default
*/
@Test
public void testSetWifiEnabledSuccess() throws Exception {
- when(mWifiStateMachine.syncGetWifiApState()).thenReturn(WifiManager.WIFI_AP_STATE_DISABLED);
when(mSettingsStore.handleWifiToggled(eq(true))).thenReturn(true);
when(mSettingsStore.isAirplaneModeOn()).thenReturn(false);
assertTrue(mWifiServiceImpl.setWifiEnabled(TEST_PACKAGE_NAME, true));
@@ -385,7 +388,6 @@
*/
@Test
public void testSetWifiEnabledNoToggle() throws Exception {
- when(mWifiStateMachine.syncGetWifiApState()).thenReturn(WifiManager.WIFI_AP_STATE_DISABLED);
when(mSettingsStore.handleWifiToggled(eq(true))).thenReturn(false);
assertTrue(mWifiServiceImpl.setWifiEnabled(TEST_PACKAGE_NAME, true));
verify(mWifiController, never()).sendMessage(eq(CMD_WIFI_TOGGLED));
@@ -402,7 +404,6 @@
eq("WifiService"));
when(mSettingsStore.isAirplaneModeOn()).thenReturn(false);
mWifiServiceImpl.setWifiEnabled(TEST_PACKAGE_NAME, true);
- verify(mWifiStateMachine, never()).syncGetWifiApState();
}
/**
@@ -441,7 +442,17 @@
*/
@Test
public void testSetWifiEnabledFromNetworkSettingsHolderWhenApEnabled() throws Exception {
- when(mWifiStateMachine.syncGetWifiApState()).thenReturn(WifiManager.WIFI_AP_STATE_ENABLED);
+ when(mFrameworkFacade.inStorageManagerCryptKeeperBounce()).thenReturn(false);
+ when(mSettingsStore.isWifiToggleEnabled()).thenReturn(false);
+ mWifiServiceImpl.checkAndStartWifi();
+
+ verify(mContext).registerReceiver(mBroadcastReceiverCaptor.capture(),
+ (IntentFilter) argThat(new IntentFilterMatcher()));
+
+ TestUtil.sendWifiApStateChanged(mBroadcastReceiverCaptor.getValue(), mContext,
+ WIFI_AP_STATE_ENABLED, WIFI_AP_STATE_ENABLING, SAP_START_FAILURE_GENERAL,
+ WIFI_IFACE_NAME, IFACE_IP_MODE_LOCAL_ONLY);
+
when(mSettingsStore.handleWifiToggled(eq(true))).thenReturn(true);
when(mContext.checkPermission(
eq(android.Manifest.permission.NETWORK_SETTINGS), anyInt(), anyInt()))
@@ -456,7 +467,17 @@
*/
@Test
public void testSetWifiEnabledFromAppFailsWhenApEnabled() throws Exception {
- when(mWifiStateMachine.syncGetWifiApState()).thenReturn(WifiManager.WIFI_AP_STATE_ENABLED);
+ when(mFrameworkFacade.inStorageManagerCryptKeeperBounce()).thenReturn(false);
+ when(mSettingsStore.isWifiToggleEnabled()).thenReturn(false);
+ mWifiServiceImpl.checkAndStartWifi();
+
+ verify(mContext).registerReceiver(mBroadcastReceiverCaptor.capture(),
+ (IntentFilter) argThat(new IntentFilterMatcher()));
+
+ TestUtil.sendWifiApStateChanged(mBroadcastReceiverCaptor.getValue(), mContext,
+ WIFI_AP_STATE_ENABLED, WIFI_AP_STATE_ENABLING, SAP_START_FAILURE_GENERAL,
+ WIFI_IFACE_NAME, IFACE_IP_MODE_LOCAL_ONLY);
+
when(mContext.checkPermission(
eq(android.Manifest.permission.NETWORK_SETTINGS), anyInt(), anyInt()))
.thenReturn(PackageManager.PERMISSION_DENIED);
@@ -472,7 +493,6 @@
*/
@Test
public void testSetWifiDisabledSuccess() throws Exception {
- when(mWifiStateMachine.syncGetWifiApState()).thenReturn(WifiManager.WIFI_AP_STATE_DISABLED);
when(mSettingsStore.handleWifiToggled(eq(false))).thenReturn(true);
assertTrue(mWifiServiceImpl.setWifiEnabled(TEST_PACKAGE_NAME, false));
verify(mWifiController).sendMessage(eq(CMD_WIFI_TOGGLED));
@@ -483,7 +503,6 @@
*/
@Test
public void testSetWifiDisabledNoToggle() throws Exception {
- when(mWifiStateMachine.syncGetWifiApState()).thenReturn(WifiManager.WIFI_AP_STATE_DISABLED);
when(mSettingsStore.handleWifiToggled(eq(false))).thenReturn(false);
assertTrue(mWifiServiceImpl.setWifiEnabled(TEST_PACKAGE_NAME, false));
verify(mWifiController, never()).sendMessage(eq(CMD_WIFI_TOGGLED));
@@ -495,7 +514,6 @@
*/
@Test(expected = SecurityException.class)
public void testSetWifiDisabledWithoutPermission() throws Exception {
- when(mWifiStateMachine.syncGetWifiApState()).thenReturn(WifiManager.WIFI_AP_STATE_DISABLED);
doThrow(new SecurityException()).when(mContext)
.enforceCallingOrSelfPermission(eq(android.Manifest.permission.CHANGE_WIFI_STATE),
eq("WifiService"));
@@ -560,6 +578,58 @@
}
/**
+ * Ensure we return the proper variable for the softap state after getting an AP state change
+ * broadcast.
+ */
+ @Test
+ public void testGetWifiApEnabled() {
+ // set up WifiServiceImpl with a live thread for testing
+ HandlerThread serviceHandlerThread = new HandlerThread("ServiceHandlerThreadForTest");
+ serviceHandlerThread.start();
+ when(mWifiInjector.getWifiServiceHandlerThread()).thenReturn(serviceHandlerThread);
+ mWifiServiceImpl = new WifiServiceImpl(mContext, mWifiInjector, mAsyncChannel);
+ mWifiServiceImpl.setWifiHandlerLogForTest(mLog);
+
+ // ap should be disabled when wifi hasn't been started
+ assertEquals(WifiManager.WIFI_AP_STATE_DISABLED, mWifiServiceImpl.getWifiApEnabledState());
+
+ when(mFrameworkFacade.inStorageManagerCryptKeeperBounce()).thenReturn(false);
+ when(mSettingsStore.isWifiToggleEnabled()).thenReturn(false);
+ mWifiServiceImpl.checkAndStartWifi();
+ mLooper.dispatchAll();
+
+ // ap should be disabled initially
+ assertEquals(WifiManager.WIFI_AP_STATE_DISABLED, mWifiServiceImpl.getWifiApEnabledState());
+
+ // send an ap state change to verify WifiServiceImpl is updated
+ verify(mContext).registerReceiver(mBroadcastReceiverCaptor.capture(),
+ (IntentFilter) argThat(new IntentFilterMatcher()));
+
+ TestUtil.sendWifiApStateChanged(mBroadcastReceiverCaptor.getValue(), mContext,
+ WIFI_AP_STATE_FAILED, WIFI_AP_STATE_DISABLED, SAP_START_FAILURE_GENERAL,
+ WIFI_IFACE_NAME, IFACE_IP_MODE_LOCAL_ONLY);
+ mLooper.dispatchAll();
+
+ assertEquals(WifiManager.WIFI_AP_STATE_FAILED, mWifiServiceImpl.getWifiApEnabledState());
+ }
+
+ /**
+ * Ensure we do not allow unpermitted callers to get the wifi ap state.
+ */
+ @Test
+ public void testGetWifiApEnabledPermissionDenied() {
+ // we should not be able to get the state
+ doThrow(new SecurityException()).when(mContext)
+ .enforceCallingOrSelfPermission(eq(android.Manifest.permission.ACCESS_WIFI_STATE),
+ eq("WifiService"));
+
+ try {
+ mWifiServiceImpl.getWifiApEnabledState();
+ fail("expected SecurityException");
+ } catch (SecurityException expected) { }
+ }
+
+ /**
* Make sure we do not start wifi if System services have to be restarted to decrypt the device.
*/
@Test
@@ -591,7 +661,6 @@
when(mSettingsStore.handleWifiToggled(true)).thenReturn(true);
when(mSettingsStore.isWifiToggleEnabled()).thenReturn(true);
when(mWifiStateMachine.syncGetWifiState()).thenReturn(WIFI_STATE_DISABLED);
- when(mWifiStateMachine.syncGetWifiApState()).thenReturn(WifiManager.WIFI_AP_STATE_DISABLED);
when(mContext.getPackageName()).thenReturn(ANDROID_SYSTEM_PACKAGE);
mWifiServiceImpl.checkAndStartWifi();
verify(mWifiController).start();
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java
index e6972ea..04d5c17 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java
@@ -16,18 +16,6 @@
package com.android.server.wifi;
-import static android.net.wifi.WifiManager.EXTRA_PREVIOUS_WIFI_AP_STATE;
-import static android.net.wifi.WifiManager.EXTRA_WIFI_AP_FAILURE_REASON;
-import static android.net.wifi.WifiManager.EXTRA_WIFI_AP_INTERFACE_NAME;
-import static android.net.wifi.WifiManager.EXTRA_WIFI_AP_MODE;
-import static android.net.wifi.WifiManager.EXTRA_WIFI_AP_STATE;
-import static android.net.wifi.WifiManager.WIFI_AP_STATE_DISABLED;
-import static android.net.wifi.WifiManager.WIFI_AP_STATE_DISABLING;
-import static android.net.wifi.WifiManager.WIFI_AP_STATE_ENABLED;
-import static android.net.wifi.WifiManager.WIFI_AP_STATE_ENABLING;
-
-import static com.android.server.wifi.LocalOnlyHotspotRequestInfo.HOTSPOT_NO_ERROR;
-
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
@@ -578,21 +566,6 @@
assertEquals("DisconnectedState", getCurrentState().getName());
}
- private void checkApStateChangedBroadcast(Intent intent, int expectedCurrentState,
- int expectedPrevState, int expectedErrorCode, String expectedIfaceName,
- int expectedMode) {
- int currentState = intent.getIntExtra(EXTRA_WIFI_AP_STATE, WIFI_AP_STATE_DISABLED);
- int prevState = intent.getIntExtra(EXTRA_PREVIOUS_WIFI_AP_STATE, WIFI_AP_STATE_DISABLED);
- int errorCode = intent.getIntExtra(EXTRA_WIFI_AP_FAILURE_REASON, HOTSPOT_NO_ERROR);
- String ifaceName = intent.getStringExtra(EXTRA_WIFI_AP_INTERFACE_NAME);
- int mode = intent.getIntExtra(EXTRA_WIFI_AP_MODE, WifiManager.IFACE_IP_MODE_UNSPECIFIED);
- assertEquals(expectedCurrentState, currentState);
- assertEquals(expectedPrevState, prevState);
- assertEquals(expectedErrorCode, errorCode);
- assertEquals(expectedIfaceName, ifaceName);
- assertEquals(expectedMode, mode);
- }
-
private void loadComponentsInApMode(int mode) throws Exception {
SoftApModeConfiguration config = new SoftApModeConfiguration(mode, new WifiConfiguration());
mWsm.setHostApRunning(config, true);
@@ -602,18 +575,6 @@
verify(mWifiNative).setupForSoftApMode(WIFI_IFACE_NAME);
verify(mSoftApManager).start();
-
- // get the SoftApManager.Listener and trigger some updates
- SoftApManager.Listener listener = mSoftApManagerListenerCaptor.getValue();
- listener.onStateChanged(WIFI_AP_STATE_ENABLING, 0);
- assertEquals(WIFI_AP_STATE_ENABLING, mWsm.syncGetWifiApState());
- listener.onStateChanged(WIFI_AP_STATE_ENABLED, 0);
- assertEquals(WIFI_AP_STATE_ENABLED, mWsm.syncGetWifiApState());
- listener.onStateChanged(WIFI_AP_STATE_DISABLING, 0);
- assertEquals(WIFI_AP_STATE_DISABLING, mWsm.syncGetWifiApState());
- // note, this will trigger a mode change when TestLooper is dispatched
- listener.onStateChanged(WIFI_AP_STATE_DISABLED, 0);
- assertEquals(WIFI_AP_STATE_DISABLED, mWsm.syncGetWifiApState());
}
private void setupMockWpsPbc() throws Exception {