Merge "Minor fixes to LockdownVpnTracker." am: 74cbf1917b

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1585566

MUST ONLY BE SUBMITTED BY AUTOMERGER

Change-Id: Ib400714e6faa8cf06487c5d6c721fdd444164fa4

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

@@ -4910,12 +4910,15 @@ public class ConnectivityService extends IConnectivityManager.Stub
         if (!mLockdownEnabled) {
             return null;
         }
-        // The legacy lockdown VPN always only applies to UID 0.
+        // The legacy lockdown VPN always only applies to userId 0.
         final NetworkAgentInfo nai = getVpnForUid(Process.FIRST_APPLICATION_UID);
         if (nai == null || !isLegacyLockdownNai(nai)) return null;
 
         // The legacy lockdown VPN must always have exactly one underlying network.
-        if (nai.declaredUnderlyingNetworks == null ||  nai.declaredUnderlyingNetworks.length != 1) {
+        // This code may run on any thread and declaredUnderlyingNetworks may change, so store it in
+        // a local variable. There is no need to make a copy because its contents cannot change.
+        final Network[] underlying = nai.declaredUnderlyingNetworks;
+        if (underlying == null ||  underlying.length != 1) {
             return null;
         }
 
@@ -4925,8 +4928,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         // Report that the VPN is not connected, so when the state of NetworkInfo objects
         // overwritten by getLegacyLockdownState will be set to CONNECTING and not CONNECTED.
         final NetworkAgentInfo defaultNetwork = getDefaultNetwork();
-        if (defaultNetwork == null
+        if (defaultNetwork == null || !defaultNetwork.network.equals(underlying[0])) {
-                || !defaultNetwork.network.equals(nai.declaredUnderlyingNetworks[0])) {
             return null;
         }
 
@@ -4985,6 +4987,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
     private BroadcastReceiver mIntentReceiver = new BroadcastReceiver() {
         @Override
         public void onReceive(Context context, Intent intent) {
+            // TODO: make BroadcastInterceptingContext use the Handler passed in to registerReceiver
+            // and put this back.
+            // ensureRunningOnConnectivityServiceThread();
             final String action = intent.getAction();
             final int userId = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, UserHandle.USER_NULL);
 

services/core/java/com/android/server/net/LockdownVpnTracker.java

@@ -16,7 +16,6 @@
 
 package com.android.server.net;
 
-import static android.net.ConnectivityManager.TYPE_NONE;
 import static android.net.NetworkCapabilities.TRANSPORT_VPN;
 import static android.net.VpnManager.NOTIFICATION_CHANNEL_VPN;
 import static android.provider.Settings.ACTION_VPN_SETTINGS;
@@ -34,7 +33,6 @@ import android.net.LinkAddress;
 import android.net.LinkProperties;
 import android.net.Network;
 import android.net.NetworkInfo;
-import android.net.NetworkInfo.DetailedState;
 import android.net.NetworkRequest;
 import android.os.Handler;
 import android.security.KeyStore;
@@ -45,7 +43,6 @@ import com.android.internal.R;
 import com.android.internal.messages.nano.SystemMessageProto.SystemMessage;
 import com.android.internal.net.VpnConfig;
 import com.android.internal.net.VpnProfile;
-import com.android.server.EventLogTags;
 import com.android.server.connectivity.Vpn;
 
 import java.util.List;
@@ -58,9 +55,6 @@ import java.util.Objects;
 public class LockdownVpnTracker {
     private static final String TAG = "LockdownVpnTracker";
 
-    /** Number of VPN attempts before waiting for user intervention. */
-    private static final int MAX_ERROR_COUNT = 4;
-
     public static final String ACTION_LOCKDOWN_RESET = "com.android.server.action.LOCKDOWN_RESET";
 
     @NonNull private final Context mContext;
@@ -83,6 +77,7 @@ public class LockdownVpnTracker {
         private Network mNetwork = null;
         private LinkProperties mLinkProperties = null;
 
+        @Override
         public void onLinkPropertiesChanged(Network network, LinkProperties lp) {
             boolean networkChanged = false;
             if (!network.equals(mNetwork)) {
@@ -100,6 +95,7 @@ public class LockdownVpnTracker {
             }
         }
 
+        @Override
         public void onLost(Network network) {
             // The default network has gone down.
             mNetwork = null;
@@ -134,8 +130,6 @@ public class LockdownVpnTracker {
     @Nullable
     private String mAcceptedEgressIface;
 
-    private int mErrorCount;
-
     public LockdownVpnTracker(@NonNull Context context,
             @NonNull Handler handler,
             @NonNull KeyStore keyStore,
@@ -165,7 +159,6 @@ public class LockdownVpnTracker {
      */
     private void handleStateChangedLocked() {
         final Network network = mDefaultNetworkCallback.getNetwork();
-        final NetworkInfo egressInfo = mCm.getNetworkInfo(network);  // Only for logging
         final LinkProperties egressProp = mDefaultNetworkCallback.getLinkProperties();
 
         final NetworkInfo vpnInfo = mVpn.getNetworkInfo();
@@ -176,11 +169,9 @@ public class LockdownVpnTracker {
         final boolean egressChanged = egressProp == null
                 || !TextUtils.equals(mAcceptedEgressIface, egressProp.getInterfaceName());
 
-        final int egressType = (egressInfo == null) ? TYPE_NONE : egressInfo.getType();
         final String egressIface = (egressProp == null) ?
                 null : egressProp.getInterfaceName();
-        Log.d(TAG, "handleStateChanged: egress=" + egressType
+        Log.d(TAG, "handleStateChanged: egress=" + mAcceptedEgressIface + "->" + egressIface);
-                + " " + mAcceptedEgressIface + "->" + egressIface);
 
         if (egressDisconnected || egressChanged) {
             mAcceptedEgressIface = null;
@@ -190,15 +181,6 @@ public class LockdownVpnTracker {
             hideNotification();
             return;
         }
-        if (vpnInfo.getDetailedState() == DetailedState.FAILED) {
-            EventLogTags.writeLockdownVpnError(egressType);
-        }
-
-        if (mErrorCount > MAX_ERROR_COUNT) {
-            // Cannot happen because ConnectivityService never sees a NetworkInfo in state FAILED.
-            showNotification(R.string.vpn_lockdown_error, R.drawable.vpn_disconnected);
-            return;
-        }
 
         // At this point, |network| is known to be non-null.
         if (!vpnInfo.isConnectedOrConnecting()) {
@@ -209,7 +191,6 @@ public class LockdownVpnTracker {
             }
 
             Log.d(TAG, "Active network connected; starting VPN");
-            EventLogTags.writeLockdownVpnConnecting(egressType);
             showNotification(R.string.vpn_lockdown_connecting, R.drawable.vpn_disconnected);
 
             mAcceptedEgressIface = egressIface;
@@ -243,7 +224,6 @@ public class LockdownVpnTracker {
 
             Log.d(TAG, "VPN connected using iface=" + iface
                     + ", sourceAddr=" + sourceAddrs.toString());
-            EventLogTags.writeLockdownVpnConnected(egressType);
             showNotification(R.string.vpn_lockdown_connected, R.drawable.vpn_connected);
         }
     }
@@ -280,7 +260,6 @@ public class LockdownVpnTracker {
         Log.d(TAG, "shutdownLocked()");
 
         mAcceptedEgressIface = null;
-        mErrorCount = 0;
 
         mVpn.stopVpnRunnerPrivileged();
         mVpn.setLockdown(false);