Merge "Implement ConnectingState" am: 026b277911 am: 51bdc6991a

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1550524

MUST ONLY BE SUBMITTED BY AUTOMERGER

Change-Id: I0413f3171875d53478bd65cb70d8905721fc4c13

services/core/java/com/android/server/vcn/VcnGatewayConnection.java

@@ -638,6 +638,22 @@ public class VcnGatewayConnection extends StateMachine {
 
         protected abstract void processStateMsg(Message msg) throws Exception;
 
+        @Override
+        public void exit() {
+            try {
+                exitState();
+            } catch (Exception e) {
+                Slog.wtf(TAG, "Uncaught exception", e);
+                sendMessage(
+                        EVENT_DISCONNECT_REQUESTED,
+                        TOKEN_ALL,
+                        new EventDisconnectRequestedInfo(
+                                DISCONNECT_REASON_INTERNAL_ERROR + e.toString()));
+            }
+        }
+
+        protected void exitState() throws Exception {}
+
         protected void logUnhandledMessage(Message msg) {
             // Log as unexpected all known messages, and log all else as unknown.
             switch (msg.what) {
@@ -664,18 +680,11 @@ public class VcnGatewayConnection extends StateMachine {
             }
         }
 
-        protected void teardownIke() {
-            if (mIkeSession != null) {
-                mIkeSession.close();
-            }
-        }
-
         protected void handleDisconnectRequested(String msg) {
             Slog.v(TAG, "Tearing down. Cause: " + msg);
             mIsRunning = false;
 
             teardownNetwork();
-            teardownIke();
 
             if (mIkeSession == null) {
                 // Already disconnected, go straight to DisconnectedState
@@ -768,6 +777,20 @@ public class VcnGatewayConnection extends StateMachine {
      * does not complete teardown in a timely fashion, it will be killed (forcibly closed).
      */
     private class DisconnectingState extends ActiveBaseState {
+        /**
+         * Whether to skip the RetryTimeoutState and go straight to the ConnectingState.
+         *
+         * <p>This is used when an underlying network change triggered a restart on a new network.
+         *
+         * <p>Reset (to false) upon exit of the DisconnectingState.
+         */
+        private boolean mSkipRetryTimeout = false;
+
+        // TODO(b/178441390): Remove this in favor of resetting retry timers on UND_NET change.
+        public void setSkipRetryTimeout(boolean shouldSkip) {
+            mSkipRetryTimeout = shouldSkip;
+        }
+
         @Override
         protected void enterState() throws Exception {
             if (mIkeSession == null) {
@@ -783,6 +806,7 @@ public class VcnGatewayConnection extends StateMachine {
                 return;
             }
 
+            mIkeSession.close();
             sendMessageDelayed(
                     EVENT_TEARDOWN_TIMEOUT_EXPIRED,
                     mCurrentToken,
@@ -822,7 +846,7 @@ public class VcnGatewayConnection extends StateMachine {
                     mIkeSession = null;
 
                     if (mIsRunning && mUnderlying != null) {
-                        transitionTo(mRetryTimeoutState);
+                        transitionTo(mSkipRetryTimeout ? mConnectingState : mRetryTimeoutState);
                     } else {
                         teardownNetwork();
                         transitionTo(mDisconnectedState);
@@ -833,6 +857,11 @@ public class VcnGatewayConnection extends StateMachine {
                     break;
             }
         }
+
+        @Override
+        protected void exitState() throws Exception {
+            mSkipRetryTimeout = false;
+        }
     }
 
     /**
@@ -843,7 +872,69 @@ public class VcnGatewayConnection extends StateMachine {
      */
     private class ConnectingState extends ActiveBaseState {
         @Override
-        protected void processStateMsg(Message msg) {}
+        protected void enterState() {
+            if (mIkeSession != null) {
+                Slog.wtf(TAG, "ConnectingState entered with active session");
+
+                // Attempt to recover.
+                mIkeSession.kill();
+                mIkeSession = null;
+            }
+
+            mIkeSession = buildIkeSession();
+        }
+
+        @Override
+        protected void processStateMsg(Message msg) {
+            switch (msg.what) {
+                case EVENT_UNDERLYING_NETWORK_CHANGED:
+                    final UnderlyingNetworkRecord oldUnderlying = mUnderlying;
+                    mUnderlying = ((EventUnderlyingNetworkChangedInfo) msg.obj).newUnderlying;
+
+                    if (oldUnderlying == null) {
+                        // This should never happen, but if it does, there's likely a nasty bug.
+                        Slog.wtf(TAG, "Old underlying network was null in connected state. Bug?");
+                    }
+
+                    // If new underlying is null, all underlying networks have been lost; disconnect
+                    if (mUnderlying == null) {
+                        transitionTo(mDisconnectingState);
+                        break;
+                    }
+
+                    if (oldUnderlying != null
+                            && mUnderlying.network.equals(oldUnderlying.network)) {
+                        break; // Only network properties have changed; continue connecting.
+                    }
+                    // Else, retry on the new network.
+
+                    // Immediately come back to the ConnectingState (skip RetryTimeout, since this
+                    // isn't a failure)
+                    mDisconnectingState.setSkipRetryTimeout(true);
+
+                    // fallthrough - disconnect, and retry on new network.
+                case EVENT_SESSION_LOST:
+                    transitionTo(mDisconnectingState);
+                    break;
+                case EVENT_SESSION_CLOSED:
+                    deferMessage(msg);
+
+                    transitionTo(mDisconnectingState);
+                    break;
+                case EVENT_SETUP_COMPLETED: // fallthrough
+                case EVENT_TRANSFORM_CREATED:
+                    // Child setup complete; move to ConnectedState for NetworkAgent registration
+                    deferMessage(msg);
+                    transitionTo(mConnectedState);
+                    break;
+                case EVENT_DISCONNECT_REQUESTED:
+                    handleDisconnectRequested(((EventDisconnectRequestedInfo) msg.obj).reason);
+                    break;
+                default:
+                    logUnhandledMessage(msg);
+                    break;
+            }
+        }
     }
 
     private abstract class ConnectedStateBase extends ActiveBaseState {}
@@ -1015,12 +1106,12 @@ public class VcnGatewayConnection extends StateMachine {
     }
 
     private IkeSessionParams buildIkeParams() {
-        // TODO: Implement this with ConnectingState
+        // TODO: Implement this once IkeSessionParams is persisted
         return null;
     }
 
     private ChildSessionParams buildChildParams() {
-        // TODO: Implement this with ConnectingState
+        // TODO: Implement this once IkeSessionParams is persisted
         return null;
     }
 

tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectingStateTest.java

@@ -0,0 +1,105 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.vcn;
+
+import static com.android.server.vcn.VcnGatewayConnection.VcnIkeSession;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+import androidx.test.filters.SmallTest;
+import androidx.test.runner.AndroidJUnit4;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/** Tests for VcnGatewayConnection.ConnectingState */
+@RunWith(AndroidJUnit4.class)
+@SmallTest
+public class VcnGatewayConnectionConnectingStateTest extends VcnGatewayConnectionTestBase {
+    private VcnIkeSession mIkeSession;
+
+    @Before
+    public void setUp() throws Exception {
+        super.setUp();
+
+        mGatewayConnection.setUnderlyingNetwork(TEST_UNDERLYING_NETWORK_RECORD_1);
+        mGatewayConnection.transitionTo(mGatewayConnection.mConnectingState);
+        mTestLooper.dispatchAll();
+
+        mIkeSession = mGatewayConnection.getIkeSession();
+    }
+
+    @Test
+    public void testEnterStateCreatesNewIkeSession() throws Exception {
+        verify(mDeps).newIkeSession(any(), any(), any(), any(), any());
+    }
+
+    @Test
+    public void testNullNetworkTriggersDisconnect() throws Exception {
+        mGatewayConnection
+                .getUnderlyingNetworkTrackerCallback()
+                .onSelectedUnderlyingNetworkChanged(null);
+        mTestLooper.dispatchAll();
+
+        assertEquals(mGatewayConnection.mDisconnectingState, mGatewayConnection.getCurrentState());
+        verify(mIkeSession).kill();
+    }
+
+    @Test
+    public void testNewNetworkTriggersReconnect() throws Exception {
+        mGatewayConnection
+                .getUnderlyingNetworkTrackerCallback()
+                .onSelectedUnderlyingNetworkChanged(TEST_UNDERLYING_NETWORK_RECORD_2);
+        mTestLooper.dispatchAll();
+
+        assertEquals(mGatewayConnection.mDisconnectingState, mGatewayConnection.getCurrentState());
+        verify(mIkeSession).close();
+        verify(mIkeSession, never()).kill();
+    }
+
+    @Test
+    public void testSameNetworkDoesNotTriggerReconnect() throws Exception {
+        mGatewayConnection
+                .getUnderlyingNetworkTrackerCallback()
+                .onSelectedUnderlyingNetworkChanged(TEST_UNDERLYING_NETWORK_RECORD_1);
+        mTestLooper.dispatchAll();
+
+        assertEquals(mGatewayConnection.mConnectingState, mGatewayConnection.getCurrentState());
+    }
+
+    @Test
+    public void testChildSessionClosedTriggersDisconnect() throws Exception {
+        getChildSessionCallback().onClosed();
+        mTestLooper.dispatchAll();
+
+        assertEquals(mGatewayConnection.mDisconnectingState, mGatewayConnection.getCurrentState());
+        verify(mIkeSession).close();
+    }
+
+    @Test
+    public void testIkeSessionClosedTriggersDisconnect() throws Exception {
+        getIkeSessionCallback().onClosed();
+        mTestLooper.dispatchAll();
+
+        assertEquals(mGatewayConnection.mRetryTimeoutState, mGatewayConnection.getCurrentState());
+        verify(mIkeSession).close();
+    }
+}

tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java

@@ -32,6 +32,7 @@ import android.net.IpSecTunnelInterfaceResponse;
 import android.net.LinkProperties;
 import android.net.Network;
 import android.net.NetworkCapabilities;
+import android.net.ipsec.ike.ChildSessionCallback;
 import android.net.ipsec.ike.IkeSessionCallback;
 import android.net.vcn.VcnGatewayConnectionConfig;
 import android.net.vcn.VcnGatewayConnectionConfigTest;
@@ -117,4 +118,11 @@ public class VcnGatewayConnectionTestBase {
         verify(mDeps).newIkeSession(any(), any(), any(), captor.capture(), any());
         return captor.getValue();
     }
+
+    protected ChildSessionCallback getChildSessionCallback() {
+        ArgumentCaptor<ChildSessionCallback> captor =
+                ArgumentCaptor.forClass(ChildSessionCallback.class);
+        verify(mDeps).newIkeSession(any(), any(), any(), any(), captor.capture());
+        return captor.getValue();
+    }
 }