From 42d38ded1ee54607aedac8465045b0a8f88094ec Mon Sep 17 00:00:00 2001
From: lucaslin <lucaslin@google.com>
Date: Fri, 6 Jan 2023 08:03:31 +0000
Subject: Clear the VPN network preference when error happens

This commit does
- Clear the VPN network preference before the Ikev2VpnRunner
  becomes stale.
- Clear the VPN network preference when the retry delay is higher
  than 5s. The clean work is not suitable for NETWORK_LOST error
  since there might be a window when the underlying network is
  back but the VPN is not connected yet. So keep the rule when
  the underlying network is lost.
- Reset the VPN network preference when starting a new IKE
  session. The rule might be set twice(One is in the constructor,
  and the other is in startOrMigrateIkeSession()) when the IKEv2
  VPN is the first time launched. There is no harm since
  ConnectivityService will always clear the old rule first then
  adding the new rule, which means it's ok for the caller to set
  the same rule multiple times and ConnectivityService can handle
  it well.

Bug: 231749077
Test: atest FrameworksNetTests
Change-Id: I2664b5fb4151e12d2531e950885bcb18e46b23c6
---
 .../java/com/android/server/connectivity/Vpn.java   | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java
index a12243b8e4fa..b97d75dc68d7 100644
--- a/services/core/java/com/android/server/connectivity/Vpn.java
+++ b/services/core/java/com/android/server/connectivity/Vpn.java
@@ -3385,6 +3385,13 @@ public class Vpn {
          *              given network to start a new IKE session.
          */
         private void startOrMigrateIkeSession(@Nullable Network underlyingNetwork) {
+            synchronized (Vpn.this) {
+                // Ignore stale runner.
+                if (mVpnRunner != this) return;
+                setVpnNetworkPreference(mSessionKey,
+                        createUserAndRestrictedProfilesRanges(mUserId,
+                                mConfig.allowedApplications, mConfig.disallowedApplications));
+            }
             if (underlyingNetwork == null) {
                 // For null underlyingNetwork case, there will not be a NetworkAgent available so
                 // no underlying network update is necessary here. Note that updating
@@ -3905,6 +3912,7 @@ public class Vpn {
                 updateState(DetailedState.FAILED, exception.getMessage());
             }
 
+            clearVpnNetworkPreference(mSessionKey);
             disconnectVpnRunner();
         }
 
@@ -4039,6 +4047,13 @@ public class Vpn {
             }
 
             resetIkeState();
+            if (errorCode != VpnManager.ERROR_CODE_NETWORK_LOST
+                    // Clear the VPN network preference when the retry delay is higher than 5s.
+                    // mRetryCount was increased when scheduleRetryNewIkeSession() is called,
+                    // therefore use mRetryCount - 1 here.
+                    && mDeps.getNextRetryDelayMs(mRetryCount - 1) > 5_000L) {
+                clearVpnNetworkPreference(mSessionKey);
+            }
         }
 
         /**
@@ -4085,13 +4100,17 @@ public class Vpn {
             mCarrierConfigManager.unregisterCarrierConfigChangeListener(
                     mCarrierConfigChangeListener);
             mConnectivityManager.unregisterNetworkCallback(mNetworkCallback);
-            clearVpnNetworkPreference(mSessionKey);
 
             mExecutor.shutdown();
         }
 
         @Override
         public void exitVpnRunner() {
+            // mSessionKey won't be changed since the Ikev2VpnRunner is created, so it's ok to use
+            // it outside the mExecutor. And clearing the VPN network preference here can prevent
+            // the case that the VPN network preference isn't cleared when Ikev2VpnRunner became
+            // stale.
+            clearVpnNetworkPreference(mSessionKey);
             try {
                 mExecutor.execute(() -> {
                     disconnectVpnRunner();
-- 
cgit v1.2.3-59-g8ed1b