Provide more feedback to Settings when sessions fail

This change updates the VPN state when IKEv2 sessions fail, and when
configuration errors occur.

Bug: 162289824
Test: Manual testing with IKEv2/PSK
Change-Id: I2e8c6f421d2898f97b0ac422b2276edf9ef923f1
Merged-In: I2e8c6f421d2898f97b0ac422b2276edf9ef923f1
(cherry picked from commit 0be82d4f4c)
This commit is contained in:
Benedict Wong
2020-09-25 19:19:15 +00:00
parent 608db6dc09
commit 360eba971e
4 changed files with 123 additions and 15 deletions

View File

@@ -77,6 +77,7 @@ import android.net.ipsec.ike.ChildSessionParams;
import android.net.ipsec.ike.IkeSession;
import android.net.ipsec.ike.IkeSessionCallback;
import android.net.ipsec.ike.IkeSessionParams;
import android.net.ipsec.ike.exceptions.IkeProtocolException;
import android.os.Binder;
import android.os.Build.VERSION_CODES;
import android.os.Bundle;
@@ -142,6 +143,7 @@ import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.atomic.AtomicInteger;
/**
@@ -2300,7 +2302,7 @@ public class Vpn {
void onChildTransformCreated(
@NonNull Network network, @NonNull IpSecTransform transform, int direction);
void onSessionLost(@NonNull Network network);
void onSessionLost(@NonNull Network network, @Nullable Exception exception);
}
/**
@@ -2457,7 +2459,7 @@ public class Vpn {
networkAgent.sendLinkProperties(lp);
} catch (Exception e) {
Log.d(TAG, "Error in ChildOpened for network " + network, e);
onSessionLost(network);
onSessionLost(network, e);
}
}
@@ -2487,7 +2489,7 @@ public class Vpn {
mIpSecManager.applyTunnelModeTransform(mTunnelIface, direction, transform);
} catch (IOException e) {
Log.d(TAG, "Transform application failed for network " + network, e);
onSessionLost(network);
onSessionLost(network, e);
}
}
@@ -2545,11 +2547,20 @@ public class Vpn {
Log.d(TAG, "Ike Session started for network " + network);
} catch (Exception e) {
Log.i(TAG, "Setup failed for network " + network + ". Aborting", e);
onSessionLost(network);
onSessionLost(network, e);
}
});
}
/** Marks the state as FAILED, and disconnects. */
private void markFailedAndDisconnect(Exception exception) {
synchronized (Vpn.this) {
updateState(DetailedState.FAILED, exception.getMessage());
}
disconnectVpnRunner();
}
/**
* Handles loss of a session
*
@@ -2559,7 +2570,7 @@ public class Vpn {
* <p>This method MUST always be called on the mExecutor thread in order to ensure
* consistency of the Ikev2VpnRunner fields.
*/
public void onSessionLost(@NonNull Network network) {
public void onSessionLost(@NonNull Network network, @Nullable Exception exception) {
if (!isActiveNetwork(network)) {
Log.d(TAG, "onSessionLost() called for obsolete network " + network);
@@ -2571,6 +2582,27 @@ public class Vpn {
return;
}
if (exception instanceof IkeProtocolException) {
final IkeProtocolException ikeException = (IkeProtocolException) exception;
switch (ikeException.getErrorType()) {
case IkeProtocolException.ERROR_TYPE_NO_PROPOSAL_CHOSEN: // Fallthrough
case IkeProtocolException.ERROR_TYPE_INVALID_KE_PAYLOAD: // Fallthrough
case IkeProtocolException.ERROR_TYPE_AUTHENTICATION_FAILED: // Fallthrough
case IkeProtocolException.ERROR_TYPE_SINGLE_PAIR_REQUIRED: // Fallthrough
case IkeProtocolException.ERROR_TYPE_FAILED_CP_REQUIRED: // Fallthrough
case IkeProtocolException.ERROR_TYPE_TS_UNACCEPTABLE:
// All the above failures are configuration errors, and are terminal
markFailedAndDisconnect(exception);
return;
// All other cases possibly recoverable.
}
} else if (exception instanceof IllegalArgumentException) {
// Failed to build IKE/ChildSessionParams; fatal profile configuration error
markFailedAndDisconnect(exception);
return;
}
mActiveNetwork = null;
// Close all obsolete state, but keep VPN alive incase a usable network comes up.
@@ -2620,12 +2652,18 @@ public class Vpn {
}
/**
* Cleans up all Ikev2VpnRunner internal state
* Disconnects and shuts down this VPN.
*
* <p>This method resets all internal Ikev2VpnRunner state, but unless called via
* VpnRunner#exit(), this Ikev2VpnRunner will still be listed as the active VPN of record
* until the next VPN is started, or the Ikev2VpnRunner is explicitly exited. This is
* necessary to ensure that the detailed state is shown in the Settings VPN menus; if the
* active VPN is cleared, Settings VPNs will not show the resultant state or errors.
*
* <p>This method MUST always be called on the mExecutor thread in order to ensure
* consistency of the Ikev2VpnRunner fields.
*/
private void shutdownVpnRunner() {
private void disconnectVpnRunner() {
mActiveNetwork = null;
mIsRunning = false;
@@ -2639,9 +2677,13 @@ public class Vpn {
@Override
public void exitVpnRunner() {
mExecutor.execute(() -> {
shutdownVpnRunner();
});
try {
mExecutor.execute(() -> {
disconnectVpnRunner();
});
} catch (RejectedExecutionException ignored) {
// The Ikev2VpnRunner has already shut down.
}
}
}

View File

@@ -270,13 +270,13 @@ public class VpnIkev2Utils {
@Override
public void onClosed() {
Log.d(mTag, "IkeClosed for network " + mNetwork);
mCallback.onSessionLost(mNetwork); // Server requested session closure. Retry?
mCallback.onSessionLost(mNetwork, null); // Server requested session closure. Retry?
}
@Override
public void onClosedExceptionally(@NonNull IkeException exception) {
Log.d(mTag, "IkeClosedExceptionally for network " + mNetwork, exception);
mCallback.onSessionLost(mNetwork);
mCallback.onSessionLost(mNetwork, exception);
}
@Override
@@ -306,13 +306,13 @@ public class VpnIkev2Utils {
@Override
public void onClosed() {
Log.d(mTag, "ChildClosed for network " + mNetwork);
mCallback.onSessionLost(mNetwork);
mCallback.onSessionLost(mNetwork, null);
}
@Override
public void onClosedExceptionally(@NonNull IkeException exception) {
Log.d(mTag, "ChildClosedExceptionally for network " + mNetwork, exception);
mCallback.onSessionLost(mNetwork);
mCallback.onSessionLost(mNetwork, exception);
}
@Override
@@ -349,7 +349,7 @@ public class VpnIkev2Utils {
@Override
public void onLost(@NonNull Network network) {
Log.d(mTag, "Tearing down; lost network: " + network);
mCallback.onSessionLost(network);
mCallback.onSessionLost(network, null);
}
}

View File

@@ -63,6 +63,7 @@ android_test {
"services.net",
],
libs: [
"android.net.ipsec.ike.stubs.module_lib",
"android.test.runner",
"android.test.base",
"android.test.mock",

View File

@@ -20,6 +20,7 @@ import static android.content.pm.UserInfo.FLAG_ADMIN;
import static android.content.pm.UserInfo.FLAG_MANAGED_PROFILE;
import static android.content.pm.UserInfo.FLAG_PRIMARY;
import static android.content.pm.UserInfo.FLAG_RESTRICTED;
import static android.net.ConnectivityManager.NetworkCallback;
import static android.net.NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED;
import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET;
import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED;
@@ -45,7 +46,9 @@ import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.timeout;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -66,6 +69,7 @@ import android.net.Ikev2VpnProfile;
import android.net.InetAddresses;
import android.net.IpPrefix;
import android.net.IpSecManager;
import android.net.IpSecTunnelInterfaceResponse;
import android.net.LinkProperties;
import android.net.LocalSocket;
import android.net.Network;
@@ -75,6 +79,8 @@ import android.net.RouteInfo;
import android.net.UidRange;
import android.net.VpnManager;
import android.net.VpnService;
import android.net.ipsec.ike.IkeSessionCallback;
import android.net.ipsec.ike.exceptions.IkeProtocolException;
import android.os.Build.VERSION_CODES;
import android.os.Bundle;
import android.os.ConditionVariable;
@@ -101,6 +107,7 @@ import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Answers;
import org.mockito.ArgumentCaptor;
import org.mockito.InOrder;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
@@ -150,6 +157,11 @@ public class VpnTest {
private static final String TEST_VPN_IDENTITY = "identity";
private static final byte[] TEST_VPN_PSK = "psk".getBytes();
private static final Network TEST_NETWORK = new Network(Integer.MAX_VALUE);
private static final String TEST_IFACE_NAME = "TEST_IFACE";
private static final int TEST_TUNNEL_RESOURCE_ID = 0x2345;
private static final long TEST_TIMEOUT_MS = 500L;
/**
* Names and UIDs for some fake packages. Important points:
* - UID is ordered increasing.
@@ -227,6 +239,13 @@ public class VpnTest {
// Deny all appops by default.
when(mAppOps.noteOpNoThrow(anyInt(), anyInt(), anyString()))
.thenReturn(AppOpsManager.MODE_IGNORED);
// Setup IpSecService
final IpSecTunnelInterfaceResponse tunnelResp =
new IpSecTunnelInterfaceResponse(
IpSecManager.Status.OK, TEST_TUNNEL_RESOURCE_ID, TEST_IFACE_NAME);
when(mIpSecService.createTunnelInterface(any(), any(), any(), any(), any()))
.thenReturn(tunnelResp);
}
@Test
@@ -988,6 +1007,52 @@ public class VpnTest {
eq(AppOpsManager.MODE_IGNORED));
}
private NetworkCallback triggerOnAvailableAndGetCallback() {
final ArgumentCaptor<NetworkCallback> networkCallbackCaptor =
ArgumentCaptor.forClass(NetworkCallback.class);
verify(mConnectivityManager, timeout(TEST_TIMEOUT_MS))
.requestNetwork(any(), networkCallbackCaptor.capture());
final NetworkCallback cb = networkCallbackCaptor.getValue();
cb.onAvailable(TEST_NETWORK);
return cb;
}
@Test
public void testStartPlatformVpnAuthenticationFailed() throws Exception {
final ArgumentCaptor<IkeSessionCallback> captor =
ArgumentCaptor.forClass(IkeSessionCallback.class);
final IkeProtocolException exception = mock(IkeProtocolException.class);
when(exception.getErrorType())
.thenReturn(IkeProtocolException.ERROR_TYPE_AUTHENTICATION_FAILED);
final Vpn vpn = startLegacyVpn(mVpnProfile);
final NetworkCallback cb = triggerOnAvailableAndGetCallback();
// Wait for createIkeSession() to be called before proceeding in order to ensure consistent
// state
verify(mIkev2SessionCreator, timeout(TEST_TIMEOUT_MS))
.createIkeSession(any(), any(), any(), any(), captor.capture(), any());
final IkeSessionCallback ikeCb = captor.getValue();
ikeCb.onClosedExceptionally(exception);
verify(mConnectivityManager, timeout(TEST_TIMEOUT_MS)).unregisterNetworkCallback(eq(cb));
assertEquals(DetailedState.FAILED, vpn.getNetworkInfo().getDetailedState());
}
@Test
public void testStartPlatformVpnIllegalArgumentExceptionInSetup() throws Exception {
when(mIkev2SessionCreator.createIkeSession(any(), any(), any(), any(), any(), any()))
.thenThrow(new IllegalArgumentException());
final Vpn vpn = startLegacyVpn(mVpnProfile);
final NetworkCallback cb = triggerOnAvailableAndGetCallback();
// Wait for createIkeSession() to be called before proceeding in order to ensure consistent
// state
verify(mConnectivityManager, timeout(TEST_TIMEOUT_MS)).unregisterNetworkCallback(eq(cb));
assertEquals(DetailedState.FAILED, vpn.getNetworkInfo().getDetailedState());
}
private void setAndVerifyAlwaysOnPackage(Vpn vpn, int uid, boolean lockdownEnabled) {
assertTrue(vpn.setAlwaysOnPackage(TEST_VPN_PKG, lockdownEnabled, null, mKeyStore));