From 1cb9f6181b9e4d370eb92b00c1aa545100879f4d Mon Sep 17 00:00:00 2001 From: lucaslin Date: Thu, 21 Apr 2022 14:28:57 +0800 Subject: [PATCH 1/3] Move mSessionKey into Ikev2VpnRunner and make it to be a final var mSessionKey is initialized in the constructor of Ikev2VpnRunner, move it into Ikev2VpnRunner and make it to be a final var can make sure that mSessionKey is initialized before someone access it. Test: atest FrameworksNetTests:VpnTest Change-Id: If7eb7e0ec8ab42b17a5636aeced65385e7849b6a --- .../com/android/server/connectivity/Vpn.java | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index c53a9a63be65b..d9db28a9aa781 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -209,7 +209,6 @@ public class Vpn { private final NetworkInfo mNetworkInfo; private int mLegacyState; @VisibleForTesting protected String mPackage; - private String mSessionKey; private int mOwnerUID; private boolean mIsPackageTargetingAtLeastQ; @VisibleForTesting @@ -1991,9 +1990,7 @@ public class Vpn { public synchronized int getActiveVpnType() { if (!mNetworkInfo.isConnectedOrConnecting()) return VpnManager.TYPE_VPN_NONE; if (mVpnRunner == null) return VpnManager.TYPE_VPN_SERVICE; - return mVpnRunner instanceof IkeV2VpnRunner - ? VpnManager.TYPE_VPN_PLATFORM - : VpnManager.TYPE_VPN_LEGACY; + return isIkev2VpnRunner() ? VpnManager.TYPE_VPN_PLATFORM : VpnManager.TYPE_VPN_LEGACY; } private void updateAlwaysOnNotification(DetailedState networkState) { @@ -2524,6 +2521,7 @@ public class Vpn { @Nullable private IpSecTunnelInterface mTunnelIface; @Nullable private IkeSession mSession; @Nullable private Network mActiveNetwork; + private final String mSessionKey; IkeV2VpnRunner(@NonNull Ikev2VpnProfile profile) { super(TAG); @@ -2869,7 +2867,6 @@ public class Vpn { */ private void disconnectVpnRunner() { mActiveNetwork = null; - mSessionKey = null; mIsRunning = false; resetIkeState(); @@ -3299,7 +3296,7 @@ public class Vpn { } private boolean isCurrentIkev2VpnLocked(@NonNull String packageName) { - return isCurrentPreparedPackage(packageName) && mVpnRunner instanceof IkeV2VpnRunner; + return isCurrentPreparedPackage(packageName) && isIkev2VpnRunner(); } /** @@ -3353,6 +3350,16 @@ public class Vpn { return VpnProfile.decode("" /* Key unused */, encoded); } + private boolean isIkev2VpnRunner() { + return (mVpnRunner instanceof IkeV2VpnRunner); + } + + @GuardedBy("this") + @Nullable + private String getSessionKeyLocked() { + return isIkev2VpnRunner() ? ((IkeV2VpnRunner) mVpnRunner).mSessionKey : null; + } + /** * Starts an already provisioned VPN Profile, keyed by package name. * @@ -3380,7 +3387,11 @@ public class Vpn { } startVpnProfilePrivileged(profile, packageName); - return mSessionKey; + if (!isIkev2VpnRunner()) { + throw new IllegalStateException("mVpnRunner shouldn't be null and should also be " + + "an instance of Ikev2VpnRunner"); + } + return getSessionKeyLocked(); } finally { Binder.restoreCallingIdentity(token); } @@ -3483,11 +3494,8 @@ public class Vpn { } private VpnProfileState makeVpnProfileState() { - // TODO: mSessionKey will be moved to Ikev2VpnRunner once aosp/2007077 is merged, so after - // merging aosp/2007077, here should check Ikev2VpnRunner is null or not. Session key will - // be null if Ikev2VpnRunner is null. - return new VpnProfileState(getStateFromLegacyState(mLegacyState), mSessionKey, mAlwaysOn, - mLockdown); + return new VpnProfileState(getStateFromLegacyState(mLegacyState), + isIkev2VpnRunner() ? getSessionKeyLocked() : null, mAlwaysOn, mLockdown); } /** From 6a75e5a3c53fbf0ef8179c160505b0670137eef4 Mon Sep 17 00:00:00 2001 From: lucaslin Date: Mon, 18 Apr 2022 13:51:55 +0800 Subject: [PATCH 2/3] Add toString() for VpnProfileState Sample log: {State: DISCONNECTED, SessionId: null, Always-on: false, Lockdown: false} {State: CONNECTED, SessionId: d7913298-c07f-49de-b84e-304b5e26da89, Always-on: true, Lockdown: false} Bug: 225010642 Test: Add a log in Vpn.java and print it. Change-Id: Ia36d794446114c0c2c27e725cba6a4dc1240ae38 --- core/java/android/net/VpnProfileState.java | 26 ++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/core/java/android/net/VpnProfileState.java b/core/java/android/net/VpnProfileState.java index c69ea1a8c2200..0f21a9d7f4713 100644 --- a/core/java/android/net/VpnProfileState.java +++ b/core/java/android/net/VpnProfileState.java @@ -24,6 +24,7 @@ import android.os.Parcelable; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.util.StringJoiner; /** * Describe the state of VPN. @@ -150,4 +151,29 @@ public final class VpnProfileState implements Parcelable { mAlwaysOn = in.readBoolean(); mLockdown = in.readBoolean(); } + + private String convertStateToString(@State int state) { + switch (state) { + case STATE_CONNECTED: + return "CONNECTED"; + case STATE_CONNECTING: + return "CONNECTING"; + case STATE_DISCONNECTED: + return "DISCONNECTED"; + case STATE_FAILED: + return "FAILED"; + default: + return "UNKNOWN"; + } + } + + @Override + public String toString() { + final StringJoiner resultJoiner = new StringJoiner(", ", "{", "}"); + resultJoiner.add("State: " + convertStateToString(getState())); + resultJoiner.add("SessionId: " + getSessionId()); + resultJoiner.add("Always-on: " + isAlwaysOn()); + resultJoiner.add("Lockdown: " + isLockdownEnabled()); + return resultJoiner.toString(); + } } From 4b6b7987a1d3c5305147144db0848048e425fcf3 Mon Sep 17 00:00:00 2001 From: lucaslin Date: Mon, 18 Apr 2022 13:53:29 +0800 Subject: [PATCH 3/3] Modify javadoc for some extra values of VpnManager event The underlying network, NetworkCapabilities and LinkProperties are not necessary to send to VPN app since the purpose of CATEGORY_EVENT_DEACTIVATED_BY_USER and CATEGORY_EVENT_ALWAYS_ON_STATE_CHANGED are to notify VPN app that VPN is deactivated by the user or VPN always-on status is changed. Bug: 191413541 Test: Javadoc only changes. Change-Id: I7fd4b6dad1a4363336f03c217e635fc45bcd0ab6 --- core/java/android/net/VpnManager.java | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/core/java/android/net/VpnManager.java b/core/java/android/net/VpnManager.java index ae7d91f92cb74..37eb74a58235e 100644 --- a/core/java/android/net/VpnManager.java +++ b/core/java/android/net/VpnManager.java @@ -187,14 +187,24 @@ public class VpnManager { /** * The network that was underlying the VPN when the event occurred, as a {@link Network}. * - * This extra will be null if there was no underlying network at the time of the event. + *

This extra will be null if there was no underlying network at the time of the event, or + * the underlying network has no bearing on the event, as in the case of: + *

    + *
  • CATEGORY_EVENT_DEACTIVATED_BY_USER + *
  • CATEGORY_EVENT_ALWAYS_ON_STATE_CHANGED + *
*/ public static final String EXTRA_UNDERLYING_NETWORK = "android.net.extra.UNDERLYING_NETWORK"; /** * The {@link NetworkCapabilities} of the underlying network when the event occurred. * - * This extra will be null if there was no underlying network at the time of the event. + *

This extra will be null if there was no underlying network at the time of the event, or + * the underlying network has no bearing on the event, as in the case of: + *

    + *
  • CATEGORY_EVENT_DEACTIVATED_BY_USER + *
  • CATEGORY_EVENT_ALWAYS_ON_STATE_CHANGED + *
*/ public static final String EXTRA_UNDERLYING_NETWORK_CAPABILITIES = "android.net.extra.UNDERLYING_NETWORK_CAPABILITIES"; @@ -202,7 +212,12 @@ public class VpnManager { /** * The {@link LinkProperties} of the underlying network when the event occurred. * - * This extra will be null if there was no underlying network at the time of the event. + *

This extra will be null if there was no underlying network at the time of the event, or + * the underlying network has no bearing on the event, as in the case of: + *

    + *
  • CATEGORY_EVENT_DEACTIVATED_BY_USER + *
  • CATEGORY_EVENT_ALWAYS_ON_STATE_CHANGED + *
*/ public static final String EXTRA_UNDERLYING_LINK_PROPERTIES = "android.net.extra.UNDERLYING_LINK_PROPERTIES";