Merge "ConnectivityService: synchronize access on mLockdownEnabled" am: dca465ab21 am: 2d59e3cab0

am: 9c8cc85a3d

Change-Id: I7cb4d1e9e1fc3dc5ca116dcc94840920cd4a3d64

services/core/java/com/android/server/ConnectivityService.java

@@ -226,7 +226,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;
@@ -997,9 +1001,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();
@@ -1087,10 +1091,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.
@@ -1253,8 +1259,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();
@@ -1580,10 +1586,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));
@@ -3436,9 +3444,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);
@@ -3482,9 +3490,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);
         }
     }
@@ -3495,13 +3503,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);
         }
     }
@@ -3527,11 +3535,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));
@@ -3596,6 +3604,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
             return false;
         }
 
+        synchronized (mVpns) {
             // Tear down existing lockdown if profile was removed
             mLockdownEnabled = LockdownVpnTracker.isEnabled();
             if (mLockdownEnabled) {
@@ -3613,17 +3622,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;
     }
@@ -3632,6 +3640,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;
@@ -3646,6 +3655,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
     }
 
+    @GuardedBy("mVpns")
     private void throwIfLockdownEnabled() {
         if (mLockdownEnabled) {
             throw new IllegalStateException("Unavailable in lockdown mode");
@@ -3693,12 +3703,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");
@@ -3874,11 +3884,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) {
@@ -3913,6 +3923,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();
@@ -3920,6 +3931,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 startAlwaysOnVpn(userId);
             }
         }
+    }
 
     private BroadcastReceiver mUserIntentReceiver = new BroadcastReceiver() {
         @Override
@@ -5205,6 +5217,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);
@@ -5213,6 +5226,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 }
             }
         }
+    }
 
     private void updateNetworkInfo(NetworkAgentInfo networkAgent, NetworkInfo newInfo) {
         final NetworkInfo.State state = newInfo.getState();
@@ -5439,28 +5453,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) {
@@ -5520,7 +5534,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) {
@@ -5540,14 +5553,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
+                        // Prevent this app (packagename = vpnConfig.user) from initiating
-                    // in the future without user intervention.
+                        // 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);