From 89ca8b306368ddc5afbe0d3cf0dff8b8b3918732 Mon Sep 17 00:00:00 2001 From: chiachangwang Date: Thu, 30 Jun 2022 01:48:45 +0000 Subject: Update the prefix in keystore for app exclusion Update the string without the underscore to prevent conflict with the existing VPN prefix. Settings app may get exception when the vpn exclusion list was set since the logic is used in the Settings relying on the prefix match. The current app exclusion prefix is also started with "VPN_", so it will be mis-used by Settings as a VPN profile. Bug: 237345836 Test: atest FrameworksNetTests Change-Id: I01e773cc15eb1ae5ffaa12aef124bbbf390cc004 --- services/core/java/com/android/server/connectivity/Vpn.java | 2 +- 1 file changed, 1 insertion(+), 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 4e5ce8ae45e4..50b6d77cfd5c 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -186,7 +186,7 @@ public class Vpn { private static final boolean LOGD = true; private static final String ANDROID_KEYSTORE_PROVIDER = "AndroidKeyStore"; /** Key containing prefix of vpn app excluded list */ - @VisibleForTesting static final String VPN_APP_EXCLUDED = "VPN_APP_EXCLUDED_"; + @VisibleForTesting static final String VPN_APP_EXCLUDED = "VPNAPPEXCLUDED_"; // Length of time (in milliseconds) that an app hosting an always-on VPN is placed on // the device idle allowlist during service launch and VPN bootstrap. -- cgit v1.2.3-59-g8ed1b From 03f0d12480b72780e67cce33473e8c55befc2669 Mon Sep 17 00:00:00 2001 From: chiachangwang Date: Wed, 29 Jun 2022 03:08:15 +0000 Subject: Stop VPN profiles by exiting VpnRunner instead of prepareInternal Change to call the VpnRunner.exit to stop vpn profile instead of using prepareInternal to leave the package intact. This aligns with the way that VpnServices are disconnected so that the package and other related information will not be reset unexpectedly. In current design, Vpn will examine if the current prepared package matches to prevent an existing always-on VPN from being dethroned by other apps when the VPN always-on is enabled. However, when the VPN is disabled using stopVpnProfile, the current package name will be updated to [LEGACY VPN]. The current package will no longer be the same with the VPN package. This resulted in the rejection of the VPN app to start the VPN using VpnManager.startVpnProfile again. Test: atest FrameworksNetTests Test: manually test with VPN app to reconnect VPN when always-on is enabled. Bug: 235322391 Change-Id: I83e1e1edf6c3a6653d87216afcd397f296f59cf2 --- .../java/com/android/server/connectivity/Vpn.java | 35 +++++++++++++--------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index 50b6d77cfd5c..c3fc8e0a7535 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -1182,20 +1182,9 @@ public class Vpn { cleanupVpnStateLocked(); } else if (mVpnRunner != null) { if (!VpnConfig.LEGACY_VPN.equals(mPackage)) { - mAppOpsManager.finishOp( - AppOpsManager.OPSTR_ESTABLISH_VPN_MANAGER, mOwnerUID, mPackage, null); - // The underlying network, NetworkCapabilities and LinkProperties are not - // necessary to send to VPN app since the purpose of this event is to notify - // VPN app that VPN is deactivated by the user. - // TODO(b/230548427): Remove SDK check once VPN related stuff are decoupled from - // ConnectivityServiceTest. - if (SdkLevel.isAtLeastT()) { - sendEventToVpnManagerApp(VpnManager.CATEGORY_EVENT_DEACTIVATED_BY_USER, - -1 /* errorClass */, -1 /* errorCode*/, mPackage, - getSessionKeyLocked(), makeVpnProfileStateLocked(), - null /* underlyingNetwork */, null /* nc */, null /* lp */); - } + notifyVpnManagerVpnStopped(mPackage, mOwnerUID); } + // cleanupVpnStateLocked() is called from mVpnRunner.exit() mVpnRunner.exit(); } @@ -4043,7 +4032,25 @@ public class Vpn { // To stop the VPN profile, the caller must be the current prepared package and must be // running an Ikev2VpnProfile. if (isCurrentIkev2VpnLocked(packageName)) { - prepareInternal(VpnConfig.LEGACY_VPN); + notifyVpnManagerVpnStopped(packageName, mOwnerUID); + + mVpnRunner.exit(); + } + } + + private synchronized void notifyVpnManagerVpnStopped(String packageName, int ownerUID) { + mAppOpsManager.finishOp( + AppOpsManager.OPSTR_ESTABLISH_VPN_MANAGER, ownerUID, packageName, null); + // The underlying network, NetworkCapabilities and LinkProperties are not + // necessary to send to VPN app since the purpose of this event is to notify + // VPN app that VPN is deactivated by the user. + // TODO(b/230548427): Remove SDK check once VPN related stuff are decoupled from + // ConnectivityServiceTest. + if (SdkLevel.isAtLeastT()) { + sendEventToVpnManagerApp(VpnManager.CATEGORY_EVENT_DEACTIVATED_BY_USER, + -1 /* errorClass */, -1 /* errorCode*/, packageName, + getSessionKeyLocked(), makeVpnProfileStateLocked(), + null /* underlyingNetwork */, null /* nc */, null /* lp */); } } -- cgit v1.2.3-59-g8ed1b From df28c2b1612c4851a3a2c51271d10072867f3bd4 Mon Sep 17 00:00:00 2001 From: chiachangwang Date: Tue, 5 Jul 2022 09:20:13 +0000 Subject: Update method visibility for testing Remove VisibleForTesting for onUserStarted, onPackageRemoved and onPackageAdded because it's not necessary now. Bug: 230548427 Test: m ; atest FrameworksNetTests Change-Id: I531feb457bc59dc9969d93311654523f3e6f5270 --- services/core/java/com/android/server/VpnManagerService.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/VpnManagerService.java b/services/core/java/com/android/server/VpnManagerService.java index 07b6843b2feb..c9a420eabbd8 100644 --- a/services/core/java/com/android/server/VpnManagerService.java +++ b/services/core/java/com/android/server/VpnManagerService.java @@ -769,8 +769,7 @@ public class VpnManagerService extends IVpnManager.Stub { } }; - @VisibleForTesting - void onUserStarted(int userId) { + private void onUserStarted(int userId) { synchronized (mVpns) { Vpn userVpn = mVpns.get(userId); if (userVpn != null) { @@ -854,8 +853,7 @@ public class VpnManagerService extends IVpnManager.Stub { } } - @VisibleForTesting - void onPackageRemoved(String packageName, int uid, boolean isReplacing) { + private void onPackageRemoved(String packageName, int uid, boolean isReplacing) { if (TextUtils.isEmpty(packageName) || uid < 0) { Log.wtf(TAG, "Invalid package in onPackageRemoved: " + packageName + " | " + uid); return; @@ -878,8 +876,7 @@ public class VpnManagerService extends IVpnManager.Stub { } } - @VisibleForTesting - void onPackageAdded(String packageName, int uid, boolean isReplacing) { + private void onPackageAdded(String packageName, int uid, boolean isReplacing) { if (TextUtils.isEmpty(packageName) || uid < 0) { Log.wtf(TAG, "Invalid package in onPackageAdded: " + packageName + " | " + uid); return; -- cgit v1.2.3-59-g8ed1b