From fe0523949bfbf4ccf023ff388933c346406cce83 Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Tue, 9 Feb 2021 18:40:27 -0800 Subject: [PATCH 1/3] Define VcnStatusCallback#onGatewayConnectionError callback. This CL updates VcnStatusCallback to have a onGatewayConnctionError() function. This function will be used to notify callers when Gateway Connection errors occur, such as authentication failures or the session unexpectedly dying. Bug: 163433613 Test: atest FrameworksVcnTests Change-Id: I9ea6e9e850eb9d8b0d374c5447895f1f04121696 --- .../android/net/vcn/IVcnStatusCallback.aidl | 5 + core/java/android/net/vcn/VcnManager.java | 94 ++++++++++++++++++- .../java/android/net/vcn/VcnManagerTest.java | 34 +++++-- 3 files changed, 124 insertions(+), 9 deletions(-) diff --git a/core/java/android/net/vcn/IVcnStatusCallback.aidl b/core/java/android/net/vcn/IVcnStatusCallback.aidl index a7386718d5ae5..555e9b5883e83 100644 --- a/core/java/android/net/vcn/IVcnStatusCallback.aidl +++ b/core/java/android/net/vcn/IVcnStatusCallback.aidl @@ -19,4 +19,9 @@ package android.net.vcn; /** @hide */ interface IVcnStatusCallback { void onEnteredSafeMode(); + void onGatewayConnectionError( + in int[] gatewayNetworkCapabilities, + int errorCode, + in String exceptionClass, + in String exceptionMessage); } \ No newline at end of file diff --git a/core/java/android/net/vcn/VcnManager.java b/core/java/android/net/vcn/VcnManager.java index aed64de52cd02..aea0ea988f509 100644 --- a/core/java/android/net/vcn/VcnManager.java +++ b/core/java/android/net/vcn/VcnManager.java @@ -17,7 +17,9 @@ package android.net.vcn; import static java.util.Objects.requireNonNull; +import android.annotation.IntDef; import android.annotation.NonNull; +import android.annotation.Nullable; import android.annotation.RequiresPermission; import android.annotation.SystemService; import android.content.Context; @@ -32,6 +34,8 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.internal.annotations.VisibleForTesting.Visibility; import java.io.IOException; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.Collections; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -262,6 +266,42 @@ public class VcnManager { } } + /** @hide */ + @Retention(RetentionPolicy.SOURCE) + @IntDef({ + VCN_ERROR_CODE_INTERNAL_ERROR, + VCN_ERROR_CODE_CONFIG_ERROR, + VCN_ERROR_CODE_NETWORK_ERROR + }) + public @interface VcnErrorCode {} + + /** + * Value indicating that an internal failure occurred in this Gateway Connection. + * + * @hide + */ + public static final int VCN_ERROR_CODE_INTERNAL_ERROR = 0; + + /** + * Value indicating that an error with this Gateway Connection's configuration occurred. + * + *

For example, this error code will be returned after authentication failures. + * + * @hide + */ + public static final int VCN_ERROR_CODE_CONFIG_ERROR = 1; + + /** + * Value indicating that a Network error occurred with this Gateway Connection. + * + *

For example, this error code will be returned if an underlying {@link android.net.Network} + * for this Gateway Connection is lost, or if an error occurs while resolving the connection + * endpoint address. + * + * @hide + */ + public static final int VCN_ERROR_CODE_NETWORK_ERROR = 2; + // TODO: make VcnStatusCallback @SystemApi /** * VcnStatusCallback is the interface for Carrier apps to receive updates for their VCNs. @@ -285,6 +325,24 @@ public class VcnManager { * via {@link #setVcnConfig(ParcelUuid, VcnConfig)}. */ public abstract void onEnteredSafeMode(); + + /** + * Invoked when a VCN Gateway Connection corresponding to this callback's subscription + * encounters an error. + * + * @param networkCapabilities an array of underlying NetworkCapabilities for the Gateway + * Connection that encountered the error for identification purposes. These will be a + * sorted list with no duplicates, matching one of the {@link + * VcnGatewayConnectionConfig}s set in the {@link VcnConfig} for this subscription + * group. + * @param errorCode {@link VcnErrorCode} to indicate the error that occurred + * @param detail Throwable to provide additional information about the error, or {@code + * null} if none + */ + public abstract void onGatewayConnectionError( + @NonNull int[] networkCapabilities, + @VcnErrorCode int errorCode, + @Nullable Throwable detail); } /** @@ -385,11 +443,12 @@ public class VcnManager { * * @hide */ - private class VcnStatusCallbackBinder extends IVcnStatusCallback.Stub { + @VisibleForTesting(visibility = Visibility.PRIVATE) + public static class VcnStatusCallbackBinder extends IVcnStatusCallback.Stub { @NonNull private final Executor mExecutor; @NonNull private final VcnStatusCallback mCallback; - private VcnStatusCallbackBinder( + public VcnStatusCallbackBinder( @NonNull Executor executor, @NonNull VcnStatusCallback callback) { mExecutor = executor; mCallback = callback; @@ -400,5 +459,36 @@ public class VcnManager { Binder.withCleanCallingIdentity( () -> mExecutor.execute(() -> mCallback.onEnteredSafeMode())); } + + // TODO(b/180521637): use ServiceSpecificException for safer Exception 'parceling' + @Override + public void onGatewayConnectionError( + @NonNull int[] networkCapabilities, + @VcnErrorCode int errorCode, + @Nullable String exceptionClass, + @Nullable String exceptionMessage) { + final Throwable cause = createThrowableByClassName(exceptionClass, exceptionMessage); + + Binder.withCleanCallingIdentity( + () -> + mExecutor.execute( + () -> + mCallback.onGatewayConnectionError( + networkCapabilities, errorCode, cause))); + } + + private static Throwable createThrowableByClassName( + @Nullable String className, @Nullable String message) { + if (className == null) { + return null; + } + + try { + Class c = Class.forName(className); + return (Throwable) c.getConstructor(String.class).newInstance(message); + } catch (ReflectiveOperationException | ClassCastException e) { + return new RuntimeException(className + ": " + message); + } + } } } diff --git a/tests/vcn/java/android/net/vcn/VcnManagerTest.java b/tests/vcn/java/android/net/vcn/VcnManagerTest.java index 1a90fc319bce2..708767605508b 100644 --- a/tests/vcn/java/android/net/vcn/VcnManagerTest.java +++ b/tests/vcn/java/android/net/vcn/VcnManagerTest.java @@ -22,6 +22,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.notNull; import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -33,6 +34,7 @@ import android.content.Context; import android.net.LinkProperties; import android.net.NetworkCapabilities; import android.net.vcn.VcnManager.VcnStatusCallback; +import android.net.vcn.VcnManager.VcnStatusCallbackBinder; import android.net.vcn.VcnManager.VcnUnderlyingNetworkPolicyListener; import android.os.ParcelUuid; @@ -40,11 +42,15 @@ import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; +import java.net.UnknownHostException; import java.util.UUID; import java.util.concurrent.Executor; public class VcnManagerTest { private static final ParcelUuid SUB_GROUP = new ParcelUuid(new UUID(0, 0)); + private static final int[] UNDERLYING_NETWORK_CAPABILITIES = { + NetworkCapabilities.NET_CAPABILITY_IMS, NetworkCapabilities.NET_CAPABILITY_INTERNET + }; private static final Executor INLINE_EXECUTOR = Runnable::run; private IVcnManagementService mMockVcnManagementService; @@ -144,14 +150,8 @@ public class VcnManagerTest { public void testRegisterVcnStatusCallback() throws Exception { mVcnManager.registerVcnStatusCallback(SUB_GROUP, INLINE_EXECUTOR, mMockStatusCallback); - ArgumentCaptor captor = - ArgumentCaptor.forClass(IVcnStatusCallback.class); verify(mMockVcnManagementService) - .registerVcnStatusCallback(eq(SUB_GROUP), captor.capture(), any()); - - IVcnStatusCallback callbackWrapper = captor.getValue(); - callbackWrapper.onEnteredSafeMode(); - verify(mMockStatusCallback).onEnteredSafeMode(); + .registerVcnStatusCallback(eq(SUB_GROUP), notNull(), any()); } @Test(expected = IllegalStateException.class) @@ -195,4 +195,24 @@ public class VcnManagerTest { public void testUnregisterNullVcnStatusCallback() throws Exception { mVcnManager.unregisterVcnStatusCallback(null); } + + @Test + public void testVcnStatusCallbackBinder() throws Exception { + IVcnStatusCallback cbBinder = + new VcnStatusCallbackBinder(INLINE_EXECUTOR, mMockStatusCallback); + + cbBinder.onEnteredSafeMode(); + verify(mMockStatusCallback).onEnteredSafeMode(); + + cbBinder.onGatewayConnectionError( + UNDERLYING_NETWORK_CAPABILITIES, + VcnManager.VCN_ERROR_CODE_NETWORK_ERROR, + "java.net.UnknownHostException", + "exception_message"); + verify(mMockStatusCallback) + .onGatewayConnectionError( + eq(UNDERLYING_NETWORK_CAPABILITIES), + eq(VcnManager.VCN_ERROR_CODE_NETWORK_ERROR), + any(UnknownHostException.class)); + } } From a65c2d8ad144e8e38a62721a5907de6682dc9ea9 Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Fri, 12 Feb 2021 15:41:34 -0800 Subject: [PATCH 2/3] Implement VCN error callback use. This CL updates VcnGatewayConnection to notify VcnManagementService (through Vcn) when errors occur with a gateway. VcnManagementService then notifies registered, permissioned VcnStatusCallbacks via Bug: 163433613 Test: atest FrameworksVcnTests Change-Id: I3be3cac4b591b19a0b0075767fde0ba2eb6e12a2 --- .../android/server/VcnManagementService.java | 89 +++++++++++++------ .../core/java/com/android/server/vcn/Vcn.java | 40 ++++++--- .../server/vcn/VcnGatewayConnection.java | 66 +++++++++++++- .../server/VcnManagementServiceTest.java | 20 ++--- ...cnGatewayConnectionConnectedStateTest.java | 66 ++++++++++++++ .../java/com/android/server/vcn/VcnTest.java | 10 +-- 6 files changed, 235 insertions(+), 56 deletions(-) diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java index ed4e1d951b431..329ab9983c909 100644 --- a/services/core/java/com/android/server/VcnManagementService.java +++ b/services/core/java/com/android/server/VcnManagementService.java @@ -33,6 +33,7 @@ import android.net.vcn.IVcnManagementService; import android.net.vcn.IVcnStatusCallback; import android.net.vcn.IVcnUnderlyingNetworkPolicyListener; import android.net.vcn.VcnConfig; +import android.net.vcn.VcnManager.VcnErrorCode; import android.net.vcn.VcnUnderlyingNetworkPolicy; import android.net.wifi.WifiInfo; import android.os.Binder; @@ -304,8 +305,8 @@ public class VcnManagementService extends IVcnManagementService.Stub { @NonNull ParcelUuid subscriptionGroup, @NonNull VcnConfig config, @NonNull TelephonySubscriptionSnapshot snapshot, - @NonNull VcnSafeModeCallback safeModeCallback) { - return new Vcn(vcnContext, subscriptionGroup, config, snapshot, safeModeCallback); + @NonNull VcnCallback vcnCallback) { + return new Vcn(vcnContext, subscriptionGroup, config, snapshot, vcnCallback); } /** Gets the subId indicated by the given {@link WifiInfo}. */ @@ -457,12 +458,10 @@ public class VcnManagementService extends IVcnManagementService.Stub { // TODO(b/176939047): Support multiple VCNs active at the same time, or limit to one active // VCN. - final VcnSafeModeCallbackImpl safeModeCallback = - new VcnSafeModeCallbackImpl(subscriptionGroup); + final VcnCallbackImpl vcnCallback = new VcnCallbackImpl(subscriptionGroup); final Vcn newInstance = - mDeps.newVcn( - mVcnContext, subscriptionGroup, config, mLastSnapshot, safeModeCallback); + mDeps.newVcn(mVcnContext, subscriptionGroup, config, mLastSnapshot, vcnCallback); mVcns.put(subscriptionGroup, newInstance); // Now that a new VCN has started, notify all registered listeners to refresh their @@ -784,20 +783,47 @@ public class VcnManagementService extends IVcnManagementService.Stub { } // TODO(b/180452282): Make name more generic and implement directly with VcnManagementService - /** Callback for signalling when a Vcn has entered safe mode. */ - public interface VcnSafeModeCallback { + /** 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 an error occurred. */ + void onGatewayConnectionError( + @NonNull int[] networkCapabilities, + @VcnErrorCode int errorCode, + @Nullable String exceptionClass, + @Nullable String exceptionMessage); } - /** VcnSafeModeCallback is used by Vcns to notify VcnManagementService on entering safe mode. */ - private class VcnSafeModeCallbackImpl implements VcnSafeModeCallback { + /** VcnCallbackImpl for Vcn signals sent up to VcnManagementService. */ + private class VcnCallbackImpl implements VcnCallback { @NonNull private final ParcelUuid mSubGroup; - private VcnSafeModeCallbackImpl(@NonNull final ParcelUuid subGroup) { + private VcnCallbackImpl(@NonNull final ParcelUuid subGroup) { mSubGroup = Objects.requireNonNull(subGroup, "Missing subGroup"); } + private boolean isCallbackPermissioned(@NonNull VcnStatusCallbackInfo cbInfo) { + if (!mSubGroup.equals(cbInfo.mSubGroup)) { + return false; + } + + if (!mLastSnapshot.packageHasPermissionsForSubscriptionGroup( + mSubGroup, cbInfo.mPkgName)) { + return false; + } + + if (!mLocationPermissionChecker.checkLocationPermission( + cbInfo.mPkgName, + "VcnStatusCallback" /* featureId */, + cbInfo.mUid, + null /* message */)) { + return false; + } + return true; + } + @Override public void onEnteredSafeMode() { synchronized (mLock) { @@ -810,23 +836,36 @@ public class VcnManagementService extends IVcnManagementService.Stub { // Notify all registered StatusCallbacks for this subGroup for (VcnStatusCallbackInfo cbInfo : mRegisteredStatusCallbacks.values()) { - if (!mSubGroup.equals(cbInfo.mSubGroup)) { - continue; - } - if (!mLastSnapshot.packageHasPermissionsForSubscriptionGroup( - mSubGroup, cbInfo.mPkgName)) { - continue; + if (isCallbackPermissioned(cbInfo)) { + Binder.withCleanCallingIdentity(() -> cbInfo.mCallback.onEnteredSafeMode()); } + } + } + } - if (!mLocationPermissionChecker.checkLocationPermission( - cbInfo.mPkgName, - "VcnStatusCallback" /* featureId */, - cbInfo.mUid, - null /* message */)) { - continue; - } + @Override + public void onGatewayConnectionError( + @NonNull int[] networkCapabilities, + @VcnErrorCode int errorCode, + @Nullable String exceptionClass, + @Nullable String exceptionMessage) { + synchronized (mLock) { + // Ignore if this subscription group doesn't exist anymore + if (!mVcns.containsKey(mSubGroup)) { + return; + } - Binder.withCleanCallingIdentity(() -> cbInfo.mCallback.onEnteredSafeMode()); + // Notify all registered StatusCallbacks for this subGroup + for (VcnStatusCallbackInfo cbInfo : mRegisteredStatusCallbacks.values()) { + if (isCallbackPermissioned(cbInfo)) { + Binder.withCleanCallingIdentity( + () -> + cbInfo.mCallback.onGatewayConnectionError( + networkCapabilities, + errorCode, + exceptionClass, + exceptionMessage)); + } } } } diff --git a/services/core/java/com/android/server/vcn/Vcn.java b/services/core/java/com/android/server/vcn/Vcn.java index 02a597e9aa693..6ad30b5442577 100644 --- a/services/core/java/com/android/server/vcn/Vcn.java +++ b/services/core/java/com/android/server/vcn/Vcn.java @@ -19,10 +19,12 @@ package com.android.server.vcn; import static com.android.server.VcnManagementService.VDBG; import android.annotation.NonNull; +import android.annotation.Nullable; import android.net.NetworkCapabilities; import android.net.NetworkRequest; import android.net.vcn.VcnConfig; import android.net.vcn.VcnGatewayConnectionConfig; +import android.net.vcn.VcnManager.VcnErrorCode; import android.os.Handler; import android.os.Message; import android.os.ParcelUuid; @@ -30,7 +32,7 @@ import android.util.Slog; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.annotations.VisibleForTesting.Visibility; -import com.android.server.VcnManagementService.VcnSafeModeCallback; +import com.android.server.VcnManagementService.VcnCallback; import com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscriptionSnapshot; import java.util.Collections; @@ -97,7 +99,7 @@ public class Vcn extends Handler { @NonNull private final ParcelUuid mSubscriptionGroup; @NonNull private final Dependencies mDeps; @NonNull private final VcnNetworkRequestListener mRequestListener; - @NonNull private final VcnSafeModeCallback mVcnSafeModeCallback; + @NonNull private final VcnCallback mVcnCallback; @NonNull private final Map mVcnGatewayConnections = @@ -125,14 +127,8 @@ public class Vcn extends Handler { @NonNull ParcelUuid subscriptionGroup, @NonNull VcnConfig config, @NonNull TelephonySubscriptionSnapshot snapshot, - @NonNull VcnSafeModeCallback vcnSafeModeCallback) { - this( - vcnContext, - subscriptionGroup, - config, - snapshot, - vcnSafeModeCallback, - new Dependencies()); + @NonNull VcnCallback vcnCallback) { + this(vcnContext, subscriptionGroup, config, snapshot, vcnCallback, new Dependencies()); } @VisibleForTesting(visibility = Visibility.PRIVATE) @@ -141,13 +137,12 @@ public class Vcn extends Handler { @NonNull ParcelUuid subscriptionGroup, @NonNull VcnConfig config, @NonNull TelephonySubscriptionSnapshot snapshot, - @NonNull VcnSafeModeCallback vcnSafeModeCallback, + @NonNull VcnCallback vcnCallback, @NonNull Dependencies deps) { super(Objects.requireNonNull(vcnContext, "Missing vcnContext").getLooper()); mVcnContext = vcnContext; mSubscriptionGroup = Objects.requireNonNull(subscriptionGroup, "Missing subscriptionGroup"); - mVcnSafeModeCallback = - Objects.requireNonNull(vcnSafeModeCallback, "Missing vcnSafeModeCallback"); + mVcnCallback = Objects.requireNonNull(vcnCallback, "Missing vcnCallback"); mDeps = Objects.requireNonNull(deps, "Missing deps"); mRequestListener = new VcnNetworkRequestListener(); @@ -246,7 +241,7 @@ public class Vcn extends Handler { private void handleEnterSafeMode() { handleTeardown(); - mVcnSafeModeCallback.onEnteredSafeMode(); + mVcnCallback.onEnteredSafeMode(); } private void handleNetworkRequested( @@ -337,6 +332,13 @@ public class Vcn extends Handler { public interface VcnGatewayStatusCallback { /** Called by a VcnGatewayConnection to indicate that it has entered safe mode. */ void onEnteredSafeMode(); + + /** Callback by a VcnGatewayConnection to indicate that an error occurred. */ + void onGatewayConnectionError( + @NonNull int[] networkCapabilities, + @VcnErrorCode int errorCode, + @Nullable String exceptionClass, + @Nullable String exceptionMessage); } private class VcnGatewayStatusCallbackImpl implements VcnGatewayStatusCallback { @@ -344,6 +346,16 @@ public class Vcn extends Handler { public void onEnteredSafeMode() { sendMessage(obtainMessage(MSG_CMD_ENTER_SAFE_MODE)); } + + @Override + public void onGatewayConnectionError( + @NonNull int[] networkCapabilities, + @VcnErrorCode int errorCode, + @Nullable String exceptionClass, + @Nullable String exceptionMessage) { + mVcnCallback.onGatewayConnectionError( + networkCapabilities, errorCode, exceptionClass, exceptionMessage); + } } /** External dependencies used by Vcn, for injection in tests */ diff --git a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java index 59cb6aca76687..8f2f56aa074d5 100644 --- a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java +++ b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java @@ -22,6 +22,9 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; +import static android.net.vcn.VcnManager.VCN_ERROR_CODE_CONFIG_ERROR; +import static android.net.vcn.VcnManager.VCN_ERROR_CODE_INTERNAL_ERROR; +import static android.net.vcn.VcnManager.VCN_ERROR_CODE_NETWORK_ERROR; import static com.android.server.VcnManagementService.VDBG; @@ -52,7 +55,9 @@ import android.net.ipsec.ike.IkeSession; import android.net.ipsec.ike.IkeSessionCallback; import android.net.ipsec.ike.IkeSessionConfiguration; import android.net.ipsec.ike.IkeSessionParams; +import android.net.ipsec.ike.exceptions.AuthenticationFailedException; import android.net.ipsec.ike.exceptions.IkeException; +import android.net.ipsec.ike.exceptions.IkeInternalException; import android.net.ipsec.ike.exceptions.IkeProtocolException; import android.net.vcn.VcnGatewayConnectionConfig; import android.net.vcn.VcnTransportInfo; @@ -951,15 +956,68 @@ public class VcnGatewayConnection extends StateMachine { removeEqualMessages(EVENT_SAFE_MODE_TIMEOUT_EXCEEDED); } - private void sessionLost(int token, @Nullable Exception exception) { + private void sessionLostWithoutCallback(int token, @Nullable Exception exception) { sendMessageAndAcquireWakeLock( EVENT_SESSION_LOST, token, new EventSessionLostInfo(exception)); } + private void sessionLost(int token, @Nullable Exception exception) { + // Only notify mGatewayStatusCallback if the session was lost with an error. All + // authentication and DNS failures are sent through + // IkeSessionCallback.onClosedExceptionally(), which calls sessionClosed() + if (exception != null) { + mGatewayStatusCallback.onGatewayConnectionError( + mConnectionConfig.getRequiredUnderlyingCapabilities(), + VCN_ERROR_CODE_INTERNAL_ERROR, + "java.lang.RuntimeException", + "Received " + + exception.getClass().getSimpleName() + + " with message: " + + exception.getMessage()); + } + + sessionLostWithoutCallback(token, exception); + } + + private void notifyStatusCallbackForSessionClosed(@NonNull Exception exception) { + final int errorCode; + final String exceptionClass; + final String exceptionMessage; + + if (exception instanceof AuthenticationFailedException) { + errorCode = VCN_ERROR_CODE_CONFIG_ERROR; + exceptionClass = exception.getClass().getName(); + exceptionMessage = exception.getMessage(); + } else if (exception instanceof IkeInternalException + && exception.getCause() instanceof IOException) { + errorCode = VCN_ERROR_CODE_NETWORK_ERROR; + exceptionClass = "java.io.IOException"; + exceptionMessage = exception.getCause().getMessage(); + } else { + errorCode = VCN_ERROR_CODE_INTERNAL_ERROR; + exceptionClass = "java.lang.RuntimeException"; + exceptionMessage = + "Received " + + exception.getClass().getSimpleName() + + " with message: " + + exception.getMessage(); + } + + mGatewayStatusCallback.onGatewayConnectionError( + mConnectionConfig.getRequiredUnderlyingCapabilities(), + errorCode, + exceptionClass, + exceptionMessage); + } + private void sessionClosed(int token, @Nullable Exception exception) { + if (exception != null) { + notifyStatusCallbackForSessionClosed(exception); + } + // SESSION_LOST MUST be sent before SESSION_CLOSED to ensure that the SM moves to the // Disconnecting state. - sessionLost(token, exception); + sessionLostWithoutCallback(token, exception); sendMessageAndAcquireWakeLock(EVENT_SESSION_CLOSED, token); } @@ -1084,6 +1142,8 @@ public class VcnGatewayConnection extends StateMachine { } protected void handleDisconnectRequested(String msg) { + // TODO(b/180526152): notify VcnStatusCallback for Network loss + Slog.v(TAG, "Tearing down. Cause: " + msg); mIsRunning = false; @@ -1228,6 +1288,8 @@ public class VcnGatewayConnection extends StateMachine { String reason = ((EventDisconnectRequestedInfo) msg.obj).reason; if (reason.equals(DISCONNECT_REASON_UNDERLYING_NETWORK_LOST)) { + // TODO(b/180526152): notify VcnStatusCallback for Network loss + // Will trigger EVENT_SESSION_CLOSED immediately. mIkeSession.kill(); break; diff --git a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java index 124ec3056fb22..45b2381ce06dc 100644 --- a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java +++ b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java @@ -75,7 +75,7 @@ import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; import com.android.internal.util.LocationPermissionChecker; -import com.android.server.VcnManagementService.VcnSafeModeCallback; +import com.android.server.VcnManagementService.VcnCallback; import com.android.server.VcnManagementService.VcnStatusCallbackInfo; import com.android.server.vcn.TelephonySubscriptionTracker; import com.android.server.vcn.Vcn; @@ -156,8 +156,8 @@ public class VcnManagementServiceTest { private final LocationPermissionChecker mLocationPermissionChecker = mock(LocationPermissionChecker.class); - private final ArgumentCaptor mSafeModeCallbackCaptor = - ArgumentCaptor.forClass(VcnSafeModeCallback.class); + private final ArgumentCaptor mVcnCallbackCaptor = + ArgumentCaptor.forClass(VcnCallback.class); private final VcnManagementService mVcnMgmtSvc; @@ -721,7 +721,7 @@ public class VcnManagementServiceTest { verify(mMockPolicyListener).onPolicyChanged(); } - private void verifyVcnSafeModeCallback( + private void verifyVcnCallback( @NonNull ParcelUuid subGroup, @NonNull TelephonySubscriptionSnapshot snapshot) throws Exception { verify(mMockDeps) @@ -730,22 +730,22 @@ public class VcnManagementServiceTest { eq(subGroup), eq(TEST_VCN_CONFIG), eq(snapshot), - mSafeModeCallbackCaptor.capture()); + mVcnCallbackCaptor.capture()); mVcnMgmtSvc.addVcnUnderlyingNetworkPolicyListener(mMockPolicyListener); - VcnSafeModeCallback safeModeCallback = mSafeModeCallbackCaptor.getValue(); - safeModeCallback.onEnteredSafeMode(); + VcnCallback vcnCallback = mVcnCallbackCaptor.getValue(); + vcnCallback.onEnteredSafeMode(); verify(mMockPolicyListener).onPolicyChanged(); } @Test - public void testVcnSafeModeCallbackOnEnteredSafeMode() throws Exception { + public void testVcnCallbackOnEnteredSafeMode() throws Exception { TelephonySubscriptionSnapshot snapshot = triggerSubscriptionTrackerCbAndGetSnapshot(Collections.singleton(TEST_UUID_1)); - verifyVcnSafeModeCallback(TEST_UUID_1, snapshot); + verifyVcnCallback(TEST_UUID_1, snapshot); } private void triggerVcnStatusCallbackOnEnteredSafeMode( @@ -771,7 +771,7 @@ public class VcnManagementServiceTest { // Trigger systemReady() to set up LocationPermissionChecker mVcnMgmtSvc.systemReady(); - verifyVcnSafeModeCallback(subGroup, snapshot); + verifyVcnCallback(subGroup, snapshot); } @Test diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java index b62a0b8c428a8..69c21b967917a 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java @@ -20,6 +20,9 @@ import static android.net.IpSecManager.DIRECTION_IN; import static android.net.IpSecManager.DIRECTION_OUT; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; +import static android.net.vcn.VcnManager.VCN_ERROR_CODE_CONFIG_ERROR; +import static android.net.vcn.VcnManager.VCN_ERROR_CODE_INTERNAL_ERROR; +import static android.net.vcn.VcnManager.VCN_ERROR_CODE_NETWORK_ERROR; import static com.android.server.vcn.VcnGatewayConnection.VcnChildSessionConfiguration; import static com.android.server.vcn.VcnGatewayConnection.VcnIkeSession; @@ -39,6 +42,11 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import android.net.LinkProperties; import android.net.NetworkAgent; import android.net.NetworkCapabilities; +import android.net.ipsec.ike.exceptions.AuthenticationFailedException; +import android.net.ipsec.ike.exceptions.IkeException; +import android.net.ipsec.ike.exceptions.IkeInternalException; +import android.net.ipsec.ike.exceptions.TemporaryFailureException; +import android.net.vcn.VcnManager.VcnErrorCode; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; @@ -48,6 +56,8 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import java.io.IOException; +import java.net.UnknownHostException; import java.util.Collections; /** Tests for VcnGatewayConnection.ConnectedState */ @@ -208,6 +218,25 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection // Since network never validated, verify mSafeModeTimeoutAlarm not canceled verifyNoMoreInteractions(mSafeModeTimeoutAlarm); + + // The child session was closed without exception, so verify that the GatewayStatusCallback + // was not notified + verifyNoMoreInteractions(mGatewayStatusCallback); + } + + @Test + public void testChildSessionClosedExceptionallyNotifiesGatewayStatusCallback() + throws Exception { + final IkeInternalException exception = new IkeInternalException(mock(IOException.class)); + getChildSessionCallback().onClosedExceptionally(exception); + mTestLooper.dispatchAll(); + + verify(mGatewayStatusCallback) + .onGatewayConnectionError( + eq(mConfig.getRequiredUnderlyingCapabilities()), + eq(VCN_ERROR_CODE_INTERNAL_ERROR), + any(), + any()); } @Test @@ -223,5 +252,42 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection // Since network never validated, verify mSafeModeTimeoutAlarm not canceled verifyNoMoreInteractions(mSafeModeTimeoutAlarm); + + // IkeSession closed with no error, so verify that the GatewayStatusCallback was not + // notified + verifyNoMoreInteractions(mGatewayStatusCallback); + } + + private void verifyIkeSessionClosedExceptionalltyNotifiesStatusCallback( + IkeException cause, @VcnErrorCode int expectedErrorType) { + getIkeSessionCallback().onClosedExceptionally(cause); + mTestLooper.dispatchAll(); + + verify(mIkeSession).close(); + + verify(mGatewayStatusCallback) + .onGatewayConnectionError( + eq(mConfig.getRequiredUnderlyingCapabilities()), + eq(expectedErrorType), + any(), + any()); + } + + @Test + public void testIkeSessionClosedExceptionallyAuthenticationFailure() throws Exception { + verifyIkeSessionClosedExceptionalltyNotifiesStatusCallback( + new AuthenticationFailedException("vcn test"), VCN_ERROR_CODE_CONFIG_ERROR); + } + + @Test + public void testIkeSessionClosedExceptionallyDnsFailure() throws Exception { + verifyIkeSessionClosedExceptionalltyNotifiesStatusCallback( + new IkeInternalException(new UnknownHostException()), VCN_ERROR_CODE_NETWORK_ERROR); + } + + @Test + public void testIkeSessionClosedExceptionallyInternalFailure() throws Exception { + verifyIkeSessionClosedExceptionalltyNotifiesStatusCallback( + new TemporaryFailureException("vcn test"), VCN_ERROR_CODE_INTERNAL_ERROR); } } diff --git a/tests/vcn/java/com/android/server/vcn/VcnTest.java b/tests/vcn/java/com/android/server/vcn/VcnTest.java index 8e142c095ae3a..9d33682712431 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnTest.java @@ -34,7 +34,7 @@ import android.net.vcn.VcnGatewayConnectionConfigTest; import android.os.ParcelUuid; import android.os.test.TestLooper; -import com.android.server.VcnManagementService.VcnSafeModeCallback; +import com.android.server.VcnManagementService.VcnCallback; import com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscriptionSnapshot; import com.android.server.vcn.Vcn.VcnGatewayStatusCallback; import com.android.server.vcn.VcnNetworkProvider.NetworkRequestListener; @@ -56,7 +56,7 @@ public class VcnTest { private VcnContext mVcnContext; private TelephonySubscriptionSnapshot mSubscriptionSnapshot; private VcnNetworkProvider mVcnNetworkProvider; - private VcnSafeModeCallback mVcnSafeModeCallback; + private VcnCallback mVcnCallback; private Vcn.Dependencies mDeps; private ArgumentCaptor mGatewayStatusCallbackCaptor; @@ -72,7 +72,7 @@ public class VcnTest { mVcnContext = mock(VcnContext.class); mSubscriptionSnapshot = mock(TelephonySubscriptionSnapshot.class); mVcnNetworkProvider = mock(VcnNetworkProvider.class); - mVcnSafeModeCallback = mock(VcnSafeModeCallback.class); + mVcnCallback = mock(VcnCallback.class); mDeps = mock(Vcn.Dependencies.class); mTestLooper = new TestLooper(); @@ -104,7 +104,7 @@ public class VcnTest { TEST_SUB_GROUP, mConfig, mSubscriptionSnapshot, - mVcnSafeModeCallback, + mVcnCallback, mDeps); } @@ -179,6 +179,6 @@ public class VcnTest { verify(gatewayConnection).teardownAsynchronously(); } verify(mVcnNetworkProvider).unregisterListener(requestListener); - verify(mVcnSafeModeCallback).onEnteredSafeMode(); + verify(mVcnCallback).onEnteredSafeMode(); } } From b79add4584fad617684d5553c9d667a743a26624 Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Tue, 16 Feb 2021 09:42:16 -0800 Subject: [PATCH 3/3] Correct comment for safe mode alarm in VcnGatewayConnection. Bug: 178140973 Test: atest FrameworksVcnTests Change-Id: I0cadd145fe41f5589fb79725a4370aed527927a0 --- .../core/java/com/android/server/vcn/VcnGatewayConnection.java | 3 +-- 1 file changed, 1 insertion(+), 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 8f2f56aa074d5..733d8cb558c5a 100644 --- a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java +++ b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java @@ -1635,8 +1635,7 @@ public class VcnGatewayConnection extends StateMachine { @Override protected void exitState() { - // Attempt to set the safe mode alarm - this requires the Vcn Network being validated - // while in ConnectedState (which cancels the previous alarm) + // Will only set a new alarm if no safe mode alarm is currently scheduled. setSafeModeAlarm(); } }