[Wifi] Clear calling identity before permissions check
WifiManager API getConnectionInfo() must clear calling identity before
calling internal APIs that check permissions. This CL saves the calling
identity before clearing it and passed it on to the permission checks
method and restores it after.
Bug: 69725889
Test: On device tests, to repro the issue, Unit tests, QR Provisioning
tests
Change-Id: I7c182e934bf1e8a6ca41fd11dd67add36aef7992
diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java
index 8db180f..48e065d 100644
--- a/service/java/com/android/server/wifi/WifiServiceImpl.java
+++ b/service/java/com/android/server/wifi/WifiServiceImpl.java
@@ -1734,12 +1734,19 @@
@Override
public WifiInfo getConnectionInfo(String callingPackage) {
enforceAccessPermission();
- mLog.info("getConnectionInfo uid=%").c(Binder.getCallingUid()).flush();
+ int uid = Binder.getCallingUid();
+ mLog.info("getConnectionInfo uid=%").c(uid).flush();
/*
* Make sure we have the latest information, by sending
* a status request to the supplicant.
*/
- return mWifiStateMachine.syncRequestConnectionInfo(callingPackage);
+ long ident = Binder.clearCallingIdentity();
+ try {
+ WifiInfo result = mWifiStateMachine.syncRequestConnectionInfo(callingPackage, uid);
+ return result;
+ } finally {
+ Binder.restoreCallingIdentity(ident);
+ }
}
/**
diff --git a/service/java/com/android/server/wifi/WifiStateMachine.java b/service/java/com/android/server/wifi/WifiStateMachine.java
index b005923..75c4844 100644
--- a/service/java/com/android/server/wifi/WifiStateMachine.java
+++ b/service/java/com/android/server/wifi/WifiStateMachine.java
@@ -29,9 +29,7 @@
import static android.telephony.TelephonyManager.CALL_STATE_IDLE;
import static android.telephony.TelephonyManager.CALL_STATE_OFFHOOK;
-import android.Manifest;
import android.app.ActivityManager;
-import android.app.AppGlobals;
import android.app.PendingIntent;
import android.bluetooth.BluetoothAdapter;
import android.content.BroadcastReceiver;
@@ -39,7 +37,6 @@
import android.content.Intent;
import android.content.IntentFilter;
import android.content.pm.ApplicationInfo;
-import android.content.pm.IPackageManager;
import android.content.pm.PackageManager;
import android.database.ContentObserver;
import android.net.ConnectivityManager;
@@ -83,7 +80,6 @@
import android.net.wifi.hotspot2.PasspointConfiguration;
import android.net.wifi.p2p.IWifiP2pManager;
import android.os.BatteryStats;
-import android.os.Binder;
import android.os.Build;
import android.os.Bundle;
import android.os.IBinder;
@@ -129,6 +125,7 @@
import com.android.server.wifi.util.TelephonyUtil.SimAuthRequestData;
import com.android.server.wifi.util.TelephonyUtil.SimAuthResponseData;
import com.android.server.wifi.util.WifiPermissionsUtil;
+import com.android.server.wifi.util.WifiPermissionsWrapper;
import java.io.BufferedReader;
import java.io.FileDescriptor;
@@ -185,7 +182,7 @@
private static final String EXTRA_OSU_PROVIDER = "OsuProvider";
private boolean mVerboseLoggingEnabled = false;
-
+ private final WifiPermissionsWrapper mWifiPermissionsWrapper;
/* debug flag, indicating if handling of ASSOCIATION_REJECT ended up blacklisting
* the corresponding BSSID.
*/
@@ -951,6 +948,7 @@
mWifiMonitor = mWifiInjector.getWifiMonitor();
mWifiDiagnostics = mWifiInjector.makeWifiDiagnostics(mWifiNative);
+ mWifiPermissionsWrapper = mWifiInjector.getWifiPermissionsWrapper();
mWifiInfo = new WifiInfo();
mSupplicantStateTracker =
@@ -1764,20 +1762,18 @@
/**
* Get status information for the current connection, if any.
*
+ * @param callingPackage string indicating the calling package of the caller
+ * @param uid the calling uid
* @return a {@link WifiInfo} object containing information about the current connection
*/
- public WifiInfo syncRequestConnectionInfo(String callingPackage) {
- int uid = Binder.getCallingUid();
+ public WifiInfo syncRequestConnectionInfo(String callingPackage, int uid) {
WifiInfo result = new WifiInfo(mWifiInfo);
- if (uid == Process.myUid()) return result;
boolean hideBssidAndSsid = true;
result.setMacAddress(WifiInfo.DEFAULT_MAC_ADDRESS);
- IPackageManager packageManager = AppGlobals.getPackageManager();
-
try {
- if (packageManager.checkUidPermission(Manifest.permission.LOCAL_MAC_ADDRESS,
- uid) == PackageManager.PERMISSION_GRANTED) {
+ if (mWifiPermissionsWrapper.getLocalMacAddressPermission(uid)
+ == PackageManager.PERMISSION_GRANTED) {
result.setMacAddress(mWifiInfo.getMacAddress());
}
if (mWifiPermissionsUtil.canAccessScanResults(
diff --git a/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java b/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
index d3c072f..4ae7d13 100644
--- a/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
+++ b/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
@@ -22,7 +22,6 @@
import android.content.pm.PackageManager;
import android.content.pm.UserInfo;
import android.net.NetworkScoreManager;
-import android.os.Binder;
import android.os.RemoteException;
import android.os.UserManager;
import android.provider.Settings;
@@ -202,24 +201,19 @@
* current user.
*/
private boolean isCurrentProfile(int uid) {
- final long token = Binder.clearCallingIdentity();
- try {
- int currentUser = mWifiPermissionsWrapper.getCurrentUser();
- int callingUserId = mWifiPermissionsWrapper.getCallingUserId(uid);
- if (callingUserId == currentUser) {
- return true;
- } else {
- List<UserInfo> userProfiles = mUserManager.getProfiles(currentUser);
- for (UserInfo user : userProfiles) {
- if (user.id == callingUserId) {
- return true;
- }
+ int currentUser = mWifiPermissionsWrapper.getCurrentUser();
+ int callingUserId = mWifiPermissionsWrapper.getCallingUserId(uid);
+ if (callingUserId == currentUser) {
+ return true;
+ } else {
+ List<UserInfo> userProfiles = mUserManager.getProfiles(currentUser);
+ for (UserInfo user : userProfiles) {
+ if (user.id == callingUserId) {
+ return true;
}
}
- return false;
- } finally {
- Binder.restoreCallingIdentity(token);
}
+ return false;
}
/**
diff --git a/service/java/com/android/server/wifi/util/WifiPermissionsWrapper.java b/service/java/com/android/server/wifi/util/WifiPermissionsWrapper.java
index 6fde01e..84aacdf 100644
--- a/service/java/com/android/server/wifi/util/WifiPermissionsWrapper.java
+++ b/service/java/com/android/server/wifi/util/WifiPermissionsWrapper.java
@@ -108,4 +108,16 @@
return AppGlobals.getPackageManager().checkUidPermission(
Manifest.permission.CHANGE_WIFI_STATE, uid);
}
+
+ /**
+ * Determines if the caller has local mac address permission.
+ *
+ * @param uid to check the permission for
+ * @return int representation of success or denied
+ * @throws RemoteException
+ */
+ public int getLocalMacAddressPermission(int uid) throws RemoteException {
+ return AppGlobals.getPackageManager().checkUidPermission(
+ Manifest.permission.LOCAL_MAC_ADDRESS, uid);
+ }
}
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java
index 7fa89f2..23e4025 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java
@@ -107,6 +107,7 @@
import com.android.server.wifi.hotspot2.PasspointProvisioningTestUtil;
import com.android.server.wifi.p2p.WifiP2pServiceImpl;
import com.android.server.wifi.util.WifiPermissionsUtil;
+import com.android.server.wifi.util.WifiPermissionsWrapper;
import org.junit.After;
import org.junit.Before;
@@ -153,6 +154,7 @@
private static final int WPS_FRAMEWORK_NETWORK_ID = 10;
private static final String DEFAULT_TEST_SSID = "\"GoogleGuest\"";
private static final String OP_PACKAGE_NAME = "com.xxx";
+ private static final int TEST_UID = Process.SYSTEM_UID + 1000;
private long mBinderToken;
@@ -376,6 +378,7 @@
@Mock ConnectivityManager mConnectivityManager;
@Mock IProvisioningCallback mProvisioningCallback;
@Mock HandlerThread mWifiServiceHandlerThread;
+ @Mock WifiPermissionsWrapper mWifiPermissionsWrapper;
public WifiStateMachineTest() throws Exception {
}
@@ -421,6 +424,7 @@
when(mWifiInjector.getClock()).thenReturn(mClock);
when(mWifiServiceHandlerThread.getLooper()).thenReturn(mLooper.getLooper());
when(mWifiInjector.getWifiServiceHandlerThread()).thenReturn(mWifiServiceHandlerThread);
+ when(mWifiInjector.getWifiPermissionsWrapper()).thenReturn(mWifiPermissionsWrapper);
when(mWifiNative.setupForClientMode(WIFI_IFACE_NAME))
.thenReturn(Pair.create(WifiNative.SETUP_SUCCESS, mClientInterface));
@@ -469,6 +473,8 @@
}).when(mTelephonyManager).listen(any(PhoneStateListener.class), anyInt());
when(mWifiPermissionsUtil.checkNetworkSettingsPermission(anyInt())).thenReturn(true);
+ when(mWifiPermissionsWrapper.getLocalMacAddressPermission(anyInt()))
+ .thenReturn(PackageManager.PERMISSION_DENIED);
initializeWsm();
mOsuProvider = PasspointProvisioningTestUtil.generateOsuProvider(true);
@@ -2294,56 +2300,29 @@
}
/**
- * Test that the process uid has full wifiInfo access.
- * Also tests that {@link WifiStateMachine#syncRequestConnectionInfo(String)} always
- * returns a copy of WifiInfo.
- */
- @Test
- public void testConnectedIdsAreVisibleFromOwnUid() throws Exception {
- assertEquals(Process.myUid(), Binder.getCallingUid());
- WifiInfo wifiInfo = mWsm.getWifiInfo();
- wifiInfo.setBSSID(sBSSID);
- wifiInfo.setSSID(WifiSsid.createFromAsciiEncoded(sSSID));
-
- connect();
- WifiInfo connectionInfo = mWsm.syncRequestConnectionInfo(mContext.getOpPackageName());
-
- assertNotEquals(wifiInfo, connectionInfo);
- assertEquals(wifiInfo.getSSID(), connectionInfo.getSSID());
- assertEquals(wifiInfo.getBSSID(), connectionInfo.getBSSID());
- }
-
- /**
* Test that connected SSID and BSSID are not exposed to an app that does not have the
* appropriate permissions.
* Also tests that {@link WifiStateMachine#syncRequestConnectionInfo(String)} always
* returns a copy of WifiInfo.
*/
@Test
- public void testConnectedIdsAreHiddenFromRandomApp() throws Exception {
- int actualUid = Binder.getCallingUid();
- int fakeUid = Process.myUid() + 100000;
- assertNotEquals(actualUid, fakeUid);
- BinderUtil.setUid(fakeUid);
- try {
- WifiInfo wifiInfo = mWsm.getWifiInfo();
+ public void testConnectedIdsAreHiddenFromAppWithoutPermission() throws Exception {
+ WifiInfo wifiInfo = mWsm.getWifiInfo();
- // Get into a connected state, with known BSSID and SSID
- connect();
- assertEquals(sBSSID, wifiInfo.getBSSID());
- assertEquals(sWifiSsid, wifiInfo.getWifiSsid());
+ // Get into a connected state, with known BSSID and SSID
+ connect();
+ assertEquals(sBSSID, wifiInfo.getBSSID());
+ assertEquals(sWifiSsid, wifiInfo.getWifiSsid());
- when(mWifiPermissionsUtil.canAccessScanResults(anyString(), eq(fakeUid), anyInt()))
- .thenReturn(false);
+ when(mWifiPermissionsUtil.canAccessScanResults(anyString(), eq(TEST_UID), anyInt()))
+ .thenReturn(false);
- WifiInfo connectionInfo = mWsm.syncRequestConnectionInfo(mContext.getOpPackageName());
+ WifiInfo connectionInfo = mWsm.syncRequestConnectionInfo(mContext.getOpPackageName(),
+ TEST_UID);
- assertNotEquals(wifiInfo, connectionInfo);
- assertEquals(WifiSsid.NONE, connectionInfo.getSSID());
- assertEquals(WifiInfo.DEFAULT_MAC_ADDRESS, connectionInfo.getBSSID());
- } finally {
- BinderUtil.setUid(actualUid);
- }
+ assertNotEquals(wifiInfo, connectionInfo);
+ assertEquals(WifiSsid.NONE, connectionInfo.getSSID());
+ assertEquals(WifiInfo.DEFAULT_MAC_ADDRESS, connectionInfo.getBSSID());
}
/**
@@ -2354,29 +2333,48 @@
*/
@Test
public void testConnectedIdsAreHiddenOnSecurityException() throws Exception {
- int actualUid = Binder.getCallingUid();
- int fakeUid = Process.myUid() + 100000;
- assertNotEquals(actualUid, fakeUid);
- BinderUtil.setUid(fakeUid);
- try {
- WifiInfo wifiInfo = mWsm.getWifiInfo();
+ WifiInfo wifiInfo = mWsm.getWifiInfo();
- // Get into a connected state, with known BSSID and SSID
- connect();
- assertEquals(sBSSID, wifiInfo.getBSSID());
- assertEquals(sWifiSsid, wifiInfo.getWifiSsid());
+ // Get into a connected state, with known BSSID and SSID
+ connect();
+ assertEquals(sBSSID, wifiInfo.getBSSID());
+ assertEquals(sWifiSsid, wifiInfo.getWifiSsid());
- when(mWifiPermissionsUtil.canAccessScanResults(anyString(), eq(fakeUid), anyInt()))
- .thenThrow(new SecurityException());
+ when(mWifiPermissionsUtil.canAccessScanResults(anyString(), eq(TEST_UID), anyInt()))
+ .thenThrow(new SecurityException());
- WifiInfo connectionInfo = mWsm.syncRequestConnectionInfo(mContext.getOpPackageName());
+ WifiInfo connectionInfo = mWsm.syncRequestConnectionInfo(mContext.getOpPackageName(),
+ TEST_UID);
- assertNotEquals(wifiInfo, connectionInfo);
- assertEquals(WifiSsid.NONE, connectionInfo.getSSID());
- assertEquals(WifiInfo.DEFAULT_MAC_ADDRESS, connectionInfo.getBSSID());
- } finally {
- BinderUtil.setUid(actualUid);
- }
+ assertNotEquals(wifiInfo, connectionInfo);
+ assertEquals(WifiSsid.NONE, connectionInfo.getSSID());
+ assertEquals(WifiInfo.DEFAULT_MAC_ADDRESS, connectionInfo.getBSSID());
+ }
+
+ /**
+ * Test that connected SSID and BSSID are exposed to system server
+ */
+ @Test
+ public void testConnectedIdsAreVisibleFromSystemServer() throws Exception {
+ when(mWifiPermissionsWrapper.getLocalMacAddressPermission(anyInt()))
+ .thenReturn(PackageManager.PERMISSION_GRANTED);
+
+ WifiInfo wifiInfo = mWsm.getWifiInfo();
+ // Get into a connected state, with known BSSID and SSID
+ connect();
+ assertEquals(sBSSID, wifiInfo.getBSSID());
+ assertEquals(sWifiSsid, wifiInfo.getWifiSsid());
+
+ when(mWifiPermissionsUtil.canAccessScanResults(anyString(), eq(TEST_UID), anyInt()))
+ .thenReturn(true);
+
+ WifiInfo connectionInfo = mWsm.syncRequestConnectionInfo(mContext.getOpPackageName(),
+ TEST_UID);
+
+ assertNotEquals(wifiInfo, connectionInfo);
+ assertEquals(wifiInfo.getSSID(), connectionInfo.getSSID());
+ assertEquals(wifiInfo.getBSSID(), connectionInfo.getBSSID());
+ assertEquals(wifiInfo.getMacAddress(), connectionInfo.getMacAddress());
}
/**
@@ -2385,30 +2383,24 @@
*/
@Test
public void testConnectedIdsAreVisibleFromPermittedApp() throws Exception {
- int actualUid = Binder.getCallingUid();
- int fakeUid = Process.myUid() + 100000;
- BinderUtil.setUid(fakeUid);
- try {
- WifiInfo wifiInfo = mWsm.getWifiInfo();
+ WifiInfo wifiInfo = mWsm.getWifiInfo();
- // Get into a connected state, with known BSSID and SSID
- connect();
- assertEquals(sBSSID, wifiInfo.getBSSID());
- assertEquals(sWifiSsid, wifiInfo.getWifiSsid());
+ // Get into a connected state, with known BSSID and SSID
+ connect();
+ assertEquals(sBSSID, wifiInfo.getBSSID());
+ assertEquals(sWifiSsid, wifiInfo.getWifiSsid());
- when(mWifiPermissionsUtil.canAccessScanResults(anyString(), eq(fakeUid), anyInt()))
- .thenReturn(true);
+ when(mWifiPermissionsUtil.canAccessScanResults(anyString(), eq(TEST_UID), anyInt()))
+ .thenReturn(true);
- WifiInfo connectionInfo = mWsm.syncRequestConnectionInfo(mContext.getOpPackageName());
+ WifiInfo connectionInfo = mWsm.syncRequestConnectionInfo(mContext.getOpPackageName(),
+ TEST_UID);
- assertNotEquals(wifiInfo, connectionInfo);
- assertEquals(wifiInfo.getSSID(), connectionInfo.getSSID());
- assertEquals(wifiInfo.getBSSID(), connectionInfo.getBSSID());
- // Access to our MAC address uses a different permission, make sure it is not granted
- assertEquals(WifiInfo.DEFAULT_MAC_ADDRESS, connectionInfo.getMacAddress());
- } finally {
- BinderUtil.setUid(actualUid);
- }
+ assertNotEquals(wifiInfo, connectionInfo);
+ assertEquals(wifiInfo.getSSID(), connectionInfo.getSSID());
+ assertEquals(wifiInfo.getBSSID(), connectionInfo.getBSSID());
+ // Access to our MAC address uses a different permission, make sure it is not granted
+ assertEquals(WifiInfo.DEFAULT_MAC_ADDRESS, connectionInfo.getMacAddress());
}
/**