summaryrefslogtreecommitdiff
path: root/service
diff options
context:
space:
mode:
authorNate Jiang <qiangjiang@google.com>2020-12-03 14:31:30 -0800
committerAnis Assi <anisassi@google.com>2021-01-08 02:01:24 -0800
commit65bfcc085766588910758922f4d31a305d23c70d (patch)
tree8d90a8d307e3954190f3a4c34d89f13a09903133 /service
parent6295916af26c0726a5ab969647cc6d11386f8b67 (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) (cherry picked from commit 1ffc81e5470b3e98f86ac04a7e4f9daec89768b3)
Diffstat (limited to 'service')
-rw-r--r--service/java/com/android/server/wifi/WifiConfigManager.java45
-rw-r--r--service/java/com/android/server/wifi/WifiInjector.java3
-rw-r--r--service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java29
-rw-r--r--service/java/com/android/server/wifi/WifiServiceImpl.java13
-rw-r--r--service/java/com/android/server/wifi/hotspot2/PasspointManager.java15
-rw-r--r--service/java/com/android/server/wifi/util/WifiPermissionsUtil.java29
6 files changed, 89 insertions, 45 deletions
diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java
index 6d04b7a68..c0cb899d2 100644
--- a/service/java/com/android/server/wifi/WifiConfigManager.java
+++ b/service/java/com/android/server/wifi/WifiConfigManager.java
@@ -913,32 +913,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
- // UIDs with the NETWORK_SETTINGS permission are always allowed since they are
- // acting on behalf of the user.
- || mWifiPermissionsUtil.checkNetworkSettingsPermission(uid)) {
- return true;
- } else {
- UserHandle currentUser = UserHandle.of(mCurrentUserId);
- UserHandle callingUser = UserHandle.getUserHandleForUid(uid);
- return currentUser.equals(callingUser)
- || mUserManager.isSameProfileGroup(currentUser, callingUser);
- }
- }
-
- /**
* 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
@@ -1334,7 +1308,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);
}
@@ -1440,7 +1414,7 @@ public class WifiConfigManager {
* @return true if successful, false otherwise.
*/
public boolean removeNetwork(int networkId, int uid, String packageName) {
- if (!doesUidBelongToCurrentUser(uid)) {
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
Log.e(TAG, "UID " + uid + " not visible to the current user");
return false;
}
@@ -1849,7 +1823,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;
}
@@ -1887,7 +1861,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;
}
@@ -1955,7 +1929,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;
}
@@ -2927,7 +2901,8 @@ public class WifiConfigManager {
Set<Integer> removedNetworkIds = new HashSet<>();
// Remove any private networks of the old user before switching the userId.
for (WifiConfiguration config : getConfiguredNetworks()) {
- if (!config.shared && doesUidBelongToCurrentUser(config.creatorUid)) {
+ if ((!config.shared && !mWifiPermissionsUtil
+ .doesUidBelongToCurrentUser(config.creatorUid))) {
removedNetworkIds.add(config.networkId);
localLog("clearInternalUserData: removed config."
+ " netId=" + config.networkId
@@ -3146,7 +3121,8 @@ public class WifiConfigManager {
// Migrate the legacy Passpoint configurations owned by the current user to
// {@link PasspointManager}.
- if (config.isLegacyPasspointConfig && doesUidBelongToCurrentUser(config.creatorUid)) {
+ 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)) {
@@ -3166,7 +3142,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 || !doesUidBelongToCurrentUser(config.creatorUid)) {
+ 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 b9aa0521f..5531c4e98 100644
--- a/service/java/com/android/server/wifi/WifiInjector.java
+++ b/service/java/com/android/server/wifi/WifiInjector.java
@@ -304,7 +304,8 @@ public class WifiInjector {
mWifiCarrierInfoManager, mWifiKeyStore, mLruConnectionTracker);
mPasspointManager = new PasspointManager(mContext, this,
wifiHandler, mWifiNative, mWifiKeyStore, mClock, new PasspointObjectFactory(),
- mWifiConfigManager, mWifiConfigStore, mWifiMetrics, mWifiCarrierInfoManager);
+ mWifiConfigManager, mWifiConfigStore, mWifiMetrics, mWifiCarrierInfoManager,
+ mWifiPermissionsUtil);
PasspointNetworkNominateHelper nominateHelper =
new PasspointNetworkNominateHelper(mPasspointManager, mWifiConfigManager,
mConnectivityLocalLog);
diff --git a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java
index 464ced0ad..2632835e6 100644
--- a/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java
+++ b/service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java
@@ -390,6 +390,7 @@ public class WifiNetworkSuggestionsManager {
private boolean mIsLastUserApprovalUiDialog = false;
private boolean mUserDataLoaded = false;
+
/**
* Listener for app-ops changes for active suggestor apps.
*/
@@ -833,6 +834,10 @@ public class WifiNetworkSuggestionsManager {
public @WifiManager.NetworkSuggestionsStatusCode int add(
List<WifiNetworkSuggestion> networkSuggestions, int uid, String packageName,
@Nullable String featureId) {
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
+ Log.e(TAG, "UID " + uid + " not visible to the current user");
+ return WifiManager.STATUS_NETWORK_SUGGESTIONS_ERROR_INTERNAL;
+ }
if (!mUserDataLoaded) {
Log.e(TAG, "Add Network suggestion before boot complete is not allowed.");
return WifiManager.STATUS_NETWORK_SUGGESTIONS_ERROR_INTERNAL;
@@ -1108,6 +1113,10 @@ public class WifiNetworkSuggestionsManager {
*/
public @WifiManager.NetworkSuggestionsStatusCode int remove(
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 (!mUserDataLoaded) {
Log.e(TAG, "Remove Network suggestion before boot complete is not allowed.");
return WifiManager.STATUS_NETWORK_SUGGESTIONS_ERROR_INTERNAL;
@@ -1166,8 +1175,12 @@ public class WifiNetworkSuggestionsManager {
* Get all network suggestion for target App
* @return List of WifiNetworkSuggestions
*/
- public @NonNull List<WifiNetworkSuggestion> get(@NonNull String packageName) {
+ public @NonNull List<WifiNetworkSuggestion> get(@NonNull String packageName, int uid) {
List<WifiNetworkSuggestion> networkSuggestionList = new ArrayList<>();
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
+ Log.e(TAG, "UID " + uid + " not visible to the current user");
+ return networkSuggestionList;
+ }
if (!mUserDataLoaded) {
Log.e(TAG, "Get Network suggestion before boot complete is not allowed.");
return networkSuggestionList;
@@ -1923,11 +1936,16 @@ public class WifiNetworkSuggestionsManager {
* @param binder IBinder instance to allow cleanup if the app dies.
* @param listener ISuggestionNetworkCallback instance to add.
* @param listenerIdentifier identifier of the listener, should be hash code of listener.
+ * @param uid uid of the app.
* @return true if succeed otherwise false.
*/
public boolean registerSuggestionConnectionStatusListener(@NonNull IBinder binder,
@NonNull ISuggestionConnectionStatusListener listener,
- int listenerIdentifier, String packageName) {
+ int listenerIdentifier, String packageName, int uid) {
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
+ Log.e(TAG, "UID " + uid + " not visible to the current user");
+ return false;
+ }
ExternalCallbackTracker<ISuggestionConnectionStatusListener> listenersTracker =
mSuggestionStatusListenerPerApp.get(packageName);
if (listenersTracker == null) {
@@ -1942,9 +1960,14 @@ public class WifiNetworkSuggestionsManager {
/**
* Unregister a listener on network connection failure.
* @param listenerIdentifier identifier of the listener, should be hash code of listener.
+ * @param uid uid of the app.
*/
public void unregisterSuggestionConnectionStatusListener(int listenerIdentifier,
- String packageName) {
+ String packageName, int uid) {
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
+ Log.e(TAG, "UID " + uid + " not visible to the current user");
+ return;
+ }
ExternalCallbackTracker<ISuggestionConnectionStatusListener> listenersTracker =
mSuggestionStatusListenerPerApp.get(packageName);
if (listenersTracker == null || listenersTracker.remove(listenerIdentifier) == null) {
diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java
index a398eacaf..20e82d845 100644
--- a/service/java/com/android/server/wifi/WifiServiceImpl.java
+++ b/service/java/com/android/server/wifi/WifiServiceImpl.java
@@ -3888,13 +3888,15 @@ public class WifiServiceImpl extends BaseWifiService {
* @return a list of network suggestions suggested by this app
*/
public List<WifiNetworkSuggestion> getNetworkSuggestions(String callingPackageName) {
- mAppOps.checkPackage(Binder.getCallingUid(), callingPackageName);
+ int callingUid = Binder.getCallingUid();
+ mAppOps.checkPackage(callingUid, callingPackageName);
enforceAccessPermission();
if (mVerboseLoggingEnabled) {
mLog.info("getNetworkSuggestionList uid=%").c(Binder.getCallingUid()).flush();
}
return mWifiThreadRunner.call(() ->
- mWifiNetworkSuggestionsManager.get(callingPackageName), Collections.emptyList());
+ mWifiNetworkSuggestionsManager.get(callingPackageName, callingUid),
+ Collections.emptyList());
}
/**
@@ -4213,7 +4215,7 @@ public class WifiServiceImpl extends BaseWifiService {
mWifiThreadRunner.post(() ->
mWifiNetworkSuggestionsManager
.registerSuggestionConnectionStatusListener(binder, listener,
- listenerIdentifier, packageName));
+ listenerIdentifier, packageName, uid));
}
/**
@@ -4223,14 +4225,15 @@ public class WifiServiceImpl extends BaseWifiService {
public void unregisterSuggestionConnectionStatusListener(
int listenerIdentifier, String packageName) {
enforceAccessPermission();
+ int uid = Binder.getCallingUid();
if (mVerboseLoggingEnabled) {
mLog.info("unregisterSuggestionConnectionStatusListener uid=%")
- .c(Binder.getCallingUid()).flush();
+ .c(uid).flush();
}
mWifiThreadRunner.post(() ->
mWifiNetworkSuggestionsManager
.unregisterSuggestionConnectionStatusListener(listenerIdentifier,
- packageName));
+ packageName, uid));
}
@Override
diff --git a/service/java/com/android/server/wifi/hotspot2/PasspointManager.java b/service/java/com/android/server/wifi/hotspot2/PasspointManager.java
index 25553b8fe..2c291f024 100644
--- a/service/java/com/android/server/wifi/hotspot2/PasspointManager.java
+++ b/service/java/com/android/server/wifi/hotspot2/PasspointManager.java
@@ -54,6 +54,7 @@ import com.android.server.wifi.hotspot2.anqp.HSOsuProvidersElement;
import com.android.server.wifi.hotspot2.anqp.OsuProviderInfo;
import com.android.server.wifi.proto.nano.WifiMetricsProto.UserActionEvent;
import com.android.server.wifi.util.InformationElementUtil;
+import com.android.server.wifi.util.WifiPermissionsUtil;
import java.io.IOException;
import java.io.PrintWriter;
@@ -118,7 +119,7 @@ public class PasspointManager {
private final PasspointProvisioner mPasspointProvisioner;
private final AppOpsManager mAppOps;
private final WifiCarrierInfoManager mWifiCarrierInfoManager;
-
+ private final WifiPermissionsUtil mWifiPermissionsUtil;
/**
* Map of package name of an app to the app ops changed listener for the app.
*/
@@ -304,7 +305,8 @@ public class PasspointManager {
PasspointObjectFactory objectFactory, WifiConfigManager wifiConfigManager,
WifiConfigStore wifiConfigStore,
WifiMetrics wifiMetrics,
- WifiCarrierInfoManager wifiCarrierInfoManager) {
+ WifiCarrierInfoManager wifiCarrierInfoManager,
+ WifiPermissionsUtil wifiPermissionsUtil) {
mPasspointEventHandler = objectFactory.makePasspointEventHandler(wifiNative,
new CallbackHandler(context));
mWifiInjector = wifiInjector;
@@ -326,6 +328,7 @@ public class PasspointManager {
this, wifiMetrics);
mAppOps = (AppOpsManager) context.getSystemService(Context.APP_OPS_SERVICE);
sPasspointManager = this;
+ mWifiPermissionsUtil = wifiPermissionsUtil;
}
/**
@@ -406,6 +409,10 @@ public class PasspointManager {
Log.e(TAG, "Set isTrusted to false on a non suggestion passpoint is not allowed");
return false;
}
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
+ Log.e(TAG, "UID " + uid + " not visible to the current user");
+ return false;
+ }
mWifiCarrierInfoManager.tryUpdateCarrierIdForPasspoint(config);
// Create a provider and install the necessary certificates and keys.
@@ -499,6 +506,10 @@ public class PasspointManager {
+ provider.getCreatorUid());
return false;
}
+ if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(callingUid)) {
+ Log.e(TAG, "UID " + callingUid + " not visible to the current user");
+ return false;
+ }
provider.uninstallCertsAndKeys();
String packageName = provider.getPackageName();
// Remove any configs corresponding to the profile in WifiConfigManager.
diff --git a/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java b/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
index 63197ea6d..ebe7ea481 100644
--- a/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
+++ b/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
@@ -30,6 +30,7 @@ import android.os.Build;
import android.os.UserHandle;
import android.os.UserManager;
import android.provider.Settings;
+import android.util.EventLog;
import android.util.Log;
import com.android.internal.annotations.GuardedBy;
@@ -589,4 +590,32 @@ public class WifiPermissionsUtil {
if (devicePolicyManager == null) return false;
return devicePolicyManager.isProfileOwnerApp(packageName);
}
+
+ /**
+ * 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;
+ }
}