Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9df1022c authored by Benedict Wong's avatar Benedict Wong Committed by Gerrit Code Review
Browse files

Merge "Provide more feedback to Settings when sessions fail"

parents 0c47ab06 1d0275d9
Loading
Loading
Loading
Loading
+52 −10
Original line number Diff line number Diff line
@@ -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;

/**
@@ -2301,7 +2303,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);
    }

    /**
@@ -2458,7 +2460,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);
            }
        }

@@ -2488,7 +2490,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);
            }
        }

@@ -2546,11 +2548,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
         *
@@ -2560,7 +2571,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);

@@ -2572,6 +2583,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.
@@ -2621,12 +2653,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;

@@ -2640,9 +2678,13 @@ public class Vpn {

        @Override
        public void exitVpnRunner() {
            try {
                mExecutor.execute(() -> {
                shutdownVpnRunner();
                    disconnectVpnRunner();
                });
            } catch (RejectedExecutionException ignored) {
                // The Ikev2VpnRunner has already shut down.
            }
        }
    }

+5 −5
Original line number Diff line number Diff line
@@ -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);
        }
    }

+1 −0
Original line number Diff line number Diff line
@@ -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",
+65 −0
Original line number Diff line number Diff line
@@ -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));