diff options
6 files changed, 170 insertions, 23 deletions
diff --git a/core/java/android/net/IpSecManager.java b/core/java/android/net/IpSecManager.java index 98acd98cc465..01d1aa533a8f 100644 --- a/core/java/android/net/IpSecManager.java +++ b/core/java/android/net/IpSecManager.java @@ -79,6 +79,16 @@ public final class IpSecManager { public static final int DIRECTION_OUT = 1; /** + * Used when applying a transform to direct traffic through an {@link IpSecTransform} for + * forwarding between interfaces. + * + * <p>See {@link #applyTransportModeTransform(Socket, int, IpSecTransform)}. + * + * @hide + */ + public static final int DIRECTION_FWD = 2; + + /** * The Security Parameter Index (SPI) 0 indicates an unknown or invalid index. * * <p>No IPsec packet may contain an SPI of 0. diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java index 794cb9301d69..d574e74d398f 100644 --- a/services/core/java/com/android/server/IpSecService.java +++ b/services/core/java/com/android/server/IpSecService.java @@ -49,6 +49,7 @@ import android.net.util.NetdService; import android.os.Binder; import android.os.IBinder; import android.os.ParcelFileDescriptor; +import android.os.Process; import android.os.RemoteException; import android.os.ServiceSpecificException; import android.system.ErrnoException; @@ -65,6 +66,7 @@ import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.Preconditions; import com.android.net.module.util.NetdUtils; +import com.android.net.module.util.PermissionUtils; import libcore.io.IoUtils; @@ -466,8 +468,7 @@ public class IpSecService extends IIpSecService.Stub { /** Safety method; guards against access of other user's UserRecords */ private void checkCallerUid(int uid) { - if (uid != Binder.getCallingUid() - && android.os.Process.SYSTEM_UID != Binder.getCallingUid()) { + if (uid != Binder.getCallingUid() && Process.SYSTEM_UID != Binder.getCallingUid()) { throw new SecurityException("Attempted access of unowned resources"); } } @@ -1105,11 +1106,15 @@ public class IpSecService extends IIpSecService.Stub { * Checks the user-provided direction field and throws an IllegalArgumentException if it is not * DIRECTION_IN or DIRECTION_OUT */ - private static void checkDirection(int direction) { + private void checkDirection(int direction) { switch (direction) { case IpSecManager.DIRECTION_OUT: case IpSecManager.DIRECTION_IN: return; + case IpSecManager.DIRECTION_FWD: + // Only NETWORK_STACK or PERMISSION_NETWORK_STACK allowed to use forward policies + PermissionUtils.enforceNetworkStackPermission(mContext); + return; } throw new IllegalArgumentException("Invalid Direction: " + direction); } @@ -1353,6 +1358,16 @@ public class IpSecService extends IIpSecService.Stub { ikey, 0xffffffff, resourceId); + netd.ipSecAddSecurityPolicy( + callerUid, + selAddrFamily, + IpSecManager.DIRECTION_FWD, + remoteAddr, + localAddr, + 0, + ikey, + 0xffffffff, + resourceId); } userRecord.mTunnelInterfaceRecords.put( @@ -1820,7 +1835,7 @@ public class IpSecService extends IIpSecService.Stub { int mark = (direction == IpSecManager.DIRECTION_OUT) ? tunnelInterfaceInfo.getOkey() - : tunnelInterfaceInfo.getIkey(); + : tunnelInterfaceInfo.getIkey(); // Ikey also used for FWD policies try { // Default to using the invalid SPI of 0 for inbound SAs. This allows policies to skip diff --git a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java index 46fd22810a58..c500041838d8 100644 --- a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java +++ b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java @@ -1672,11 +1672,24 @@ public class VcnGatewayConnection extends StateMachine { @NonNull Network underlyingNetwork, @NonNull IpSecTransform transform, int direction) { + if (direction != IpSecManager.DIRECTION_IN && direction != IpSecManager.DIRECTION_OUT) { + Slog.wtf(TAG, "Applying transform for unexpected direction: " + direction); + } + try { tunnelIface.setUnderlyingNetwork(underlyingNetwork); // Transforms do not need to be persisted; the IkeSession will keep them alive mIpSecManager.applyTunnelModeTransform(tunnelIface, direction, transform); + + // For inbound transforms, additionally allow forwarded traffic to bridge to DUN (as + // needed) + final Set<Integer> exposedCaps = mConnectionConfig.getAllExposedCapabilities(); + if (direction == IpSecManager.DIRECTION_IN + && exposedCaps.contains(NET_CAPABILITY_DUN)) { + mIpSecManager.applyTunnelModeTransform( + tunnelIface, IpSecManager.DIRECTION_FWD, transform); + } } catch (IOException e) { Slog.d(TAG, "Transform application failed for network " + token, e); sessionLost(token, e); diff --git a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java index 32c95f149979..cf2c9c783ac7 100644 --- a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java +++ b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java @@ -16,9 +16,14 @@ package com.android.server; +import static android.content.pm.PackageManager.PERMISSION_DENIED; import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.net.INetd.IF_STATE_DOWN; import static android.net.INetd.IF_STATE_UP; +import static android.net.IpSecManager.DIRECTION_FWD; +import static android.net.IpSecManager.DIRECTION_IN; +import static android.net.IpSecManager.DIRECTION_OUT; +import static android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK; import static android.system.OsConstants.AF_INET; import static android.system.OsConstants.AF_INET6; @@ -56,6 +61,7 @@ import android.os.Binder; import android.os.ParcelFileDescriptor; import android.system.Os; import android.test.mock.MockContext; +import android.util.ArraySet; import androidx.test.filters.SmallTest; @@ -71,6 +77,7 @@ import java.net.Inet4Address; import java.net.Socket; import java.util.Arrays; import java.util.Collection; +import java.util.Set; /** Unit tests for {@link IpSecService}. */ @SmallTest @@ -119,7 +126,18 @@ public class IpSecServiceParameterizedTest { AppOpsManager mMockAppOps = mock(AppOpsManager.class); ConnectivityManager mMockConnectivityMgr = mock(ConnectivityManager.class); - MockContext mMockContext = new MockContext() { + TestContext mTestContext = new TestContext(); + + private class TestContext extends MockContext { + private Set<String> mAllowedPermissions = new ArraySet<>(Arrays.asList( + android.Manifest.permission.MANAGE_IPSEC_TUNNELS, + android.Manifest.permission.NETWORK_STACK, + PERMISSION_MAINLINE_NETWORK_STACK)); + + private void setAllowedPermissions(String... permissions) { + mAllowedPermissions = new ArraySet<>(permissions); + } + @Override public Object getSystemService(String name) { switch(name) { @@ -147,20 +165,22 @@ public class IpSecServiceParameterizedTest { @Override public void enforceCallingOrSelfPermission(String permission, String message) { - if (permission == android.Manifest.permission.MANAGE_IPSEC_TUNNELS) { + if (mAllowedPermissions.contains(permission)) { return; + } else { + throw new SecurityException("Unavailable permission requested"); } - throw new SecurityException("Unavailable permission requested"); } @Override public int checkCallingOrSelfPermission(String permission) { - if (android.Manifest.permission.NETWORK_STACK.equals(permission)) { + if (mAllowedPermissions.contains(permission)) { return PERMISSION_GRANTED; + } else { + return PERMISSION_DENIED; } - throw new UnsupportedOperationException(); } - }; + } INetd mMockNetd; PackageManager mMockPkgMgr; @@ -194,7 +214,7 @@ public class IpSecServiceParameterizedTest { mMockNetd = mock(INetd.class); mMockPkgMgr = mock(PackageManager.class); mMockIpSecSrvConfig = mock(IpSecService.IpSecServiceConfiguration.class); - mIpSecService = new IpSecService(mMockContext, mMockIpSecSrvConfig); + mIpSecService = new IpSecService(mTestContext, mMockIpSecSrvConfig); // Injecting mock netd when(mMockIpSecSrvConfig.getNetdInstance()).thenReturn(mMockNetd); @@ -664,6 +684,21 @@ public class IpSecServiceParameterizedTest { assertNotNull(createTunnelResp); assertEquals(IpSecManager.Status.OK, createTunnelResp.status); + for (int direction : new int[] {DIRECTION_IN, DIRECTION_OUT, DIRECTION_FWD}) { + for (int selAddrFamily : ADDRESS_FAMILIES) { + verify(mMockNetd).ipSecAddSecurityPolicy( + eq(mUid), + eq(selAddrFamily), + eq(direction), + anyString(), + anyString(), + eq(0), + anyInt(), // iKey/oKey + anyInt(), // mask + eq(createTunnelResp.resourceId)); + } + } + return createTunnelResp; } @@ -798,16 +833,51 @@ public class IpSecServiceParameterizedTest { } @Test - public void testApplyTunnelModeTransform() throws Exception { - verifyApplyTunnelModeTransformCommon(false); + public void testApplyTunnelModeTransformOutbound() throws Exception { + verifyApplyTunnelModeTransformCommon(false /* closeSpiBeforeApply */, DIRECTION_OUT); } @Test - public void testApplyTunnelModeTransformReleasedSpi() throws Exception { - verifyApplyTunnelModeTransformCommon(true); + public void testApplyTunnelModeTransformOutboundNonNetworkStack() throws Exception { + mTestContext.setAllowedPermissions(android.Manifest.permission.MANAGE_IPSEC_TUNNELS); + verifyApplyTunnelModeTransformCommon(false /* closeSpiBeforeApply */, DIRECTION_OUT); } - public void verifyApplyTunnelModeTransformCommon(boolean closeSpiBeforeApply) throws Exception { + @Test + public void testApplyTunnelModeTransformOutboundReleasedSpi() throws Exception { + verifyApplyTunnelModeTransformCommon(true /* closeSpiBeforeApply */, DIRECTION_OUT); + } + + @Test + public void testApplyTunnelModeTransformInbound() throws Exception { + verifyApplyTunnelModeTransformCommon(true /* closeSpiBeforeApply */, DIRECTION_IN); + } + + @Test + public void testApplyTunnelModeTransformInboundNonNetworkStack() throws Exception { + mTestContext.setAllowedPermissions(android.Manifest.permission.MANAGE_IPSEC_TUNNELS); + verifyApplyTunnelModeTransformCommon(true /* closeSpiBeforeApply */, DIRECTION_IN); + } + + @Test + public void testApplyTunnelModeTransformForward() throws Exception { + verifyApplyTunnelModeTransformCommon(true /* closeSpiBeforeApply */, DIRECTION_FWD); + } + + @Test + public void testApplyTunnelModeTransformForwardNonNetworkStack() throws Exception { + mTestContext.setAllowedPermissions(android.Manifest.permission.MANAGE_IPSEC_TUNNELS); + + try { + verifyApplyTunnelModeTransformCommon(true /* closeSpiBeforeApply */, DIRECTION_FWD); + fail("Expected security exception due to use of forward policies without NETWORK_STACK" + + " or MAINLINE_NETWORK_STACK permission"); + } catch (SecurityException expected) { + } + } + + public void verifyApplyTunnelModeTransformCommon(boolean closeSpiBeforeApply, int direction) + throws Exception { IpSecConfig ipSecConfig = new IpSecConfig(); ipSecConfig.setMode(IpSecTransform.MODE_TUNNEL); addDefaultSpisAndRemoteAddrToIpSecConfig(ipSecConfig); @@ -825,17 +895,17 @@ public class IpSecServiceParameterizedTest { int transformResourceId = createTransformResp.resourceId; int tunnelResourceId = createTunnelResp.resourceId; mIpSecService.applyTunnelModeTransform( - tunnelResourceId, IpSecManager.DIRECTION_OUT, transformResourceId, BLESSED_PACKAGE); + tunnelResourceId, direction, transformResourceId, BLESSED_PACKAGE); for (int selAddrFamily : ADDRESS_FAMILIES) { verify(mMockNetd) .ipSecUpdateSecurityPolicy( eq(mUid), eq(selAddrFamily), - eq(IpSecManager.DIRECTION_OUT), + eq(direction), anyString(), anyString(), - eq(TEST_SPI), + eq(direction == DIRECTION_OUT ? TEST_SPI : 0), anyInt(), // iKey/oKey anyInt(), // mask eq(tunnelResourceId)); diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java index eedaac48293c..39f7386cbb76 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java @@ -16,8 +16,11 @@ package com.android.server.vcn; +import static android.net.IpSecManager.DIRECTION_FWD; import static android.net.IpSecManager.DIRECTION_IN; import static android.net.IpSecManager.DIRECTION_OUT; +import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; +import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import static android.net.ipsec.ike.exceptions.IkeProtocolException.ERROR_TYPE_AUTHENTICATION_FAILED; @@ -54,6 +57,8 @@ import android.net.ipsec.ike.ChildSaProposal; 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.VcnGatewayConnectionConfigTest; import android.net.vcn.VcnManager.VcnErrorCode; import androidx.test.filters.SmallTest; @@ -143,8 +148,9 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection assertEquals(mGatewayConnection.mConnectedState, mGatewayConnection.getCurrentState()); } - @Test - public void testCreatedTransformsAreApplied() throws Exception { + private void verifyVcnTransformsApplied( + VcnGatewayConnection vcnGatewayConnection, boolean expectForwardTransform) + throws Exception { for (int direction : new int[] {DIRECTION_IN, DIRECTION_OUT}) { getChildSessionCallback().onIpSecTransformCreated(makeDummyIpSecTransform(), direction); mTestLooper.dispatchAll(); @@ -154,7 +160,40 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection eq(TEST_IPSEC_TUNNEL_RESOURCE_ID), eq(direction), anyInt(), any()); } - assertEquals(mGatewayConnection.mConnectedState, mGatewayConnection.getCurrentState()); + verify(mIpSecSvc, expectForwardTransform ? times(1) : never()) + .applyTunnelModeTransform( + eq(TEST_IPSEC_TUNNEL_RESOURCE_ID), eq(DIRECTION_FWD), anyInt(), any()); + + assertEquals(vcnGatewayConnection.mConnectedState, vcnGatewayConnection.getCurrentState()); + } + + @Test + public void testCreatedTransformsAreApplied() throws Exception { + verifyVcnTransformsApplied(mGatewayConnection, false /* expectForwardTransform */); + } + + @Test + public void testCreatedTransformsAreAppliedWithDun() throws Exception { + VcnGatewayConnectionConfig gatewayConfig = + VcnGatewayConnectionConfigTest.buildTestConfigWithExposedCaps( + NET_CAPABILITY_INTERNET, NET_CAPABILITY_DUN); + VcnGatewayConnection gatewayConnection = + new VcnGatewayConnection( + mVcnContext, + TEST_SUB_GRP, + TEST_SUBSCRIPTION_SNAPSHOT, + gatewayConfig, + mGatewayStatusCallback, + true /* isMobileDataEnabled */, + mDeps); + gatewayConnection.setUnderlyingNetwork(TEST_UNDERLYING_NETWORK_RECORD_1); + final VcnIkeSession session = + gatewayConnection.buildIkeSession(TEST_UNDERLYING_NETWORK_RECORD_1.network); + gatewayConnection.setIkeSession(session); + gatewayConnection.transitionTo(gatewayConnection.mConnectedState); + mTestLooper.dispatchAll(); + + verifyVcnTransformsApplied(gatewayConnection, true /* expectForwardTransform */); } @Test diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java index 284f1f88658e..1ecb4c9ee298 100644 --- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java +++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java @@ -220,7 +220,7 @@ public class VcnGatewayConnectionTestBase { protected VcnChildSessionCallback getChildSessionCallback() { ArgumentCaptor<ChildSessionCallback> captor = ArgumentCaptor.forClass(ChildSessionCallback.class); - verify(mDeps).newIkeSession(any(), any(), any(), any(), captor.capture()); + verify(mDeps, atLeastOnce()).newIkeSession(any(), any(), any(), any(), captor.capture()); return (VcnChildSessionCallback) captor.getValue(); } |