ConnectivityService: synchronize access on mLockdownEnabled
The mLockdownEnabled boolean and the mLockdownTracker objects are read and mutated in many places involving vpn logic inside of ConnectivityService. This includes codepaths run on the ConnectivityService handler and codepaths run on Binder calls from IConnectivityManager.aidl, however the access to these variables are not synchronized. This patch adds proper synchronization to mLockdownEnabled and mLockdownTracker by moving access to them into the mVpns lock used for all of vpn logic. Bug: 18331877 Test: runtest frameworks-net Change-Id: I4abde43b1036861f4486dd2b5567782d10204bd6
This commit is contained in:
@@ -224,7 +224,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
@GuardedBy("mVpns")
|
||||
private final SparseArray<Vpn> mVpns = new SparseArray<Vpn>();
|
||||
|
||||
// TODO: investigate if mLockdownEnabled can be removed and replaced everywhere by
|
||||
// a direct call to LockdownVpnTracker.isEnabled().
|
||||
@GuardedBy("mVpns")
|
||||
private boolean mLockdownEnabled;
|
||||
@GuardedBy("mVpns")
|
||||
private LockdownVpnTracker mLockdownTracker;
|
||||
|
||||
final private Context mContext;
|
||||
@@ -972,9 +976,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
}
|
||||
|
||||
private Network[] getVpnUnderlyingNetworks(int uid) {
|
||||
synchronized (mVpns) {
|
||||
if (!mLockdownEnabled) {
|
||||
int user = UserHandle.getUserId(uid);
|
||||
synchronized (mVpns) {
|
||||
Vpn vpn = mVpns.get(user);
|
||||
if (vpn != null && vpn.appliesToUid(uid)) {
|
||||
return vpn.getUnderlyingNetworks();
|
||||
@@ -1062,10 +1066,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
if (isNetworkWithLinkPropertiesBlocked(state.linkProperties, uid, ignoreBlocked)) {
|
||||
state.networkInfo.setDetailedState(DetailedState.BLOCKED, null, null);
|
||||
}
|
||||
synchronized (mVpns) {
|
||||
if (mLockdownTracker != null) {
|
||||
mLockdownTracker.augmentNetworkInfo(state.networkInfo);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Return NetworkInfo for the active (i.e., connected) network interface.
|
||||
@@ -1228,8 +1234,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
result.put(nai.network, nc);
|
||||
}
|
||||
|
||||
if (!mLockdownEnabled) {
|
||||
synchronized (mVpns) {
|
||||
if (!mLockdownEnabled) {
|
||||
Vpn vpn = mVpns.get(userId);
|
||||
if (vpn != null) {
|
||||
Network[] networks = vpn.getUnderlyingNetworks();
|
||||
@@ -1555,10 +1561,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
}
|
||||
|
||||
private Intent makeGeneralIntent(NetworkInfo info, String bcastType) {
|
||||
synchronized (mVpns) {
|
||||
if (mLockdownTracker != null) {
|
||||
info = new NetworkInfo(info);
|
||||
mLockdownTracker.augmentNetworkInfo(info);
|
||||
}
|
||||
}
|
||||
|
||||
Intent intent = new Intent(bcastType);
|
||||
intent.putExtra(ConnectivityManager.EXTRA_NETWORK_INFO, new NetworkInfo(info));
|
||||
@@ -3422,9 +3430,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
public boolean prepareVpn(@Nullable String oldPackage, @Nullable String newPackage,
|
||||
int userId) {
|
||||
enforceCrossUserPermission(userId);
|
||||
throwIfLockdownEnabled();
|
||||
|
||||
synchronized (mVpns) {
|
||||
throwIfLockdownEnabled();
|
||||
Vpn vpn = mVpns.get(userId);
|
||||
if (vpn != null) {
|
||||
return vpn.prepare(oldPackage, newPackage);
|
||||
@@ -3468,9 +3476,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
*/
|
||||
@Override
|
||||
public ParcelFileDescriptor establishVpn(VpnConfig config) {
|
||||
throwIfLockdownEnabled();
|
||||
int user = UserHandle.getUserId(Binder.getCallingUid());
|
||||
synchronized (mVpns) {
|
||||
throwIfLockdownEnabled();
|
||||
return mVpns.get(user).establish(config);
|
||||
}
|
||||
}
|
||||
@@ -3481,13 +3489,13 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
*/
|
||||
@Override
|
||||
public void startLegacyVpn(VpnProfile profile) {
|
||||
throwIfLockdownEnabled();
|
||||
int user = UserHandle.getUserId(Binder.getCallingUid());
|
||||
final LinkProperties egress = getActiveLinkProperties();
|
||||
if (egress == null) {
|
||||
throw new IllegalStateException("Missing active network connection");
|
||||
}
|
||||
int user = UserHandle.getUserId(Binder.getCallingUid());
|
||||
synchronized (mVpns) {
|
||||
throwIfLockdownEnabled();
|
||||
mVpns.get(user).startLegacyVpn(profile, mKeyStore, egress);
|
||||
}
|
||||
}
|
||||
@@ -3513,11 +3521,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
@Override
|
||||
public VpnInfo[] getAllVpnInfo() {
|
||||
enforceConnectivityInternalPermission();
|
||||
synchronized (mVpns) {
|
||||
if (mLockdownEnabled) {
|
||||
return new VpnInfo[0];
|
||||
}
|
||||
|
||||
synchronized (mVpns) {
|
||||
List<VpnInfo> infoList = new ArrayList<>();
|
||||
for (int i = 0; i < mVpns.size(); i++) {
|
||||
VpnInfo info = createVpnInfo(mVpns.valueAt(i));
|
||||
@@ -3582,6 +3590,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
return false;
|
||||
}
|
||||
|
||||
synchronized (mVpns) {
|
||||
// Tear down existing lockdown if profile was removed
|
||||
mLockdownEnabled = LockdownVpnTracker.isEnabled();
|
||||
if (mLockdownEnabled) {
|
||||
@@ -3599,17 +3608,16 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
return true;
|
||||
}
|
||||
int user = UserHandle.getUserId(Binder.getCallingUid());
|
||||
synchronized (mVpns) {
|
||||
Vpn vpn = mVpns.get(user);
|
||||
if (vpn == null) {
|
||||
Slog.w(TAG, "VPN for user " + user + " not ready yet. Skipping lockdown");
|
||||
return false;
|
||||
}
|
||||
setLockdownTracker(new LockdownVpnTracker(mContext, mNetd, this, vpn, profile));
|
||||
}
|
||||
} else {
|
||||
setLockdownTracker(null);
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
@@ -3618,6 +3626,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
* Internally set new {@link LockdownVpnTracker}, shutting down any existing
|
||||
* {@link LockdownVpnTracker}. Can be {@code null} to disable lockdown.
|
||||
*/
|
||||
@GuardedBy("mVpns")
|
||||
private void setLockdownTracker(LockdownVpnTracker tracker) {
|
||||
// Shutdown any existing tracker
|
||||
final LockdownVpnTracker existing = mLockdownTracker;
|
||||
@@ -3632,6 +3641,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
}
|
||||
}
|
||||
|
||||
@GuardedBy("mVpns")
|
||||
private void throwIfLockdownEnabled() {
|
||||
if (mLockdownEnabled) {
|
||||
throw new IllegalStateException("Unavailable in lockdown mode");
|
||||
@@ -3679,12 +3689,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
enforceConnectivityInternalPermission();
|
||||
enforceCrossUserPermission(userId);
|
||||
|
||||
synchronized (mVpns) {
|
||||
// Can't set always-on VPN if legacy VPN is already in lockdown mode.
|
||||
if (LockdownVpnTracker.isEnabled()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
synchronized (mVpns) {
|
||||
Vpn vpn = mVpns.get(userId);
|
||||
if (vpn == null) {
|
||||
Slog.w(TAG, "User " + userId + " has no Vpn configuration");
|
||||
@@ -3860,11 +3870,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
}
|
||||
userVpn = new Vpn(mHandler.getLooper(), mContext, mNetd, userId);
|
||||
mVpns.put(userId, userVpn);
|
||||
}
|
||||
if (mUserManager.getUserInfo(userId).isPrimary() && LockdownVpnTracker.isEnabled()) {
|
||||
updateLockdownVpn();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private void onUserStop(int userId) {
|
||||
synchronized (mVpns) {
|
||||
@@ -3899,6 +3909,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
}
|
||||
|
||||
private void onUserUnlocked(int userId) {
|
||||
synchronized (mVpns) {
|
||||
// User present may be sent because of an unlock, which might mean an unlocked keystore.
|
||||
if (mUserManager.getUserInfo(userId).isPrimary() && LockdownVpnTracker.isEnabled()) {
|
||||
updateLockdownVpn();
|
||||
@@ -3906,6 +3917,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
startAlwaysOnVpn(userId);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private BroadcastReceiver mUserIntentReceiver = new BroadcastReceiver() {
|
||||
@Override
|
||||
@@ -5215,6 +5227,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
}
|
||||
|
||||
private void notifyLockdownVpn(NetworkAgentInfo nai) {
|
||||
synchronized (mVpns) {
|
||||
if (mLockdownTracker != null) {
|
||||
if (nai != null && nai.isVPN()) {
|
||||
mLockdownTracker.onVpnStateChanged(nai.networkInfo);
|
||||
@@ -5223,6 +5236,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private void updateNetworkInfo(NetworkAgentInfo networkAgent, NetworkInfo newInfo) {
|
||||
final NetworkInfo.State state = newInfo.getState();
|
||||
@@ -5449,28 +5463,28 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
|
||||
@Override
|
||||
public boolean addVpnAddress(String address, int prefixLength) {
|
||||
throwIfLockdownEnabled();
|
||||
int user = UserHandle.getUserId(Binder.getCallingUid());
|
||||
synchronized (mVpns) {
|
||||
throwIfLockdownEnabled();
|
||||
return mVpns.get(user).addAddress(address, prefixLength);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean removeVpnAddress(String address, int prefixLength) {
|
||||
throwIfLockdownEnabled();
|
||||
int user = UserHandle.getUserId(Binder.getCallingUid());
|
||||
synchronized (mVpns) {
|
||||
throwIfLockdownEnabled();
|
||||
return mVpns.get(user).removeAddress(address, prefixLength);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean setUnderlyingNetworksForVpn(Network[] networks) {
|
||||
throwIfLockdownEnabled();
|
||||
int user = UserHandle.getUserId(Binder.getCallingUid());
|
||||
boolean success;
|
||||
final boolean success;
|
||||
synchronized (mVpns) {
|
||||
throwIfLockdownEnabled();
|
||||
success = mVpns.get(user).setUnderlyingNetworks(networks);
|
||||
}
|
||||
if (success) {
|
||||
@@ -5530,7 +5544,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
setAlwaysOnVpnPackage(userId, null, false);
|
||||
setVpnPackageAuthorization(alwaysOnPackage, userId, false);
|
||||
}
|
||||
}
|
||||
|
||||
// Turn Always-on VPN off
|
||||
if (mLockdownEnabled && userId == UserHandle.USER_SYSTEM) {
|
||||
@@ -5550,14 +5563,15 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
if (vpnConfig.legacy) {
|
||||
prepareVpn(VpnConfig.LEGACY_VPN, VpnConfig.LEGACY_VPN, userId);
|
||||
} else {
|
||||
// Prevent this app (packagename = vpnConfig.user) from initiating VPN connections
|
||||
// in the future without user intervention.
|
||||
// Prevent this app (packagename = vpnConfig.user) from initiating
|
||||
// VPN connections in the future without user intervention.
|
||||
setVpnPackageAuthorization(vpnConfig.user, userId, false);
|
||||
|
||||
prepareVpn(null, VpnConfig.LEGACY_VPN, userId);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Settings.Global.putString(mContext.getContentResolver(),
|
||||
Settings.Global.NETWORK_AVOID_BAD_WIFI, null);
|
||||
|
||||
Reference in New Issue
Block a user