Skip to content

Commit 1ffc81e

Browse files
Nate Jiangrpius
Nate Jiang
authored and
rpius
committed
[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 e799efb) (cherry picked from commit 23685b8)
1 parent b2bf62b commit 1ffc81e

File tree

10 files changed

+216
-69
lines changed

10 files changed

+216
-69
lines changed

‎service/java/com/android/server/wifi/WifiConfigManager.java

Lines changed: 11 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -912,32 +912,6 @@ private boolean canModifyNetwork(WifiConfiguration config, int uid,
912912
&& mWifiPermissionsUtil.checkNetworkSettingsPermission(uid);
913913
}
914914

915-
/**
916-
* Check if the given UID belongs to the current foreground user. This is
917-
* used to prevent apps running in background users from modifying network
918-
* configurations.
919-
* <p>
920-
* UIDs belonging to system internals (such as SystemUI) are always allowed,
921-
* since they always run as {@link UserHandle#USER_SYSTEM}.
922-
*
923-
* @param uid uid of the app.
924-
* @return true if the given UID belongs to the current foreground user,
925-
* otherwise false.
926-
*/
927-
private boolean doesUidBelongToCurrentUser(int uid) {
928-
if (uid == android.os.Process.SYSTEM_UID
929-
// UIDs with the NETWORK_SETTINGS permission are always allowed since they are
930-
// acting on behalf of the user.
931-
|| mWifiPermissionsUtil.checkNetworkSettingsPermission(uid)) {
932-
return true;
933-
} else {
934-
UserHandle currentUser = UserHandle.of(mCurrentUserId);
935-
UserHandle callingUser = UserHandle.getUserHandleForUid(uid);
936-
return currentUser.equals(callingUser)
937-
|| mUserManager.isSameProfileGroup(currentUser, callingUser);
938-
}
939-
}
940-
941915
/**
942916
* Copy over public elements from an external WifiConfiguration object to the internal
943917
* configuration object if element has been set in the provided external WifiConfiguration.
@@ -1334,7 +1308,7 @@ private NetworkUpdateResult addOrUpdateNetworkInternal(WifiConfiguration config,
13341308
*/
13351309
public NetworkUpdateResult addOrUpdateNetwork(WifiConfiguration config, int uid,
13361310
@Nullable String packageName) {
1337-
if (!doesUidBelongToCurrentUser(uid)) {
1311+
if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
13381312
Log.e(TAG, "UID " + uid + " not visible to the current user");
13391313
return new NetworkUpdateResult(WifiConfiguration.INVALID_NETWORK_ID);
13401314
}
@@ -1440,7 +1414,7 @@ private boolean removeNetworkInternal(WifiConfiguration config, int uid) {
14401414
* @return true if successful, false otherwise.
14411415
*/
14421416
public boolean removeNetwork(int networkId, int uid, String packageName) {
1443-
if (!doesUidBelongToCurrentUser(uid)) {
1417+
if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
14441418
Log.e(TAG, "UID " + uid + " not visible to the current user");
14451419
return false;
14461420
}
@@ -1849,7 +1823,7 @@ public boolean enableNetwork(int networkId, boolean disableOthers, int uid,
18491823
if (mVerboseLoggingEnabled) {
18501824
Log.v(TAG, "Enabling network " + networkId + " (disableOthers " + disableOthers + ")");
18511825
}
1852-
if (!doesUidBelongToCurrentUser(uid)) {
1826+
if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
18531827
Log.e(TAG, "UID " + uid + " not visible to the current user");
18541828
return false;
18551829
}
@@ -1887,7 +1861,7 @@ public boolean disableNetwork(int networkId, int uid, String packageName) {
18871861
if (mVerboseLoggingEnabled) {
18881862
Log.v(TAG, "Disabling network " + networkId);
18891863
}
1890-
if (!doesUidBelongToCurrentUser(uid)) {
1864+
if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
18911865
Log.e(TAG, "UID " + uid + " not visible to the current user");
18921866
return false;
18931867
}
@@ -1955,7 +1929,7 @@ public boolean updateLastConnectUid(int networkId, int uid) {
19551929
if (mVerboseLoggingEnabled) {
19561930
Log.v(TAG, "Update network last connect UID for " + networkId);
19571931
}
1958-
if (!doesUidBelongToCurrentUser(uid)) {
1932+
if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
19591933
Log.e(TAG, "UID " + uid + " not visible to the current user");
19601934
return false;
19611935
}
@@ -2927,7 +2901,8 @@ private Set<Integer> clearInternalDataForCurrentUser() {
29272901
Set<Integer> removedNetworkIds = new HashSet<>();
29282902
// Remove any private networks of the old user before switching the userId.
29292903
for (WifiConfiguration config : getConfiguredNetworks()) {
2930-
if (!config.shared && doesUidBelongToCurrentUser(config.creatorUid)) {
2904+
if ((!config.shared && !mWifiPermissionsUtil
2905+
.doesUidBelongToCurrentUser(config.creatorUid))) {
29312906
removedNetworkIds.add(config.networkId);
29322907
localLog("clearInternalUserData: removed config."
29332908
+ " netId=" + config.networkId
@@ -3146,7 +3121,8 @@ public boolean saveToStore(boolean forceWrite) {
31463121

31473122
// Migrate the legacy Passpoint configurations owned by the current user to
31483123
// {@link PasspointManager}.
3149-
if (config.isLegacyPasspointConfig && doesUidBelongToCurrentUser(config.creatorUid)) {
3124+
if (config.isLegacyPasspointConfig && !mWifiPermissionsUtil
3125+
.doesUidBelongToCurrentUser(config.creatorUid)) {
31503126
legacyPasspointNetId.add(config.networkId);
31513127
// Migrate the legacy Passpoint configuration and add it to PasspointManager.
31523128
if (!PasspointManager.addLegacyPasspointConfig(config)) {
@@ -3166,7 +3142,8 @@ public boolean saveToStore(boolean forceWrite) {
31663142
// because all networks were previously stored in a central file. We cannot
31673143
// write these private networks to the user specific store until the corresponding
31683144
// user logs in.
3169-
if (config.shared || !doesUidBelongToCurrentUser(config.creatorUid)) {
3145+
if (config.shared || !mWifiPermissionsUtil
3146+
.doesUidBelongToCurrentUser(config.creatorUid)) {
31703147
sharedConfigurations.add(config);
31713148
} else {
31723149
userConfigurations.add(config);

‎service/java/com/android/server/wifi/WifiInjector.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,8 @@ awareMetrics, rttMetrics, new WifiPowerMetrics(mBatteryStats), mWifiP2pMetrics,
304304
mWifiCarrierInfoManager, mWifiKeyStore, mLruConnectionTracker);
305305
mPasspointManager = new PasspointManager(mContext, this,
306306
wifiHandler, mWifiNative, mWifiKeyStore, mClock, new PasspointObjectFactory(),
307-
mWifiConfigManager, mWifiConfigStore, mWifiMetrics, mWifiCarrierInfoManager);
307+
mWifiConfigManager, mWifiConfigStore, mWifiMetrics, mWifiCarrierInfoManager,
308+
mWifiPermissionsUtil);
308309
PasspointNetworkNominateHelper nominateHelper =
309310
new PasspointNetworkNominateHelper(mPasspointManager, mWifiConfigManager,
310311
mConnectivityLocalLog);

‎service/java/com/android/server/wifi/WifiNetworkSuggestionsManager.java

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,7 @@ public WifiConfiguration createInternalWifiConfiguration() {
390390
private boolean mIsLastUserApprovalUiDialog = false;
391391

392392
private boolean mUserDataLoaded = false;
393+
393394
/**
394395
* Listener for app-ops changes for active suggestor apps.
395396
*/
@@ -833,6 +834,10 @@ private void updateWifiConfigInWcmIfPresent(
833834
public @WifiManager.NetworkSuggestionsStatusCode int add(
834835
List<WifiNetworkSuggestion> networkSuggestions, int uid, String packageName,
835836
@Nullable String featureId) {
837+
if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
838+
Log.e(TAG, "UID " + uid + " not visible to the current user");
839+
return WifiManager.STATUS_NETWORK_SUGGESTIONS_ERROR_INTERNAL;
840+
}
836841
if (!mUserDataLoaded) {
837842
Log.e(TAG, "Add Network suggestion before boot complete is not allowed.");
838843
return WifiManager.STATUS_NETWORK_SUGGESTIONS_ERROR_INTERNAL;
@@ -1108,6 +1113,10 @@ private void removeInternal(
11081113
*/
11091114
public @WifiManager.NetworkSuggestionsStatusCode int remove(
11101115
List<WifiNetworkSuggestion> networkSuggestions, int uid, String packageName) {
1116+
if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
1117+
Log.e(TAG, "UID " + uid + " not visible to the current user");
1118+
return WifiManager.STATUS_NETWORK_SUGGESTIONS_ERROR_INTERNAL;
1119+
}
11111120
if (!mUserDataLoaded) {
11121121
Log.e(TAG, "Remove Network suggestion before boot complete is not allowed.");
11131122
return WifiManager.STATUS_NETWORK_SUGGESTIONS_ERROR_INTERNAL;
@@ -1166,8 +1175,12 @@ public void removeApp(@NonNull String packageName) {
11661175
* Get all network suggestion for target App
11671176
* @return List of WifiNetworkSuggestions
11681177
*/
1169-
public @NonNull List<WifiNetworkSuggestion> get(@NonNull String packageName) {
1178+
public @NonNull List<WifiNetworkSuggestion> get(@NonNull String packageName, int uid) {
11701179
List<WifiNetworkSuggestion> networkSuggestionList = new ArrayList<>();
1180+
if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
1181+
Log.e(TAG, "UID " + uid + " not visible to the current user");
1182+
return networkSuggestionList;
1183+
}
11711184
if (!mUserDataLoaded) {
11721185
Log.e(TAG, "Get Network suggestion before boot complete is not allowed.");
11731186
return networkSuggestionList;
@@ -1923,11 +1936,16 @@ int internalConnectionEventToSuggestionFailureCode(int connectionEvent) {
19231936
* @param binder IBinder instance to allow cleanup if the app dies.
19241937
* @param listener ISuggestionNetworkCallback instance to add.
19251938
* @param listenerIdentifier identifier of the listener, should be hash code of listener.
1939+
* @param uid uid of the app.
19261940
* @return true if succeed otherwise false.
19271941
*/
19281942
public boolean registerSuggestionConnectionStatusListener(@NonNull IBinder binder,
19291943
@NonNull ISuggestionConnectionStatusListener listener,
1930-
int listenerIdentifier, String packageName) {
1944+
int listenerIdentifier, String packageName, int uid) {
1945+
if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
1946+
Log.e(TAG, "UID " + uid + " not visible to the current user");
1947+
return false;
1948+
}
19311949
ExternalCallbackTracker<ISuggestionConnectionStatusListener> listenersTracker =
19321950
mSuggestionStatusListenerPerApp.get(packageName);
19331951
if (listenersTracker == null) {
@@ -1942,9 +1960,14 @@ public boolean registerSuggestionConnectionStatusListener(@NonNull IBinder binde
19421960
/**
19431961
* Unregister a listener on network connection failure.
19441962
* @param listenerIdentifier identifier of the listener, should be hash code of listener.
1963+
* @param uid uid of the app.
19451964
*/
19461965
public void unregisterSuggestionConnectionStatusListener(int listenerIdentifier,
1947-
String packageName) {
1966+
String packageName, int uid) {
1967+
if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
1968+
Log.e(TAG, "UID " + uid + " not visible to the current user");
1969+
return;
1970+
}
19481971
ExternalCallbackTracker<ISuggestionConnectionStatusListener> listenersTracker =
19491972
mSuggestionStatusListenerPerApp.get(packageName);
19501973
if (listenersTracker == null || listenersTracker.remove(listenerIdentifier) == null) {

‎service/java/com/android/server/wifi/WifiServiceImpl.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3888,13 +3888,15 @@ public int removeNetworkSuggestions(
38883888
* @return a list of network suggestions suggested by this app
38893889
*/
38903890
public List<WifiNetworkSuggestion> getNetworkSuggestions(String callingPackageName) {
3891-
mAppOps.checkPackage(Binder.getCallingUid(), callingPackageName);
3891+
int callingUid = Binder.getCallingUid();
3892+
mAppOps.checkPackage(callingUid, callingPackageName);
38923893
enforceAccessPermission();
38933894
if (mVerboseLoggingEnabled) {
38943895
mLog.info("getNetworkSuggestionList uid=%").c(Binder.getCallingUid()).flush();
38953896
}
38963897
return mWifiThreadRunner.call(() ->
3897-
mWifiNetworkSuggestionsManager.get(callingPackageName), Collections.emptyList());
3898+
mWifiNetworkSuggestionsManager.get(callingPackageName, callingUid),
3899+
Collections.emptyList());
38983900
}
38993901

39003902
/**
@@ -4213,7 +4215,7 @@ public void registerSuggestionConnectionStatusListener(IBinder binder,
42134215
mWifiThreadRunner.post(() ->
42144216
mWifiNetworkSuggestionsManager
42154217
.registerSuggestionConnectionStatusListener(binder, listener,
4216-
listenerIdentifier, packageName));
4218+
listenerIdentifier, packageName, uid));
42174219
}
42184220

42194221
/**
@@ -4223,14 +4225,15 @@ public void registerSuggestionConnectionStatusListener(IBinder binder,
42234225
public void unregisterSuggestionConnectionStatusListener(
42244226
int listenerIdentifier, String packageName) {
42254227
enforceAccessPermission();
4228+
int uid = Binder.getCallingUid();
42264229
if (mVerboseLoggingEnabled) {
42274230
mLog.info("unregisterSuggestionConnectionStatusListener uid=%")
4228-
.c(Binder.getCallingUid()).flush();
4231+
.c(uid).flush();
42294232
}
42304233
mWifiThreadRunner.post(() ->
42314234
mWifiNetworkSuggestionsManager
42324235
.unregisterSuggestionConnectionStatusListener(listenerIdentifier,
4233-
packageName));
4236+
packageName, uid));
42344237
}
42354238

42364239
@Override

‎service/java/com/android/server/wifi/hotspot2/PasspointManager.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import com.android.server.wifi.hotspot2.anqp.OsuProviderInfo;
5555
import com.android.server.wifi.proto.nano.WifiMetricsProto.UserActionEvent;
5656
import com.android.server.wifi.util.InformationElementUtil;
57+
import com.android.server.wifi.util.WifiPermissionsUtil;
5758

5859
import java.io.IOException;
5960
import java.io.PrintWriter;
@@ -118,7 +119,7 @@ public class PasspointManager {
118119
private final PasspointProvisioner mPasspointProvisioner;
119120
private final AppOpsManager mAppOps;
120121
private final WifiCarrierInfoManager mWifiCarrierInfoManager;
121-
122+
private final WifiPermissionsUtil mWifiPermissionsUtil;
122123
/**
123124
* Map of package name of an app to the app ops changed listener for the app.
124125
*/
@@ -304,7 +305,8 @@ public PasspointManager(Context context, WifiInjector wifiInjector, Handler hand
304305
PasspointObjectFactory objectFactory, WifiConfigManager wifiConfigManager,
305306
WifiConfigStore wifiConfigStore,
306307
WifiMetrics wifiMetrics,
307-
WifiCarrierInfoManager wifiCarrierInfoManager) {
308+
WifiCarrierInfoManager wifiCarrierInfoManager,
309+
WifiPermissionsUtil wifiPermissionsUtil) {
308310
mPasspointEventHandler = objectFactory.makePasspointEventHandler(wifiNative,
309311
new CallbackHandler(context));
310312
mWifiInjector = wifiInjector;
@@ -326,6 +328,7 @@ public PasspointManager(Context context, WifiInjector wifiInjector, Handler hand
326328
this, wifiMetrics);
327329
mAppOps = (AppOpsManager) context.getSystemService(Context.APP_OPS_SERVICE);
328330
sPasspointManager = this;
331+
mWifiPermissionsUtil = wifiPermissionsUtil;
329332
}
330333

331334
/**
@@ -406,6 +409,10 @@ public boolean addOrUpdateProvider(PasspointConfiguration config, int uid,
406409
Log.e(TAG, "Set isTrusted to false on a non suggestion passpoint is not allowed");
407410
return false;
408411
}
412+
if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(uid)) {
413+
Log.e(TAG, "UID " + uid + " not visible to the current user");
414+
return false;
415+
}
409416

410417
mWifiCarrierInfoManager.tryUpdateCarrierIdForPasspoint(config);
411418
// Create a provider and install the necessary certificates and keys.
@@ -499,6 +506,10 @@ private boolean removeProviderInternal(PasspointProvider provider, int callingUi
499506
+ provider.getCreatorUid());
500507
return false;
501508
}
509+
if (!mWifiPermissionsUtil.doesUidBelongToCurrentUser(callingUid)) {
510+
Log.e(TAG, "UID " + callingUid + " not visible to the current user");
511+
return false;
512+
}
502513
provider.uninstallCertsAndKeys();
503514
String packageName = provider.getPackageName();
504515
// Remove any configs corresponding to the profile in WifiConfigManager.

‎service/java/com/android/server/wifi/util/WifiPermissionsUtil.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import android.os.UserHandle;
3131
import android.os.UserManager;
3232
import android.provider.Settings;
33+
import android.util.EventLog;
3334
import android.util.Log;
3435

3536
import com.android.internal.annotations.GuardedBy;
@@ -589,4 +590,32 @@ public boolean isProfileOwner(int uid, @Nullable String packageName) {
589590
if (devicePolicyManager == null) return false;
590591
return devicePolicyManager.isProfileOwnerApp(packageName);
591592
}
593+
594+
/**
595+
* Check if the given UID belongs to the current foreground user. This is
596+
* used to prevent apps running in background users from modifying network
597+
* configurations.
598+
* <p>
599+
* UIDs belonging to system internals (such as SystemUI) are always allowed,
600+
* since they always run as {@link UserHandle#USER_SYSTEM}.
601+
*
602+
* @param uid uid of the app.
603+
* @return true if the given UID belongs to the current foreground user,
604+
* otherwise false.
605+
*/
606+
public boolean doesUidBelongToCurrentUser(int uid) {
607+
if (uid == android.os.Process.SYSTEM_UID
608+
// UIDs with the NETWORK_SETTINGS permission are always allowed since they are
609+
// acting on behalf of the user.
610+
|| checkNetworkSettingsPermission(uid)) {
611+
return true;
612+
}
613+
boolean isCurrentProfile = isCurrentProfile(uid);
614+
if (!isCurrentProfile) {
615+
// Fix for b/174749461
616+
EventLog.writeEvent(0x534e4554, "174749461", -1,
617+
"Non foreground user trying to modify wifi configuration");
618+
}
619+
return isCurrentProfile;
620+
}
592621
}

0 commit comments

Comments
 (0)