From 80056e834ea0811c0ba1e90063d8c561b8eca6dd Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Tue, 23 Mar 2021 01:26:17 -0700 Subject: [PATCH 1/5] 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()); - } } From 134c10d9c6a6a698aa362040f14f857fb124a297 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 1 Apr 2021 14:09:02 -0700 Subject: [PATCH 2/5] Add TRANSPORT_CELLULAR to VCN filter This change adds the CELLULAR transport to the filter used by the Vcn to determine if a configuration can satisfy a given NetworkRequest Bug: 184101137 Test: atest FrameworksVcnTests Change-Id: I7df3f14b5ef66551728b398fd4a9e233d1df4706 --- services/core/java/com/android/server/vcn/Vcn.java | 2 ++ tests/vcn/java/com/android/server/vcn/VcnTest.java | 2 ++ 2 files changed, 4 insertions(+) diff --git a/services/core/java/com/android/server/vcn/Vcn.java b/services/core/java/com/android/server/vcn/Vcn.java index ca7289fd540af..ae806aa500a67 100644 --- a/services/core/java/com/android/server/vcn/Vcn.java +++ b/services/core/java/com/android/server/vcn/Vcn.java @@ -17,6 +17,7 @@ package com.android.server.vcn; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED; +import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; 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; @@ -395,6 +396,7 @@ public class Vcn extends Handler { private boolean isRequestSatisfiedByGatewayConnectionConfig( @NonNull NetworkRequest request, @NonNull VcnGatewayConnectionConfig config) { final NetworkCapabilities.Builder builder = new NetworkCapabilities.Builder(); + builder.addTransportType(TRANSPORT_CELLULAR); builder.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED); for (int cap : config.getAllExposedCapabilities()) { builder.addCapability(cap); diff --git a/tests/vcn/java/com/android/server/vcn/VcnTest.java b/tests/vcn/java/com/android/server/vcn/VcnTest.java index 9dc14b68baf89..90eb75e865e73 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnTest.java @@ -19,6 +19,7 @@ package com.android.server.vcn; 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.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.vcn.VcnManager.VCN_STATUS_CODE_ACTIVE; import static android.net.vcn.VcnManager.VCN_STATUS_CODE_SAFE_MODE; @@ -133,6 +134,7 @@ public class VcnTest { private void startVcnGatewayWithCapabilities( NetworkRequestListener requestListener, int... netCapabilities) { final NetworkRequest.Builder requestBuilder = new NetworkRequest.Builder(); + requestBuilder.addTransportType(TRANSPORT_CELLULAR); for (final int netCapability : netCapabilities) { requestBuilder.addCapability(netCapability); } From 7d6b4907cc2db06af79b505cf806293e9beb75e4 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 1 Apr 2021 16:58:27 -0700 Subject: [PATCH 3/5] Populate legacy type in VCN NetworkAgentConfig This change ensures that the VCN populates the legacy type field, otherwise ConnectivityService crashes on an unknown legacy type. Bug: 184304972 Test: atest FrameworksVcnTests Change-Id: I59dc46dc34d2812a6c8f73c72d208fa735791d74 --- .../java/com/android/server/vcn/VcnGatewayConnection.java | 7 ++++++- .../server/vcn/VcnGatewayConnectionConnectedStateTest.java | 4 +++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java index 1780260d36c1e..eed2006382260 100644 --- a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java +++ b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java @@ -32,6 +32,7 @@ import static com.android.server.VcnManagementService.VDBG; import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; +import android.net.ConnectivityManager; import android.net.InetAddresses; import android.net.IpPrefix; import android.net.IpSecManager; @@ -1457,6 +1458,10 @@ public class VcnGatewayConnection extends StateMachine { buildNetworkCapabilities(mConnectionConfig, mUnderlying); final LinkProperties lp = buildConnectedLinkProperties(mConnectionConfig, tunnelIface, childConfig); + final NetworkAgentConfig nac = + new NetworkAgentConfig.Builder() + .setLegacyType(ConnectivityManager.TYPE_MOBILE) + .build(); final NetworkAgent agent = mDeps.newNetworkAgent( @@ -1465,7 +1470,7 @@ public class VcnGatewayConnection extends StateMachine { caps, lp, Vcn.getNetworkScore(), - new NetworkAgentConfig.Builder().build(), + nac, mVcnContext.getVcnNetworkProvider(), () -> { Slog.d(TAG, "NetworkAgent was unwanted"); diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java index fdd7e3e6bdcfa..3b0be301edde3 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java @@ -32,6 +32,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.argThat; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -40,6 +41,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; +import android.net.ConnectivityManager; import android.net.LinkProperties; import android.net.NetworkAgent; import android.net.NetworkCapabilities; @@ -217,7 +219,7 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection ncCaptor.capture(), lpCaptor.capture(), anyInt(), - any(), + argThat(nac -> nac.getLegacyType() == ConnectivityManager.TYPE_MOBILE), any(), any(), any()); From f78c3512df1c7048c127a35ff75805730cfa2ab9 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 1 Apr 2021 18:23:45 -0700 Subject: [PATCH 4/5] Add/remove internal addresses from IpSecTunnelInterface This change corrects a bug where IpSecTunnelInterface was not removing old addresses, and resulted in EEXIST when trying to re-add an address to a given interface. Test: atest FrameworksVcnTests Change-Id: I43434c801354483a7c7d0092891799bf86da23eb --- .../server/vcn/VcnGatewayConnection.java | 16 ++--- ...cnGatewayConnectionConnectedStateTest.java | 67 +++++++++++++++++-- .../vcn/VcnGatewayConnectionTestBase.java | 8 ++- 3 files changed, 76 insertions(+), 15 deletions(-) diff --git a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java index eed2006382260..efe78377575e8 100644 --- a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java +++ b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java @@ -1518,13 +1518,6 @@ public class VcnGatewayConnection extends StateMachine { } } - protected void setupInterface( - int token, - @NonNull IpSecTunnelInterface tunnelIface, - @NonNull VcnChildSessionConfiguration childConfig) { - setupInterface(token, tunnelIface, childConfig, null); - } - protected void setupInterface( int token, @NonNull IpSecTunnelInterface tunnelIface, @@ -1609,9 +1602,11 @@ public class VcnGatewayConnection extends StateMachine { transformCreatedInfo.direction); break; case EVENT_SETUP_COMPLETED: + final VcnChildSessionConfiguration oldChildConfig = mChildConfig; mChildConfig = ((EventSetupCompletedInfo) msg.obj).childSessionConfig; - setupInterfaceAndNetworkAgent(mCurrentToken, mTunnelIface, mChildConfig); + setupInterfaceAndNetworkAgent( + mCurrentToken, mTunnelIface, mChildConfig, oldChildConfig); break; case EVENT_DISCONNECT_REQUESTED: handleDisconnectRequested((EventDisconnectRequestedInfo) msg.obj); @@ -1655,8 +1650,9 @@ public class VcnGatewayConnection extends StateMachine { protected void setupInterfaceAndNetworkAgent( int token, @NonNull IpSecTunnelInterface tunnelIface, - @NonNull VcnChildSessionConfiguration childConfig) { - setupInterface(token, tunnelIface, childConfig); + @NonNull VcnChildSessionConfiguration childConfig, + @NonNull VcnChildSessionConfiguration oldChildConfig) { + setupInterface(token, tunnelIface, childConfig, oldChildConfig); if (mNetworkAgent == null) { mNetworkAgent = buildNetworkAgent(tunnelIface, childConfig); diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java index 3b0be301edde3..54086c2a332f6 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java @@ -42,6 +42,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import android.net.ConnectivityManager; +import android.net.LinkAddress; import android.net.LinkProperties; import android.net.NetworkAgent; import android.net.NetworkCapabilities; @@ -60,8 +61,11 @@ import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import java.io.IOException; +import java.net.InetAddress; import java.net.UnknownHostException; +import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.function.Consumer; /** Tests for VcnGatewayConnection.ConnectedState */ @@ -169,12 +173,14 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection } private void triggerChildOpened() { + triggerChildOpened(Collections.singletonList(TEST_INTERNAL_ADDR), TEST_DNS_ADDR); + } + + private void triggerChildOpened(List internalAddresses, InetAddress dnsAddress) { final VcnChildSessionConfiguration mMockChildSessionConfig = mock(VcnChildSessionConfiguration.class); - doReturn(Collections.singletonList(TEST_INTERNAL_ADDR)) - .when(mMockChildSessionConfig) - .getInternalAddresses(); - doReturn(Collections.singletonList(TEST_DNS_ADDR)) + doReturn(internalAddresses).when(mMockChildSessionConfig).getInternalAddresses(); + doReturn(Collections.singletonList(dnsAddress)) .when(mMockChildSessionConfig) .getInternalDnsServers(); @@ -248,6 +254,59 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection assertFalse(mGatewayConnection.isInSafeMode()); } + @Test + public void testInternalAndDnsAddressesChanged() throws Exception { + final List startingInternalAddrs = + Arrays.asList(new LinkAddress[] {TEST_INTERNAL_ADDR, TEST_INTERNAL_ADDR_2}); + triggerChildOpened(startingInternalAddrs, TEST_DNS_ADDR); + mTestLooper.dispatchAll(); + + for (LinkAddress addr : startingInternalAddrs) { + verify(mIpSecSvc) + .addAddressToTunnelInterface( + eq(TEST_IPSEC_TUNNEL_RESOURCE_ID), eq(addr), any()); + } + + verify(mDeps) + .newNetworkAgent( + any(), + any(), + any(), + argThat( + lp -> + startingInternalAddrs.equals(lp.getLinkAddresses()) + && Collections.singletonList(TEST_DNS_ADDR) + .equals(lp.getDnsServers())), + anyInt(), + any(), + any(), + any(), + any()); + + // Trigger another connection event, and verify that the addresses change + final List newInternalAddrs = + Arrays.asList(new LinkAddress[] {TEST_INTERNAL_ADDR_2, TEST_INTERNAL_ADDR_3}); + triggerChildOpened(newInternalAddrs, TEST_DNS_ADDR_2); + mTestLooper.dispatchAll(); + + // Verify addresses on tunnel network added/removed + for (LinkAddress addr : newInternalAddrs) { + verify(mIpSecSvc) + .addAddressToTunnelInterface( + eq(TEST_IPSEC_TUNNEL_RESOURCE_ID), eq(addr), any()); + } + verify(mIpSecSvc) + .removeAddressFromTunnelInterface( + eq(TEST_IPSEC_TUNNEL_RESOURCE_ID), eq(TEST_INTERNAL_ADDR), any()); + + // TODO(b/184579891): Also verify link properties updated and sent when sendLinkProperties + // is mockable + + // Verify that IpSecTunnelInterface only created once + verify(mIpSecSvc).createTunnelInterface(any(), any(), any(), any(), any()); + verifyNoMoreInteractions(mIpSecSvc); + } + @Test public void testSuccessfulConnectionExitsSafeMode() throws Exception { verifySafeModeTimeoutNotifiesCallbackAndUnregistersNetworkAgent( diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java index 884b2338d877e..c5ed8f6ddcc7c 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java @@ -74,8 +74,14 @@ public class VcnGatewayConnectionTestBase { protected static final ParcelUuid TEST_SUB_GRP = new ParcelUuid(UUID.randomUUID()); protected static final InetAddress TEST_DNS_ADDR = InetAddresses.parseNumericAddress("2001:DB8:0:1::"); + protected static final InetAddress TEST_DNS_ADDR_2 = + InetAddresses.parseNumericAddress("2001:DB8:0:2::"); protected static final LinkAddress TEST_INTERNAL_ADDR = - new LinkAddress(InetAddresses.parseNumericAddress("2001:DB8:0:2::"), 64); + new LinkAddress(InetAddresses.parseNumericAddress("2001:DB8:1:1::"), 64); + protected static final LinkAddress TEST_INTERNAL_ADDR_2 = + new LinkAddress(InetAddresses.parseNumericAddress("2001:DB8:1:2::"), 64); + protected static final LinkAddress TEST_INTERNAL_ADDR_3 = + new LinkAddress(InetAddresses.parseNumericAddress("2001:DB8:1:3::"), 64); protected static final int TEST_IPSEC_SPI_VALUE = 0x1234; protected static final int TEST_IPSEC_SPI_RESOURCE_ID = 1; From 0906d6bc1b339096149323e9d781a5fe4e591569 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Mon, 5 Apr 2021 15:01:13 -0700 Subject: [PATCH 5/5] Don't process dup unwanted() when unregistering NetworkAgent In cases where the NetworkAgent is unregistered (eg. Safe mode), but the VCN is not shut down, the NetworkAgent.unwanted() call should not trigger a teardown. Test: atest FrameworksVcnTests Change-Id: Icf32cb464bce2aae2846448d5a6a53a97f558398 --- .../java/com/android/server/vcn/VcnGatewayConnection.java | 8 +++++++- .../vcn/VcnGatewayConnectionConnectedStateTest.java | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java index efe78377575e8..20c08eb2ce928 100644 --- a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java +++ b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java @@ -1474,7 +1474,13 @@ public class VcnGatewayConnection extends StateMachine { mVcnContext.getVcnNetworkProvider(), () -> { Slog.d(TAG, "NetworkAgent was unwanted"); - teardownAsynchronously(); + // If network agent has already been torn down, skip sending the + // disconnect. Unwanted() is always called, even when networkAgents + // are unregistered in teardownNetwork(), so prevent duplicate + // notifications. + if (mNetworkAgent != null) { + teardownAsynchronously(); + } } /* networkUnwantedCallback */, (status) -> { if (status == NetworkAgent.VALIDATION_STATUS_VALID) { diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java index 54086c2a332f6..34c00182f855a 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java @@ -312,6 +312,9 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection verifySafeModeTimeoutNotifiesCallbackAndUnregistersNetworkAgent( mGatewayConnection.mConnectedState); + assertTrue(mGatewayConnection.isInSafeMode()); + assertFalse(mGatewayConnection.isQuitting()); + triggerChildOpened(); mTestLooper.dispatchAll();