summaryrefslogtreecommitdiff
path: root/service
diff options
context:
space:
mode:
authorNate Jiang <qiangjiang@google.com>2020-12-03 14:31:30 -0800
committerRoshan Pius <rpius@google.com>2020-12-24 03:54:55 +0000
commit95673e85133c78773870fb5161bb300b7f2ee51e (patch)
tree4f1cbb5f3051a155f2f4464eaf032608322d9458 /service
parent7b701dfe989079784ff1076ce67cb2fb7665b798 (diff)
[Suggestion] Check foreground user for API call
Also, squashes the follow up commit to create a single CL for backporting: ======= PasspointManager: Don't allow bg user to modify passpoint profiles Also, add safety net logging for this bug. ======= Bug: 174749461 Test: atest com.android.server.wifi Change-Id: Ifc79ffeb04a7be99a9c60d9414b72e88275c0514 Merged-In: Ifc79ffeb04a7be99a9c60d9414b72e88275c0514 (cherry picked from commit e799efba85cbe52044a067869af71d9c15b573bb) (cherry picked from commit 23685b8604571ec623e539f4f9c66db65c9dde81)
Diffstat (limited to 'service')
-rw-r--r--service/java/com/android/server/wifi/ClientModeImpl.java4
-rw-r--r--service/java/com/android/server/wifi/WifiConfigManager.java44
-rw-r--r--service/java/com/android/server/wifi/WifiInjector.java2
-rw-r--r--service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java10
-rw-r--r--service/java/com/android/server/wifi/WifiServiceImpl.java3
-rw-r--r--service/java/com/android/server/wifi/hotspot2/PasspointManager.java20
-rw-r--r--service/java/com/android/server/wifi/util/WifiPermissionsUtil.java29
7 files changed, 71 insertions, 41 deletions
diff --git a/service/java/com/android/server/wifi/ClientModeImpl.java b/service/java/com/android/server/wifi/ClientModeImpl.java
index d98d022b7..12c45e90f 100644
--- a/service/java/com/android/server/wifi/ClientModeImpl.java
+++ b/service/java/com/android/server/wifi/ClientModeImpl.java
@@ -3678,7 +3678,7 @@ public class ClientModeImpl extends StateMachine {
break;
case CMD_REMOVE_PASSPOINT_CONFIG:
int removeResult = mPasspointManager.removeProvider(
- (String) message.obj) ? SUCCESS : FAILURE;
+ message.sendingUid, (String) message.obj) ? SUCCESS : FAILURE;
replyToMessage(message, message.what, removeResult);
break;
case CMD_GET_PASSPOINT_CONFIGS:
@@ -4517,7 +4517,7 @@ public class ClientModeImpl extends StateMachine {
break;
case CMD_REMOVE_PASSPOINT_CONFIG:
String fqdn = (String) message.obj;
- if (mPasspointManager.removeProvider(fqdn)) {
+ if (mPasspointManager.removeProvider(message.sendingUid, fqdn)) {
if (isProviderOwnedNetwork(mTargetNetworkId, fqdn)
|| isProviderOwnedNetwork(mLastNetworkId, fqdn)) {
logd("Disconnect from current network since its provider is removed");
diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java
index 0e07e086f..00430c80f 100644
--- a/service/java/com/android/server/wifi/WifiConfigManager.java
+++ b/service/java/com/android/server/wifi/WifiConfigManager.java
@@ -897,28 +897,6 @@ public class WifiConfigManager {
}
/**
- * Check if the given UID belongs to the current foreground user. This is
- * used to prevent apps running in background users from modifying network
- * configurations.
- * <p>
- * UIDs belonging to system internals (such as SystemUI) are always allowed,
- * since they always run as {@link UserHandle#USER_SYSTEM}.
- *
- * @param uid uid of the app.
- * @return true if the given UID belongs to the current foreground user,
- * otherwise false.
- */
- private boolean doesUidBelongToCurrentUser(int uid) {
- if (uid == android.os.Process.SYSTEM_UID || uid == mSystemUiUid) {
- return true;
- } else {
- return WifiConfigurationUtil.doesUidBelongToAnyProfile(
- uid, mUserManager.getProfiles(mCurrentUserId));
- }
- }
-
- /**
- * Copy over public elements from an external WifiConfiguration object to the internal
* configuration object if element has been set in the provided external WifiConfiguration.
* The only exception is the hidden |IpConfiguration| parameters, these need to be copied over
* for every update.
@@ -1285,7 +1263,7 @@ public class WifiConfigManager {
*/
public NetworkUpdateResult addOrUpdateNetwork(WifiConfiguration config, int uid,
@Nullable String packageName) {
- if (!doesUidBelongToCurrentUser(uid)) {
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
Log.e(TAG, "UID " + uid + " not visible to the current user");
return new NetworkUpdateResult(WifiConfiguration.INVALID_NETWORK_ID);
}
@@ -1387,7 +1365,7 @@ public class WifiConfigManager {
* @return true if successful, false otherwise.
*/
public boolean removeNetwork(int networkId, int uid) {
- if (!doesUidBelongToCurrentUser(uid)) {
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
Log.e(TAG, "UID " + uid + " not visible to the current user");
return false;
}
@@ -1771,7 +1749,7 @@ public class WifiConfigManager {
if (mVerboseLoggingEnabled) {
Log.v(TAG, "Enabling network " + networkId + " (disableOthers " + disableOthers + ")");
}
- if (!doesUidBelongToCurrentUser(uid)) {
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
Log.e(TAG, "UID " + uid + " not visible to the current user");
return false;
}
@@ -1809,7 +1787,7 @@ public class WifiConfigManager {
if (mVerboseLoggingEnabled) {
Log.v(TAG, "Disabling network " + networkId);
}
- if (!doesUidBelongToCurrentUser(uid)) {
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
Log.e(TAG, "UID " + uid + " not visible to the current user");
return false;
}
@@ -1846,7 +1824,7 @@ public class WifiConfigManager {
if (mVerboseLoggingEnabled) {
Log.v(TAG, "Update network last connect UID for " + networkId);
}
- if (!doesUidBelongToCurrentUser(uid)) {
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
Log.e(TAG, "UID " + uid + " not visible to the current user");
return false;
}
@@ -3011,8 +2989,8 @@ public class WifiConfigManager {
Set<Integer> removedNetworkIds = new HashSet<>();
// Remove any private networks of the old user before switching the userId.
for (WifiConfiguration config : getInternalConfiguredNetworks()) {
- if (!config.shared && WifiConfigurationUtil.doesUidBelongToAnyProfile(
- config.creatorUid, mUserManager.getProfiles(userId))) {
+ if (!config.shared && !mWifiPermissionsUtil
+ .doesUidBelongToCurrentUser(config.creatorUid)) {
removedNetworkIds.add(config.networkId);
localLog("clearInternalUserData: removed config."
+ " netId=" + config.networkId
@@ -3222,8 +3200,8 @@ public class WifiConfigManager {
// Migrate the legacy Passpoint configurations owned by the current user to
// {@link PasspointManager}.
- if (config.isLegacyPasspointConfig && WifiConfigurationUtil.doesUidBelongToAnyProfile(
- config.creatorUid, mUserManager.getProfiles(mCurrentUserId))) {
+ if (config.isLegacyPasspointConfig && !mWifiPermissionsUtil
+ .doesUidBelongToCurrentUser(config.creatorUid)) {
legacyPasspointNetId.add(config.networkId);
// Migrate the legacy Passpoint configuration and add it to PasspointManager.
if (!PasspointManager.addLegacyPasspointConfig(config)) {
@@ -3240,8 +3218,8 @@ public class WifiConfigManager {
// because all networks were previously stored in a central file. We cannot
// write these private networks to the user specific store until the corresponding
// user logs in.
- if (config.shared || !WifiConfigurationUtil.doesUidBelongToAnyProfile(
- config.creatorUid, mUserManager.getProfiles(mCurrentUserId))) {
+ if (config.shared || !mWifiPermissionsUtil
+ .doesUidBelongToCurrentUser(config.creatorUid)) {
sharedConfigurations.add(config);
} else {
userConfigurations.add(config);
diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java
index 9ffca1326..1f89f6df8 100644
--- a/service/java/com/android/server/wifi/WifiInjector.java
+++ b/service/java/com/android/server/wifi/WifiInjector.java
@@ -282,7 +282,7 @@ public class WifiInjector {
mPasspointManager = new PasspointManager(mContext, this,
new Handler(mWifiCoreHandlerThread.getLooper()), mWifiNative, mWifiKeyStore, mClock,
mSimAccessor, new PasspointObjectFactory(), mWifiConfigManager, mWifiConfigStore,
- mWifiMetrics, makeTelephonyManager(), subscriptionManager);
+ mWifiMetrics, makeTelephonyManager(), subscriptionManager, mWifiPermissionsUtil);
mPasspointNetworkEvaluator = new PasspointNetworkEvaluator(
mPasspointManager, mWifiConfigManager, mConnectivityLocalLog,
mCarrierNetworkConfig, this, subscriptionManager);
diff --git a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java
index 8feb3711e..0cb844509 100644
--- a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java
+++ b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java
@@ -557,6 +557,10 @@ public class WifiNetworkSuggestionsManager {
*/
public @WifiManager.NetworkSuggestionsStatusCode int add(
List<WifiNetworkSuggestion> networkSuggestions, int uid, String packageName) {
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
+ Log.e(TAG, "UID " + uid + " not visible to the current user");
+ return WifiManager.STATUS_NETWORK_SUGGESTIONS_ERROR_INTERNAL;
+ }
if (mVerboseLoggingEnabled) {
Log.v(TAG, "Adding " + networkSuggestions.size() + " networks from " + packageName);
}
@@ -667,7 +671,11 @@ public class WifiNetworkSuggestionsManager {
* Remove the provided list of network suggestions from the corresponding app's active list.
*/
public @WifiManager.NetworkSuggestionsStatusCode int remove(
- List<WifiNetworkSuggestion> networkSuggestions, String packageName) {
+ List<WifiNetworkSuggestion> networkSuggestions, int uid, String packageName) {
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
+ Log.e(TAG, "UID " + uid + " not visible to the current user");
+ return WifiManager.STATUS_NETWORK_SUGGESTIONS_ERROR_INTERNAL;
+ }
if (mVerboseLoggingEnabled) {
Log.v(TAG, "Removing " + networkSuggestions.size() + " networks from " + packageName);
}
diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java
index a4639924b..eedf2bd75 100644
--- a/service/java/com/android/server/wifi/WifiServiceImpl.java
+++ b/service/java/com/android/server/wifi/WifiServiceImpl.java
@@ -3314,11 +3314,12 @@ public class WifiServiceImpl extends BaseWifiService {
if (mVerboseLoggingEnabled) {
mLog.info("removeNetworkSuggestions uid=%").c(Binder.getCallingUid()).flush();
}
+ int callingUid = Binder.getCallingUid();
Mutable<Integer> success = new Mutable<>();
boolean runWithScissorsSuccess = mWifiInjector.getClientModeImplHandler().runWithScissors(
() -> {
success.value = mWifiNetworkSuggestionsManager.remove(
- networkSuggestions, callingPackageName);
+ networkSuggestions, callingUid, callingPackageName);
}, RUN_WITH_SCISSORS_TIMEOUT_MILLIS);
if (!runWithScissorsSuccess) {
Log.e(TAG, "Failed to post runnable to remove network suggestions");
diff --git a/service/java/com/android/server/wifi/hotspot2/PasspointManager.java b/service/java/com/android/server/wifi/hotspot2/PasspointManager.java
index 8f4349a1f..bdb323be8 100644
--- a/service/java/com/android/server/wifi/hotspot2/PasspointManager.java
+++ b/service/java/com/android/server/wifi/hotspot2/PasspointManager.java
@@ -59,6 +59,7 @@ import com.android.server.wifi.hotspot2.anqp.NAIRealmElement;
import com.android.server.wifi.hotspot2.anqp.OsuProviderInfo;
import com.android.server.wifi.util.InformationElementUtil;
import com.android.server.wifi.util.TelephonyUtil;
+import com.android.server.wifi.util.WifiPermissionsUtil;
import java.io.PrintWriter;
import java.security.cert.X509Certificate;
@@ -117,6 +118,8 @@ public class PasspointManager {
private final TelephonyManager mTelephonyManager;
private final AppOpsManager mAppOps;
private final SubscriptionManager mSubscriptionManager;
+ private final WifiPermissionsUtil mWifiPermissionsUtil;
+
/**
* Map of package name of an app to the app ops changed listener for the app.
@@ -249,7 +252,7 @@ public class PasspointManager {
for (Map.Entry<String, PasspointProvider> entry : getPasspointProviderWithPackage(
packageName).entrySet()) {
String fqdn = entry.getValue().getConfig().getHomeSp().getFqdn();
- removeProvider(fqdn);
+ removeProvider(Process.WIFI_UID, fqdn);
disconnectIfPasspointNetwork(fqdn);
}
}
@@ -297,7 +300,8 @@ public class PasspointManager {
PasspointObjectFactory objectFactory, WifiConfigManager wifiConfigManager,
WifiConfigStore wifiConfigStore,
WifiMetrics wifiMetrics,
- TelephonyManager telephonyManager, SubscriptionManager subscriptionManager) {
+ TelephonyManager telephonyManager, SubscriptionManager subscriptionManager,
+ WifiPermissionsUtil wifiPermissionsUtil) {
mPasspointEventHandler = objectFactory.makePasspointEventHandler(wifiNative,
new CallbackHandler(context));
mWifiInjector = wifiInjector;
@@ -322,6 +326,7 @@ public class PasspointManager {
this, wifiMetrics);
mAppOps = (AppOpsManager) context.getSystemService(Context.APP_OPS_SERVICE);
sPasspointManager = this;
+ mWifiPermissionsUtil = wifiPermissionsUtil;
}
/**
@@ -361,6 +366,10 @@ public class PasspointManager {
Log.e(TAG, "Invalid configuration");
return false;
}
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
+ Log.e(TAG, "UID " + uid + " not visible to the current user");
+ return false;
+ }
// For Hotspot 2.0 Release 1, the CA Certificate must be trusted by one of the pre-loaded
// public CAs in the system key store on the device. Since the provisioning method
@@ -619,16 +628,21 @@ public class PasspointManager {
/**
* Remove a Passpoint provider identified by the given FQDN.
*
+ * @param callingUid Calling UID.
* @param fqdn The FQDN of the provider to remove
* @return true if a provider is removed, false otherwise
*/
- public boolean removeProvider(String fqdn) {
+ public boolean removeProvider(int callingUid, String fqdn) {
mWifiMetrics.incrementNumPasspointProviderUninstallation();
String packageName;
if (!mProviders.containsKey(fqdn)) {
Log.e(TAG, "Config doesn't exist");
return false;
}
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(callingUid)) {
+ Log.e(TAG, "UID " + callingUid + " not visible to the current user");
+ return false;
+ }
mProviders.get(fqdn).uninstallCertsAndKeys();
packageName = mProviders.get(fqdn).getPackageName();
mProviders.remove(fqdn);
diff --git a/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java b/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
index 2834ad765..a08d87673 100644
--- a/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
+++ b/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
@@ -29,6 +29,7 @@ import android.os.RemoteException;
import android.os.UserHandle;
import android.os.UserManager;
import android.util.Slog;
+import android.util.EventLog;
import com.android.internal.annotations.GuardedBy;
import com.android.server.wifi.WifiInjector;
@@ -516,4 +517,32 @@ public class WifiPermissionsUtil {
}
return mode == AppOpsManager.MODE_ALLOWED;
}
+
+ /**
+ * Check if the given UID belongs to the current foreground user. This is
+ * used to prevent apps running in background users from modifying network
+ * configurations.
+ * <p>
+ * UIDs belonging to system internals (such as SystemUI) are always allowed,
+ * since they always run as {@link UserHandle#USER_SYSTEM}.
+ *
+ * @param uid uid of the app.
+ * @return true if the given UID belongs to the current foreground user,
+ * otherwise false.
+ */
+ public boolean doesUidBelongToCurrentUser(int uid) {
+ if (uid == android.os.Process.SYSTEM_UID
+ // UIDs with the NETWORK_SETTINGS permission are always allowed since they are
+ // acting on behalf of the user.
+ || checkNetworkSettingsPermission(uid)) {
+ return true;
+ }
+ boolean isCurrentProfile = isCurrentProfile(uid);
+ if (!isCurrentProfile) {
+ // Fix for b/174749461
+ EventLog.writeEvent(0x534e4554, "174749461", -1,
+ "Non foreground user trying to modify wifi configuration");
+ }
+ return isCurrentProfile;
+ }
}