diff options
4 files changed, 123 insertions, 15 deletions
diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index 1c93d4eb599b..347beab0c42f 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -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. + } } } diff --git a/services/core/java/com/android/server/connectivity/VpnIkev2Utils.java b/services/core/java/com/android/server/connectivity/VpnIkev2Utils.java index 103f659cc258..626303001ba0 100644 --- a/services/core/java/com/android/server/connectivity/VpnIkev2Utils.java +++ b/services/core/java/com/android/server/connectivity/VpnIkev2Utils.java @@ -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); } } diff --git a/tests/net/Android.bp b/tests/net/Android.bp index 124b6609f687..0fe84abcbc7b 100644 --- a/tests/net/Android.bp +++ b/tests/net/Android.bp @@ -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", diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index de1c5759ee87..91ffa8e9d0d8 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -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)); |