Tidy up VPN code
This commit addressed the comments in different commits, it did
- Make isVpnApp() to be a static method.
- Rename getRedactedNetworkCapabilitiesOfUnderlyingNetwork to
getRedactedNetworkCapabilities and rename
getRedactedLinkPropertiesOfUnderlyingNetwork to
getRedactedLinkProperties.
- Reduce the scope of synchronized block in handleSessionLost().
- Fast return when there is an IllegalArgumentException in
handleSessionLost().
- Remove inaccurate comment.
- Update the document of mCurrentToken.
- Define an initial token value of IKE session.
- Add "/* exeception */" annotation to handleSessionLost(null)
calls.
Bug: 236409954
Test: atest FrameworksNetTests:VpnTest
Change-Id: I8c938f8796345dc2eee6c8eff513b0f993bc8165
(cherry picked from commit 7146e2cb02)
Merged-In: I8c938f8796345dc2eee6c8eff513b0f993bc8165
This commit is contained in:
@@ -222,6 +222,11 @@ public class Vpn {
|
|||||||
*/
|
*/
|
||||||
private static final int VPN_DEFAULT_SCORE = 101;
|
private static final int VPN_DEFAULT_SCORE = 101;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The initial token value of IKE session.
|
||||||
|
*/
|
||||||
|
private static final int STARTING_TOKEN = -1;
|
||||||
|
|
||||||
// TODO: create separate trackers for each unique VPN to support
|
// TODO: create separate trackers for each unique VPN to support
|
||||||
// automated reconnection
|
// automated reconnection
|
||||||
|
|
||||||
@@ -795,7 +800,7 @@ public class Vpn {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private boolean isVpnApp(String packageName) {
|
private static boolean isVpnApp(String packageName) {
|
||||||
return packageName != null && !VpnConfig.LEGACY_VPN.equals(packageName);
|
return packageName != null && !VpnConfig.LEGACY_VPN.equals(packageName);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2593,7 +2598,7 @@ public class Vpn {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Nullable
|
@Nullable
|
||||||
protected synchronized NetworkCapabilities getRedactedNetworkCapabilitiesOfUnderlyingNetwork(
|
private synchronized NetworkCapabilities getRedactedNetworkCapabilities(
|
||||||
NetworkCapabilities nc) {
|
NetworkCapabilities nc) {
|
||||||
if (nc == null) return null;
|
if (nc == null) return null;
|
||||||
return mConnectivityManager.getRedactedNetworkCapabilitiesForPackage(
|
return mConnectivityManager.getRedactedNetworkCapabilitiesForPackage(
|
||||||
@@ -2601,8 +2606,7 @@ public class Vpn {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Nullable
|
@Nullable
|
||||||
protected synchronized LinkProperties getRedactedLinkPropertiesOfUnderlyingNetwork(
|
private synchronized LinkProperties getRedactedLinkProperties(LinkProperties lp) {
|
||||||
LinkProperties lp) {
|
|
||||||
if (lp == null) return null;
|
if (lp == null) return null;
|
||||||
return mConnectivityManager.getRedactedLinkPropertiesForPackage(lp, mOwnerUID, mPackage);
|
return mConnectivityManager.getRedactedLinkPropertiesForPackage(lp, mOwnerUID, mPackage);
|
||||||
}
|
}
|
||||||
@@ -2716,11 +2720,13 @@ public class Vpn {
|
|||||||
private boolean mIsRunning = true;
|
private boolean mIsRunning = true;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The token used by the primary/current/active IKE session.
|
* The token that identifies the most recently created IKE session.
|
||||||
*
|
*
|
||||||
* <p>This token MUST be updated when the VPN switches to use a new IKE session.
|
* <p>This token is monotonically increasing and will never be reset in the lifetime of this
|
||||||
|
* Ikev2VpnRunner, but it does get reset across runs. It also MUST be accessed on the
|
||||||
|
* executor thread and updated when a new IKE session is created.
|
||||||
*/
|
*/
|
||||||
private int mCurrentToken = -1;
|
private int mCurrentToken = STARTING_TOKEN;
|
||||||
|
|
||||||
@Nullable private IpSecTunnelInterface mTunnelIface;
|
@Nullable private IpSecTunnelInterface mTunnelIface;
|
||||||
@Nullable private Network mActiveNetwork;
|
@Nullable private Network mActiveNetwork;
|
||||||
@@ -3223,7 +3229,7 @@ public class Vpn {
|
|||||||
mExecutor.schedule(
|
mExecutor.schedule(
|
||||||
() -> {
|
() -> {
|
||||||
if (isActiveToken(token)) {
|
if (isActiveToken(token)) {
|
||||||
handleSessionLost(null, network);
|
handleSessionLost(null /* exception */, network);
|
||||||
} else {
|
} else {
|
||||||
Log.d(
|
Log.d(
|
||||||
TAG,
|
TAG,
|
||||||
@@ -3240,7 +3246,7 @@ public class Vpn {
|
|||||||
TimeUnit.MILLISECONDS);
|
TimeUnit.MILLISECONDS);
|
||||||
} else {
|
} else {
|
||||||
Log.d(TAG, "Call handleSessionLost for losing network " + network);
|
Log.d(TAG, "Call handleSessionLost for losing network " + network);
|
||||||
handleSessionLost(null, network);
|
handleSessionLost(null /* exception */, network);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -3311,13 +3317,15 @@ public class Vpn {
|
|||||||
// already terminated due to other failures.
|
// already terminated due to other failures.
|
||||||
cancelHandleNetworkLostTimeout();
|
cancelHandleNetworkLostTimeout();
|
||||||
|
|
||||||
synchronized (Vpn.this) {
|
|
||||||
// Ignore stale runner.
|
|
||||||
if (mVpnRunner != this) return;
|
|
||||||
|
|
||||||
String category = null;
|
String category = null;
|
||||||
int errorClass = -1;
|
int errorClass = -1;
|
||||||
int errorCode = -1;
|
int errorCode = -1;
|
||||||
|
if (exception instanceof IllegalArgumentException) {
|
||||||
|
// Failed to build IKE/ChildSessionParams; fatal profile configuration error
|
||||||
|
markFailedAndDisconnect(exception);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
if (exception instanceof IkeProtocolException) {
|
if (exception instanceof IkeProtocolException) {
|
||||||
final IkeProtocolException ikeException = (IkeProtocolException) exception;
|
final IkeProtocolException ikeException = (IkeProtocolException) exception;
|
||||||
category = VpnManager.CATEGORY_EVENT_IKE_ERROR;
|
category = VpnManager.CATEGORY_EVENT_IKE_ERROR;
|
||||||
@@ -3335,13 +3343,8 @@ public class Vpn {
|
|||||||
break;
|
break;
|
||||||
// All other cases possibly recoverable.
|
// All other cases possibly recoverable.
|
||||||
default:
|
default:
|
||||||
// All the above failures are configuration errors, and are terminal
|
|
||||||
errorClass = VpnManager.ERROR_CLASS_RECOVERABLE;
|
errorClass = VpnManager.ERROR_CLASS_RECOVERABLE;
|
||||||
}
|
}
|
||||||
} else if (exception instanceof IllegalArgumentException) {
|
|
||||||
// Failed to build IKE/ChildSessionParams; fatal profile configuration error
|
|
||||||
markFailedAndDisconnect(exception);
|
|
||||||
return;
|
|
||||||
} else if (exception instanceof IkeNetworkLostException) {
|
} else if (exception instanceof IkeNetworkLostException) {
|
||||||
category = VpnManager.CATEGORY_EVENT_NETWORK_ERROR;
|
category = VpnManager.CATEGORY_EVENT_NETWORK_ERROR;
|
||||||
errorClass = VpnManager.ERROR_CLASS_RECOVERABLE;
|
errorClass = VpnManager.ERROR_CLASS_RECOVERABLE;
|
||||||
@@ -3360,16 +3363,19 @@ public class Vpn {
|
|||||||
Log.wtf(TAG, "onSessionLost: exception = " + exception);
|
Log.wtf(TAG, "onSessionLost: exception = " + exception);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
synchronized (Vpn.this) {
|
||||||
|
// Ignore stale runner.
|
||||||
|
if (mVpnRunner != this) return;
|
||||||
|
|
||||||
// TODO(b/230548427): Remove SDK check once VPN related stuff are
|
// TODO(b/230548427): Remove SDK check once VPN related stuff are
|
||||||
// decoupled from ConnectivityServiceTest.
|
// decoupled from ConnectivityServiceTest.
|
||||||
if (SdkLevel.isAtLeastT() && category != null && isVpnApp(mPackage)) {
|
if (SdkLevel.isAtLeastT() && category != null && isVpnApp(mPackage)) {
|
||||||
sendEventToVpnManagerApp(category, errorClass, errorCode,
|
sendEventToVpnManagerApp(category, errorClass, errorCode,
|
||||||
getPackage(), mSessionKey, makeVpnProfileStateLocked(),
|
getPackage(), mSessionKey, makeVpnProfileStateLocked(),
|
||||||
mActiveNetwork,
|
mActiveNetwork,
|
||||||
getRedactedNetworkCapabilitiesOfUnderlyingNetwork(
|
getRedactedNetworkCapabilities(mUnderlyingNetworkCapabilities),
|
||||||
mUnderlyingNetworkCapabilities),
|
getRedactedLinkProperties(mUnderlyingLinkProperties));
|
||||||
getRedactedLinkPropertiesOfUnderlyingNetwork(
|
}
|
||||||
mUnderlyingLinkProperties));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (errorClass == VpnManager.ERROR_CLASS_NOT_RECOVERABLE) {
|
if (errorClass == VpnManager.ERROR_CLASS_NOT_RECOVERABLE) {
|
||||||
@@ -3378,7 +3384,6 @@ public class Vpn {
|
|||||||
} else {
|
} else {
|
||||||
scheduleRetryNewIkeSession();
|
scheduleRetryNewIkeSession();
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
mUnderlyingNetworkCapabilities = null;
|
mUnderlyingNetworkCapabilities = null;
|
||||||
mUnderlyingLinkProperties = null;
|
mUnderlyingLinkProperties = null;
|
||||||
|
|||||||
Reference in New Issue
Block a user