From 80056e834ea0811c0ba1e90063d8c561b8eca6dd Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Tue, 23 Mar 2021 01:26:17 -0700 Subject: [PATCH] Allow soft-start and opportunistic safe mode This change adds support for opportunistic safe mode, where the VCN will continue to provide networks, but not restrict underlying networks. Similarly, this change allows for soft-starting of the VCN, where VCN underlying networks can be selected without restricting them directly. Additionally, this change ensures networks are torn down when VcnGatewayConnections enter safe mode. This change is required due to changes in the lifecycle of the VcnGatwayConnection, where in safe mode they are NOT torn down, but allowed to continue retrying. During these broken-connectivity windows, the VCN network should be torn down to prevent blackholing traffic. Bug: 183174340 Test: atest FrameworksVcnTests Change-Id: I50f2c0e92552281731c843db89e9a9a1ccff5346 --- .../android/server/VcnManagementService.java | 20 +-- .../core/java/com/android/server/vcn/Vcn.java | 126 +++++++++--------- .../server/vcn/VcnGatewayConnection.java | 119 +++++++++++++---- .../server/VcnManagementServiceTest.java | 52 ++++---- ...cnGatewayConnectionConnectedStateTest.java | 72 ++++++++-- ...nGatewayConnectionConnectingStateTest.java | 5 +- ...tewayConnectionDisconnectingStateTest.java | 5 +- ...atewayConnectionRetryTimeoutStateTest.java | 5 +- .../vcn/VcnGatewayConnectionTestBase.java | 16 ++- .../java/com/android/server/vcn/VcnTest.java | 120 ++++++----------- 10 files changed, 314 insertions(+), 226 deletions(-) diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java index 46f4b0b41a45a..f7ae58ca0eb41 100644 --- a/services/core/java/com/android/server/VcnManagementService.java +++ b/services/core/java/com/android/server/VcnManagementService.java @@ -542,16 +542,7 @@ public class VcnManagementService extends IVcnManagementService.Stub { if (mVcns.containsKey(subscriptionGroup)) { final Vcn vcn = mVcns.get(subscriptionGroup); - final int status = vcn.getStatus(); vcn.updateConfig(config); - - // TODO(b/183174340): Remove this once opportunistic-safe-mode is supported - // Only notify VcnStatusCallbacks if this VCN was previously in Safe Mode - if (status == VCN_STATUS_CODE_SAFE_MODE) { - // TODO(b/181789060): invoke asynchronously after Vcn notifies through VcnCallback - notifyAllPermissionedStatusCallbacksLocked( - subscriptionGroup, VCN_STATUS_CODE_ACTIVE); - } } else { startVcnLocked(subscriptionGroup, config); } @@ -941,8 +932,8 @@ public class VcnManagementService extends IVcnManagementService.Stub { // TODO(b/180452282): Make name more generic and implement directly with VcnManagementService /** Callback for Vcn signals sent up to VcnManagementService. */ public interface VcnCallback { - /** Called by a Vcn to signal that it has entered safe mode. */ - void onEnteredSafeMode(); + /** Called by a Vcn to signal that its safe mode status has changed. */ + void onSafeModeStatusChanged(boolean isInSafeMode); /** Called by a Vcn to signal that an error occurred. */ void onGatewayConnectionError( @@ -1004,15 +995,18 @@ public class VcnManagementService extends IVcnManagementService.Stub { } @Override - public void onEnteredSafeMode() { + public void onSafeModeStatusChanged(boolean isInSafeMode) { synchronized (mLock) { // Ignore if this subscription group doesn't exist anymore if (!mVcns.containsKey(mSubGroup)) { return; } + final int status = + isInSafeMode ? VCN_STATUS_CODE_SAFE_MODE : VCN_STATUS_CODE_ACTIVE; + notifyAllPolicyListenersLocked(); - notifyAllPermissionedStatusCallbacksLocked(mSubGroup, VCN_STATUS_CODE_SAFE_MODE); + notifyAllPermissionedStatusCallbacksLocked(mSubGroup, status); } } diff --git a/services/core/java/com/android/server/vcn/Vcn.java b/services/core/java/com/android/server/vcn/Vcn.java index 54689358802f1..ca7289fd540af 100644 --- a/services/core/java/com/android/server/vcn/Vcn.java +++ b/services/core/java/com/android/server/vcn/Vcn.java @@ -96,17 +96,21 @@ public class Vcn extends Handler { */ private static final int MSG_EVENT_GATEWAY_CONNECTION_QUIT = MSG_EVENT_BASE + 3; + /** + * Triggers reevaluation of safe mode conditions. + * + *

Upon entering safe mode, the VCN will only provide gateway connections opportunistically, + * leaving the underlying networks marked as NOT_VCN_MANAGED. + * + *

Any VcnGatewayConnection in safe mode will result in the entire Vcn instance being put + * into safe mode. Upon receiving this message, the Vcn MUST query all VcnGatewayConnections to + * determine if any are in safe mode. + */ + private static final int MSG_EVENT_SAFE_MODE_STATE_CHANGED = MSG_EVENT_BASE + 4; + /** Triggers an immediate teardown of the entire Vcn, including GatewayConnections. */ private static final int MSG_CMD_TEARDOWN = MSG_CMD_BASE; - /** - * Causes this VCN to immediately enter safe mode. - * - *

Upon entering safe mode, the VCN will unregister its RequestListener, tear down all of its - * VcnGatewayConnections, and notify VcnManagementService that it is in safe mode. - */ - private static final int MSG_CMD_ENTER_SAFE_MODE = MSG_CMD_BASE + 1; - @NonNull private final VcnContext mVcnContext; @NonNull private final ParcelUuid mSubscriptionGroup; @NonNull private final Dependencies mDeps; @@ -233,6 +237,11 @@ public class Vcn extends Handler { @Override public void handleMessage(@NonNull Message msg) { + if (mCurrentStatus != VCN_STATUS_CODE_ACTIVE + && mCurrentStatus != VCN_STATUS_CODE_SAFE_MODE) { + return; + } + switch (msg.what) { case MSG_EVENT_CONFIG_UPDATED: handleConfigUpdated((VcnConfig) msg.obj); @@ -246,12 +255,12 @@ public class Vcn extends Handler { case MSG_EVENT_GATEWAY_CONNECTION_QUIT: handleGatewayConnectionQuit((VcnGatewayConnectionConfig) msg.obj); break; + case MSG_EVENT_SAFE_MODE_STATE_CHANGED: + handleSafeModeStatusChanged(); + break; case MSG_CMD_TEARDOWN: handleTeardown(); break; - case MSG_CMD_ENTER_SAFE_MODE: - handleEnterSafeMode(); - break; default: Slog.wtf(getLogTag(), "Unknown msg.what: " + msg.what); } @@ -263,40 +272,28 @@ public class Vcn extends Handler { mConfig = config; - // TODO(b/183174340): Remove this once opportunistic safe mode is supported. - if (mCurrentStatus == VCN_STATUS_CODE_ACTIVE) { - // VCN is already active - teardown any GatewayConnections whose configs have been - // removed and get all current requests - for (final Entry entry : - mVcnGatewayConnections.entrySet()) { - final VcnGatewayConnectionConfig gatewayConnectionConfig = entry.getKey(); - final VcnGatewayConnection gatewayConnection = entry.getValue(); + // Teardown any GatewayConnections whose configs have been removed and get all current + // requests + for (final Entry entry : + mVcnGatewayConnections.entrySet()) { + final VcnGatewayConnectionConfig gatewayConnectionConfig = entry.getKey(); + final VcnGatewayConnection gatewayConnection = entry.getValue(); - // GatewayConnectionConfigs must match exactly (otherwise authentication or - // connection details may have changed). - if (!mConfig.getGatewayConnectionConfigs().contains(gatewayConnectionConfig)) { - if (gatewayConnection == null) { - Slog.wtf( - getLogTag(), - "Found gatewayConnectionConfig without GatewayConnection"); - } else { - gatewayConnection.teardownAsynchronously(); - } + // GatewayConnectionConfigs must match exactly (otherwise authentication or + // connection details may have changed). + if (!mConfig.getGatewayConnectionConfigs().contains(gatewayConnectionConfig)) { + if (gatewayConnection == null) { + Slog.wtf( + getLogTag(), "Found gatewayConnectionConfig without GatewayConnection"); + } else { + gatewayConnection.teardownAsynchronously(); } } - - // Trigger a re-evaluation of all NetworkRequests (to make sure any that can be - // satisfied start a new GatewayConnection) - mVcnContext.getVcnNetworkProvider().resendAllRequests(mRequestListener); - } else if (mCurrentStatus == VCN_STATUS_CODE_SAFE_MODE) { - // If this VCN was not previously active, it is exiting Safe Mode. Re-register the - // request listener to get NetworkRequests again (and all cached requests). - mVcnContext.getVcnNetworkProvider().registerListener(mRequestListener); - } else { - // Ignored; VCN was not active; config updates ignored. - return; } - mCurrentStatus = VCN_STATUS_CODE_ACTIVE; + + // Trigger a re-evaluation of all NetworkRequests (to make sure any that can be + // satisfied start a new GatewayConnection) + mVcnContext.getVcnNetworkProvider().resendAllRequests(mRequestListener); } private void handleTeardown() { @@ -309,21 +306,27 @@ public class Vcn extends Handler { mCurrentStatus = VCN_STATUS_CODE_INACTIVE; } - private void handleEnterSafeMode() { - // TODO(b/183174340): Remove this once opportunistic-safe-mode is supported - handleTeardown(); + private void handleSafeModeStatusChanged() { + boolean hasSafeModeGatewayConnection = false; - mCurrentStatus = VCN_STATUS_CODE_SAFE_MODE; - mVcnCallback.onEnteredSafeMode(); + // If any VcnGatewayConnection is in safe mode, mark the entire VCN as being in safe mode + for (VcnGatewayConnection gatewayConnection : mVcnGatewayConnections.values()) { + if (gatewayConnection.isInSafeMode()) { + hasSafeModeGatewayConnection = true; + break; + } + } + + final int oldStatus = mCurrentStatus; + mCurrentStatus = + hasSafeModeGatewayConnection ? VCN_STATUS_CODE_SAFE_MODE : VCN_STATUS_CODE_ACTIVE; + if (oldStatus != mCurrentStatus) { + mVcnCallback.onSafeModeStatusChanged(hasSafeModeGatewayConnection); + } } private void handleNetworkRequested( @NonNull NetworkRequest request, int score, int providerId) { - if (mCurrentStatus != VCN_STATUS_CODE_ACTIVE) { - Slog.v(getLogTag(), "Received NetworkRequest while inactive. Ignore for now"); - return; - } - if (score > getNetworkScore()) { if (VDBG) { Slog.v( @@ -376,19 +379,16 @@ public class Vcn extends Handler { mVcnGatewayConnections.remove(config); // Trigger a re-evaluation of all NetworkRequests (to make sure any that can be satisfied - // start a new GatewayConnection), but only if the Vcn is still alive - if (mCurrentStatus == VCN_STATUS_CODE_ACTIVE) { - mVcnContext.getVcnNetworkProvider().resendAllRequests(mRequestListener); - } + // start a new GatewayConnection). VCN is always alive here, courtesy of the liveness check + // in handleMessage() + mVcnContext.getVcnNetworkProvider().resendAllRequests(mRequestListener); } private void handleSubscriptionsChanged(@NonNull TelephonySubscriptionSnapshot snapshot) { mLastSnapshot = snapshot; - if (mCurrentStatus == VCN_STATUS_CODE_ACTIVE) { - for (VcnGatewayConnection gatewayConnection : mVcnGatewayConnections.values()) { - gatewayConnection.updateSubscriptionSnapshot(mLastSnapshot); - } + for (VcnGatewayConnection gatewayConnection : mVcnGatewayConnections.values()) { + gatewayConnection.updateSubscriptionSnapshot(mLastSnapshot); } } @@ -418,8 +418,8 @@ public class Vcn extends Handler { /** Callback used for passing status signals from a VcnGatewayConnection to its managing Vcn. */ @VisibleForTesting(visibility = Visibility.PACKAGE) public interface VcnGatewayStatusCallback { - /** Called by a VcnGatewayConnection to indicate that it has entered safe mode. */ - void onEnteredSafeMode(); + /** Called by a VcnGatewayConnection to indicate that it's safe mode status has changed. */ + void onSafeModeStatusChanged(); /** Callback by a VcnGatewayConnection to indicate that an error occurred. */ void onGatewayConnectionError( @@ -445,8 +445,8 @@ public class Vcn extends Handler { } @Override - public void onEnteredSafeMode() { - sendMessage(obtainMessage(MSG_CMD_ENTER_SAFE_MODE)); + public void onSafeModeStatusChanged() { + sendMessage(obtainMessage(MSG_EVENT_SAFE_MODE_STATE_CHANGED)); } @Override diff --git a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java index 2ba8edd3b1d01..1780260d36c1e 100644 --- a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java +++ b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java @@ -44,6 +44,7 @@ import android.net.Network; import android.net.NetworkAgent; import android.net.NetworkAgentConfig; import android.net.NetworkCapabilities; +import android.net.NetworkProvider; import android.net.RouteInfo; import android.net.TelephonyNetworkSpecifier; import android.net.Uri; @@ -92,6 +93,7 @@ import java.util.List; import java.util.Objects; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; /** * A single VCN Gateway Connection, providing a single public-facing VCN network. @@ -503,6 +505,15 @@ public class VcnGatewayConnection extends StateMachine { */ private boolean mIsQuitting = false; + /** + * Whether the VcnGatewayConnection is in safe mode. + * + *

Upon hitting the safe mode timeout, this will be set to {@code true}. In safe mode, this + * VcnGatewayConnection will continue attempting to connect, and if a successful connection is + * made, safe mode will be exited. + */ + private boolean mIsInSafeMode = false; + /** * The token used by the primary/current/active session. * @@ -562,8 +573,7 @@ public class VcnGatewayConnection extends StateMachine { *

Set in Connected state, always @NonNull in Connected, Migrating states, @Nullable * otherwise. */ - @VisibleForTesting(visibility = Visibility.PRIVATE) - NetworkAgent mNetworkAgent; + private NetworkAgent mNetworkAgent; @Nullable private WakeupMessage mTeardownTimeoutAlarm; @Nullable private WakeupMessage mDisconnectRequestAlarm; @@ -628,6 +638,14 @@ public class VcnGatewayConnection extends StateMachine { start(); } + /** Queries whether this VcnGatewayConnection is in safe mode. */ + public boolean isInSafeMode() { + // Accessing internal state; must only be done on looper thread. + mVcnContext.ensureRunningOnLooperThread(); + + return mIsInSafeMode; + } + /** * Asynchronously tears down this GatewayConnection, and any resources used. * @@ -1162,6 +1180,15 @@ public class VcnGatewayConnection extends StateMachine { } } + protected void handleSafeModeTimeoutExceeded() { + mSafeModeTimeoutAlarm = null; + + // Connectivity for this GatewayConnection is broken; tear down the Network. + teardownNetwork(); + mIsInSafeMode = true; + mGatewayStatusCallback.onSafeModeStatusChanged(); + } + protected void logUnexpectedEvent(int what) { Slog.d(TAG, String.format( "Unexpected event code %d in state %s", what, this.getClass().getSimpleName())); @@ -1315,8 +1342,7 @@ public class VcnGatewayConnection extends StateMachine { } break; case EVENT_SAFE_MODE_TIMEOUT_EXCEEDED: - mGatewayStatusCallback.onEnteredSafeMode(); - mSafeModeTimeoutAlarm = null; + handleSafeModeTimeoutExceeded(); break; default: logUnhandledMessage(msg); @@ -1401,8 +1427,7 @@ public class VcnGatewayConnection extends StateMachine { handleDisconnectRequested((EventDisconnectRequestedInfo) msg.obj); break; case EVENT_SAFE_MODE_TIMEOUT_EXCEEDED: - mGatewayStatusCallback.onEnteredSafeMode(); - mSafeModeTimeoutAlarm = null; + handleSafeModeTimeoutExceeded(); break; default: logUnhandledMessage(msg); @@ -1434,28 +1459,23 @@ public class VcnGatewayConnection extends StateMachine { buildConnectedLinkProperties(mConnectionConfig, tunnelIface, childConfig); final NetworkAgent agent = - new NetworkAgent( - mVcnContext.getContext(), - mVcnContext.getLooper(), + mDeps.newNetworkAgent( + mVcnContext, TAG, caps, lp, Vcn.getNetworkScore(), new NetworkAgentConfig.Builder().build(), - mVcnContext.getVcnNetworkProvider()) { - @Override - public void onNetworkUnwanted() { - Slog.d(TAG, "NetworkAgent was unwanted"); - teardownAsynchronously(); - } - - @Override - public void onValidationStatus(int status, @Nullable Uri redirectUri) { - if (status == NetworkAgent.VALIDATION_STATUS_VALID) { - clearFailedAttemptCounterAndSafeModeAlarm(); - } - } - }; + mVcnContext.getVcnNetworkProvider(), + () -> { + Slog.d(TAG, "NetworkAgent was unwanted"); + teardownAsynchronously(); + } /* networkUnwantedCallback */, + (status) -> { + if (status == NetworkAgent.VALIDATION_STATUS_VALID) { + clearFailedAttemptCounterAndSafeModeAlarm(); + } + } /* validationStatusCallback */); agent.register(); agent.markConnected(); @@ -1469,6 +1489,11 @@ public class VcnGatewayConnection extends StateMachine { // Validated connection, clear failed attempt counter mFailedAttempts = 0; cancelSafeModeAlarm(); + + if (mIsInSafeMode) { + mIsInSafeMode = false; + mGatewayStatusCallback.onSafeModeStatusChanged(); + } } protected void applyTransform( @@ -1587,8 +1612,7 @@ public class VcnGatewayConnection extends StateMachine { handleDisconnectRequested((EventDisconnectRequestedInfo) msg.obj); break; case EVENT_SAFE_MODE_TIMEOUT_EXCEEDED: - mGatewayStatusCallback.onEnteredSafeMode(); - mSafeModeTimeoutAlarm = null; + handleSafeModeTimeoutExceeded(); break; default: logUnhandledMessage(msg); @@ -1692,8 +1716,7 @@ public class VcnGatewayConnection extends StateMachine { handleDisconnectRequested((EventDisconnectRequestedInfo) msg.obj); break; case EVENT_SAFE_MODE_TIMEOUT_EXCEEDED: - mGatewayStatusCallback.onEnteredSafeMode(); - mSafeModeTimeoutAlarm = null; + handleSafeModeTimeoutExceeded(); break; default: logUnhandledMessage(msg); @@ -1934,6 +1957,16 @@ public class VcnGatewayConnection extends StateMachine { mIkeSession = session; } + @VisibleForTesting(visibility = Visibility.PRIVATE) + NetworkAgent getNetworkAgent() { + return mNetworkAgent; + } + + @VisibleForTesting(visibility = Visibility.PRIVATE) + void setNetworkAgent(@Nullable NetworkAgent networkAgent) { + mNetworkAgent = networkAgent; + } + @VisibleForTesting(visibility = Visibility.PRIVATE) void sendDisconnectRequestedAndAcquireWakelock(String reason, boolean shouldQuit) { sendMessageAndAcquireWakeLock( @@ -2018,6 +2051,38 @@ public class VcnGatewayConnection extends StateMachine { return new WakeupMessage(vcnContext.getContext(), handler, tag, runnable); } + /** Builds a new NetworkAgent. */ + public NetworkAgent newNetworkAgent( + @NonNull VcnContext vcnContext, + @NonNull String tag, + @NonNull NetworkCapabilities caps, + @NonNull LinkProperties lp, + @NonNull int score, + @NonNull NetworkAgentConfig nac, + @NonNull NetworkProvider provider, + @NonNull Runnable networkUnwantedCallback, + @NonNull Consumer validationStatusCallback) { + return new NetworkAgent( + vcnContext.getContext(), + vcnContext.getLooper(), + tag, + caps, + lp, + score, + nac, + provider) { + @Override + public void onNetworkUnwanted() { + networkUnwantedCallback.run(); + } + + @Override + public void onValidationStatus(int status, @Nullable Uri redirectUri) { + validationStatusCallback.accept(status); + } + }; + } + /** Gets the elapsed real time since boot, in millis. */ public long getElapsedRealTime() { return SystemClock.elapsedRealtime(); diff --git a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java index 4ffbf3147ee66..43e6676e1d4c6 100644 --- a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java +++ b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java @@ -536,17 +536,6 @@ public class VcnManagementServiceTest { verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_ACTIVE); } - @Test - public void testSetVcnConfigInSafeModeNotifiesStatusCallback() throws Exception { - setupSubscriptionAndStartVcn(TEST_SUBSCRIPTION_ID, TEST_UUID_2, false /* isActive */); - mVcnMgmtSvc.registerVcnStatusCallback(TEST_UUID_2, mMockStatusCallback, TEST_PACKAGE_NAME); - verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_SAFE_MODE); - - mVcnMgmtSvc.setVcnConfig(TEST_UUID_2, TEST_VCN_CONFIG, TEST_PACKAGE_NAME); - - verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_ACTIVE); - } - @Test public void testClearVcnConfigRequiresNonSystemServer() throws Exception { doReturn(Process.SYSTEM_UID).when(mMockDeps).getBinderCallingUid(); @@ -902,7 +891,9 @@ public class VcnManagementServiceTest { } private void triggerVcnSafeMode( - @NonNull ParcelUuid subGroup, @NonNull TelephonySubscriptionSnapshot snapshot) + @NonNull ParcelUuid subGroup, + @NonNull TelephonySubscriptionSnapshot snapshot, + boolean isInSafeMode) throws Exception { verify(mMockDeps) .newVcn( @@ -913,22 +904,32 @@ public class VcnManagementServiceTest { mVcnCallbackCaptor.capture()); VcnCallback vcnCallback = mVcnCallbackCaptor.getValue(); - vcnCallback.onEnteredSafeMode(); + vcnCallback.onSafeModeStatusChanged(isInSafeMode); } - @Test - public void testVcnEnteringSafeModeNotifiesPolicyListeners() throws Exception { + private void verifyVcnSafeModeChangesNotifiesPolicyListeners(boolean enterSafeMode) + throws Exception { TelephonySubscriptionSnapshot snapshot = triggerSubscriptionTrackerCbAndGetSnapshot(Collections.singleton(TEST_UUID_1)); mVcnMgmtSvc.addVcnUnderlyingNetworkPolicyListener(mMockPolicyListener); - triggerVcnSafeMode(TEST_UUID_1, snapshot); + triggerVcnSafeMode(TEST_UUID_1, snapshot, enterSafeMode); verify(mMockPolicyListener).onPolicyChanged(); } - private void triggerVcnStatusCallbackOnEnteredSafeMode( + @Test + public void testVcnEnteringSafeModeNotifiesPolicyListeners() throws Exception { + verifyVcnSafeModeChangesNotifiesPolicyListeners(true /* enterSafeMode */); + } + + @Test + public void testVcnExitingSafeModeNotifiesPolicyListeners() throws Exception { + verifyVcnSafeModeChangesNotifiesPolicyListeners(false /* enterSafeMode */); + } + + private void triggerVcnStatusCallbackOnSafeModeStatusChanged( @NonNull ParcelUuid subGroup, @NonNull String pkgName, int uid, @@ -951,12 +952,13 @@ public class VcnManagementServiceTest { mVcnMgmtSvc.registerVcnStatusCallback(subGroup, mMockStatusCallback, pkgName); - triggerVcnSafeMode(subGroup, snapshot); + triggerVcnSafeMode(subGroup, snapshot, true /* enterSafeMode */); } @Test - public void testVcnStatusCallbackOnEnteredSafeModeWithCarrierPrivileges() throws Exception { - triggerVcnStatusCallbackOnEnteredSafeMode( + public void testVcnStatusCallbackOnSafeModeStatusChangedWithCarrierPrivileges() + throws Exception { + triggerVcnStatusCallbackOnSafeModeStatusChanged( TEST_UUID_1, TEST_PACKAGE_NAME, TEST_UID, @@ -967,8 +969,9 @@ public class VcnManagementServiceTest { } @Test - public void testVcnStatusCallbackOnEnteredSafeModeWithoutCarrierPrivileges() throws Exception { - triggerVcnStatusCallbackOnEnteredSafeMode( + public void testVcnStatusCallbackOnSafeModeStatusChangedWithoutCarrierPrivileges() + throws Exception { + triggerVcnStatusCallbackOnSafeModeStatusChanged( TEST_UUID_1, TEST_PACKAGE_NAME, TEST_UID, @@ -980,8 +983,9 @@ public class VcnManagementServiceTest { } @Test - public void testVcnStatusCallbackOnEnteredSafeModeWithoutLocationPermission() throws Exception { - triggerVcnStatusCallbackOnEnteredSafeMode( + public void testVcnStatusCallbackOnSafeModeStatusChangedWithoutLocationPermission() + throws Exception { + triggerVcnStatusCallbackOnSafeModeStatusChanged( TEST_UUID_1, TEST_PACKAGE_NAME, TEST_UID, diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java index 2fadd44440f32..fdd7e3e6bdcfa 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java @@ -60,17 +60,24 @@ import org.mockito.ArgumentCaptor; import java.io.IOException; import java.net.UnknownHostException; import java.util.Collections; +import java.util.function.Consumer; /** Tests for VcnGatewayConnection.ConnectedState */ @RunWith(AndroidJUnit4.class) @SmallTest public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnectionTestBase { private VcnIkeSession mIkeSession; + private NetworkAgent mNetworkAgent; @Before public void setUp() throws Exception { super.setUp(); + mNetworkAgent = mock(NetworkAgent.class); + doReturn(mNetworkAgent) + .when(mDeps) + .newNetworkAgent(any(), any(), any(), any(), anyInt(), any(), any(), any(), any()); + mGatewayConnection.setUnderlyingNetwork(TEST_UNDERLYING_NETWORK_RECORD_1); mIkeSession = mGatewayConnection.buildIkeSession(TEST_UNDERLYING_NETWORK_RECORD_1.network); @@ -159,11 +166,7 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection assertEquals(mGatewayConnection.mConnectedState, mGatewayConnection.getCurrentState()); } - @Test - public void testChildOpenedRegistersNetwork() throws Exception { - // Verify scheduled but not canceled when entering ConnectedState - verifySafeModeTimeoutAlarmAndGetCallback(false /* expectCanceled */); - + private void triggerChildOpened() { final VcnChildSessionConfiguration mMockChildSessionConfig = mock(VcnChildSessionConfiguration.class); doReturn(Collections.singletonList(TEST_INTERNAL_ADDR)) @@ -174,6 +177,31 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection .getInternalDnsServers(); getChildSessionCallback().onOpened(mMockChildSessionConfig); + } + + private void triggerValidation(int status) { + final ArgumentCaptor> validationCallbackCaptor = + ArgumentCaptor.forClass(Consumer.class); + verify(mDeps) + .newNetworkAgent( + any(), + any(), + any(), + any(), + anyInt(), + any(), + any(), + any(), + validationCallbackCaptor.capture()); + + validationCallbackCaptor.getValue().accept(status); + } + + @Test + public void testChildOpenedRegistersNetwork() throws Exception { + // Verify scheduled but not canceled when entering ConnectedState + verifySafeModeTimeoutAlarmAndGetCallback(false /* expectCanceled */); + triggerChildOpened(); mTestLooper.dispatchAll(); assertEquals(mGatewayConnection.mConnectedState, mGatewayConnection.getCurrentState()); @@ -182,15 +210,20 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection ArgumentCaptor.forClass(LinkProperties.class); final ArgumentCaptor ncCaptor = ArgumentCaptor.forClass(NetworkCapabilities.class); - verify(mConnMgr) - .registerNetworkAgent( - any(), - any(), - lpCaptor.capture(), + verify(mDeps) + .newNetworkAgent( + eq(mVcnContext), + any(String.class), ncCaptor.capture(), + lpCaptor.capture(), + anyInt(), any(), any(), - anyInt()); + any(), + any()); + verify(mNetworkAgent).register(); + verify(mNetworkAgent).markConnected(); + verify(mIpSecSvc) .addAddressToTunnelInterface( eq(TEST_IPSEC_TUNNEL_RESOURCE_ID), eq(TEST_INTERNAL_ADDR), any()); @@ -208,9 +241,22 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection // Now that Vcn Network is up, notify it as validated and verify the SafeMode alarm is // canceled - mGatewayConnection.mNetworkAgent.onValidationStatus( - NetworkAgent.VALIDATION_STATUS_VALID, null /* redirectUri */); + triggerValidation(NetworkAgent.VALIDATION_STATUS_VALID); verify(mSafeModeTimeoutAlarm).cancel(); + assertFalse(mGatewayConnection.isInSafeMode()); + } + + @Test + public void testSuccessfulConnectionExitsSafeMode() throws Exception { + verifySafeModeTimeoutNotifiesCallbackAndUnregistersNetworkAgent( + mGatewayConnection.mConnectedState); + + triggerChildOpened(); + mTestLooper.dispatchAll(); + + triggerValidation(NetworkAgent.VALIDATION_STATUS_VALID); + + assertFalse(mGatewayConnection.isInSafeMode()); } @Test diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectingStateTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectingStateTest.java index 7afa4494ee8b4..bfe8c73d63898 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectingStateTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectingStateTest.java @@ -118,8 +118,9 @@ public class VcnGatewayConnectionConnectingStateTest extends VcnGatewayConnectio } @Test - public void testSafeModeTimeoutNotifiesCallback() { - verifySafeModeTimeoutNotifiesCallback(mGatewayConnection.mConnectingState); + public void testSafeModeTimeoutNotifiesCallbackAndUnregistersNetworkAgent() { + verifySafeModeTimeoutNotifiesCallbackAndUnregistersNetworkAgent( + mGatewayConnection.mConnectingState); } @Test diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionDisconnectingStateTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionDisconnectingStateTest.java index 99feffdebc8e3..9da8b451c9fc4 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionDisconnectingStateTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionDisconnectingStateTest.java @@ -86,8 +86,9 @@ public class VcnGatewayConnectionDisconnectingStateTest extends VcnGatewayConnec } @Test - public void testSafeModeTimeoutNotifiesCallback() { - verifySafeModeTimeoutNotifiesCallback(mGatewayConnection.mDisconnectingState); + public void testSafeModeTimeoutNotifiesCallbackAndUnregistersNetworkAgent() { + verifySafeModeTimeoutNotifiesCallbackAndUnregistersNetworkAgent( + mGatewayConnection.mDisconnectingState); } @Test diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionRetryTimeoutStateTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionRetryTimeoutStateTest.java index 85a0277f8b483..6dbf7d552bb61 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionRetryTimeoutStateTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionRetryTimeoutStateTest.java @@ -96,8 +96,9 @@ public class VcnGatewayConnectionRetryTimeoutStateTest extends VcnGatewayConnect } @Test - public void testSafeModeTimeoutNotifiesCallback() { - verifySafeModeTimeoutNotifiesCallback(mGatewayConnection.mRetryTimeoutState); + public void testSafeModeTimeoutNotifiesCallbackAndUnregistersNetworkAgent() { + verifySafeModeTimeoutNotifiesCallbackAndUnregistersNetworkAgent( + mGatewayConnection.mRetryTimeoutState); } @Test diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java index a660735470a41..884b2338d877e 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java @@ -21,6 +21,8 @@ import static com.android.server.vcn.VcnGatewayConnection.VcnIkeSession; import static com.android.server.vcn.VcnTestUtils.setupIpSecManager; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.atLeastOnce; @@ -42,6 +44,7 @@ import android.net.IpSecTunnelInterfaceResponse; import android.net.LinkAddress; import android.net.LinkProperties; import android.net.Network; +import android.net.NetworkAgent; import android.net.NetworkCapabilities; import android.net.ipsec.ike.ChildSessionCallback; import android.net.ipsec.ike.IkeSessionCallback; @@ -267,7 +270,12 @@ public class VcnGatewayConnectionTestBase { expectCanceled); } - protected void verifySafeModeTimeoutNotifiesCallback(@NonNull State expectedState) { + protected void verifySafeModeTimeoutNotifiesCallbackAndUnregistersNetworkAgent( + @NonNull State expectedState) { + // Set a NetworkAgent, and expect it to be unregistered and cleared + final NetworkAgent mockNetworkAgent = mock(NetworkAgent.class); + mGatewayConnection.setNetworkAgent(mockNetworkAgent); + // SafeMode timer starts when VcnGatewayConnection exits DisconnectedState (the initial // state) final Runnable delayedEvent = @@ -275,7 +283,11 @@ public class VcnGatewayConnectionTestBase { delayedEvent.run(); mTestLooper.dispatchAll(); - verify(mGatewayStatusCallback).onEnteredSafeMode(); + verify(mGatewayStatusCallback).onSafeModeStatusChanged(); assertEquals(expectedState, mGatewayConnection.getCurrentState()); + assertTrue(mGatewayConnection.isInSafeMode()); + + verify(mockNetworkAgent).unregister(); + assertNull(mGatewayConnection.getNetworkAgent()); } } diff --git a/tests/vcn/java/com/android/server/vcn/VcnTest.java b/tests/vcn/java/com/android/server/vcn/VcnTest.java index 540be38ed7988..9dc14b68baf89 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnTest.java @@ -20,13 +20,11 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.NetworkCapabilities.NET_CAPABILITY_MMS; import static android.net.vcn.VcnManager.VCN_STATUS_CODE_ACTIVE; -import static android.net.vcn.VcnManager.VCN_STATUS_CODE_INACTIVE; import static android.net.vcn.VcnManager.VCN_STATUS_CODE_SAFE_MODE; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.argThat; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; @@ -54,7 +52,6 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Set; import java.util.UUID; @@ -160,8 +157,7 @@ public class VcnTest { mTestLooper.dispatchAll(); for (final VcnGatewayConnection gateway : gatewayConnections) { - verify(gateway, status == VCN_STATUS_CODE_ACTIVE ? times(1) : never()) - .updateSubscriptionSnapshot(eq(updatedSnapshot)); + verify(gateway).updateSubscriptionSnapshot(eq(updatedSnapshot)); } } @@ -202,32 +198,53 @@ public class VcnTest { private void verifySafeMode( NetworkRequestListener requestListener, - Set expectedGatewaysTornDown) { - assertEquals(VCN_STATUS_CODE_SAFE_MODE, mVcn.getStatus()); - for (final VcnGatewayConnection gatewayConnection : expectedGatewaysTornDown) { - verify(gatewayConnection).teardownAsynchronously(); + Set activeGateways, + boolean expectInSafeMode) { + for (VcnGatewayConnection gatewayConnection : activeGateways) { + verify(gatewayConnection, never()).teardownAsynchronously(); } - verify(mVcnNetworkProvider).unregisterListener(requestListener); - verify(mVcnCallback).onEnteredSafeMode(); + + assertEquals( + expectInSafeMode ? VCN_STATUS_CODE_SAFE_MODE : VCN_STATUS_CODE_ACTIVE, + mVcn.getStatus()); + verify(mVcnCallback).onSafeModeStatusChanged(expectInSafeMode); } @Test - public void testGatewayEnteringSafeModeNotifiesVcn() { + public void testGatewayEnteringAndExitingSafeModeNotifiesVcn() { final NetworkRequestListener requestListener = verifyAndGetRequestListener(); final Set gatewayConnections = startGatewaysAndGetGatewayConnections(requestListener); - // Doesn't matter which callback this gets - any Gateway entering Safemode should shut down - // all Gateways + // Doesn't matter which callback this gets, or which VCN is in safe mode - any Gateway + // entering Safemode should trigger safe mode final VcnGatewayStatusCallback statusCallback = mGatewayStatusCallbackCaptor.getValue(); - statusCallback.onEnteredSafeMode(); + final VcnGatewayConnection gatewayConnection = gatewayConnections.iterator().next(); + + doReturn(true).when(gatewayConnection).isInSafeMode(); + statusCallback.onSafeModeStatusChanged(); mTestLooper.dispatchAll(); - verifySafeMode(requestListener, gatewayConnections); + verifySafeMode(requestListener, gatewayConnections, true /* expectInSafeMode */); + + // Verify that when all GatewayConnections exit safe mode, the VCN also exits safe mode + doReturn(false).when(gatewayConnection).isInSafeMode(); + statusCallback.onSafeModeStatusChanged(); + mTestLooper.dispatchAll(); + + verifySafeMode(requestListener, gatewayConnections, false /* expectInSafeMode */); + + // Re-trigger, verify safe mode callback does not get fired again for identical state + statusCallback.onSafeModeStatusChanged(); + mTestLooper.dispatchAll(); + + // Expect only once still; from above. + verify(mVcnCallback).onSafeModeStatusChanged(false); } - @Test - public void testGatewayQuit() { + private void verifyGatewayQuit(int status) { + mVcn.setStatus(status); + final NetworkRequestListener requestListener = verifyAndGetRequestListener(); final Set gatewayConnections = new ArraySet<>(startGatewaysAndGetGatewayConnections(requestListener)); @@ -240,7 +257,7 @@ public class VcnTest { assertEquals(1, mVcn.getVcnGatewayConnections().size()); verify(mVcnNetworkProvider).resendAllRequests(requestListener); - // Verify that the VcnGatewayConnection is restarted + // Verify that the VcnGatewayConnection is restarted if a request exists for it triggerVcnRequestListeners(requestListener); mTestLooper.dispatchAll(); assertEquals(2, mVcn.getVcnGatewayConnections().size()); @@ -254,21 +271,13 @@ public class VcnTest { } @Test - public void testGatewayQuitWhileInactive() { - final NetworkRequestListener requestListener = verifyAndGetRequestListener(); - final Set gatewayConnections = - new ArraySet<>(startGatewaysAndGetGatewayConnections(requestListener)); + public void testGatewayQuitReevaluatesRequests() { + verifyGatewayQuit(VCN_STATUS_CODE_ACTIVE); + } - mVcn.teardownAsynchronously(); - mTestLooper.dispatchAll(); - - final VcnGatewayStatusCallback statusCallback = mGatewayStatusCallbackCaptor.getValue(); - statusCallback.onQuit(); - mTestLooper.dispatchAll(); - - // Verify that the VCN requests the networkRequests be resent - assertEquals(1, mVcn.getVcnGatewayConnections().size()); - verify(mVcnNetworkProvider, never()).resendAllRequests(requestListener); + @Test + public void testGatewayQuitReevaluatesRequestsInSafeMode() { + verifyGatewayQuit(VCN_STATUS_CODE_SAFE_MODE); } @Test @@ -298,49 +307,4 @@ public class VcnTest { verify(removedGatewayConnection).teardownAsynchronously(); verify(mVcnNetworkProvider).resendAllRequests(requestListener); } - - @Test - public void testUpdateConfigExitsSafeMode() { - final NetworkRequestListener requestListener = verifyAndGetRequestListener(); - final Set gatewayConnections = - new ArraySet<>(startGatewaysAndGetGatewayConnections(requestListener)); - - final VcnGatewayStatusCallback statusCallback = mGatewayStatusCallbackCaptor.getValue(); - statusCallback.onEnteredSafeMode(); - mTestLooper.dispatchAll(); - verifySafeMode(requestListener, gatewayConnections); - - doAnswer(invocation -> { - final NetworkRequestListener listener = invocation.getArgument(0); - triggerVcnRequestListeners(listener); - return null; - }).when(mVcnNetworkProvider).registerListener(eq(requestListener)); - - mVcn.updateConfig(mConfig); - mTestLooper.dispatchAll(); - - // Registered on start, then re-registered with new configs - verify(mVcnNetworkProvider, times(2)).registerListener(eq(requestListener)); - assertEquals(VCN_STATUS_CODE_ACTIVE, mVcn.getStatus()); - for (final int[] caps : TEST_CAPS) { - // Expect each gateway connection created only on initial startup - verify(mDeps) - .newVcnGatewayConnection( - eq(mVcnContext), - eq(TEST_SUB_GROUP), - eq(mSubscriptionSnapshot), - argThat(config -> Arrays.equals(caps, config.getExposedCapabilities())), - any()); - } - } - - @Test - public void testIgnoreNetworkRequestWhileInactive() { - mVcn.setStatus(VCN_STATUS_CODE_INACTIVE); - - final NetworkRequestListener requestListener = verifyAndGetRequestListener(); - triggerVcnRequestListeners(requestListener); - - verify(mDeps, never()).newVcnGatewayConnection(any(), any(), any(), any(), any()); - } }