diff options
7 files changed, 55 insertions, 33 deletions
diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java index b48bc900aa84..81d4b9da63c8 100644 --- a/services/core/java/com/android/server/IpSecService.java +++ b/services/core/java/com/android/server/IpSecService.java @@ -48,7 +48,6 @@ import android.net.TrafficStats; import android.net.util.NetdService; import android.os.Binder; import android.os.IBinder; -import android.os.INetworkManagementService; import android.os.ParcelFileDescriptor; import android.os.RemoteException; import android.os.ServiceSpecificException; @@ -64,6 +63,7 @@ import android.util.SparseBooleanArray; 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 libcore.io.IoUtils; @@ -117,9 +117,6 @@ public class IpSecService extends IIpSecService.Stub { /* Binder context for this service */ private final Context mContext; - /* NetworkManager instance */ - private final INetworkManagementService mNetworkManager; - /** * The next non-repeating global ID for tracking resources between users, this service, and * kernel data structures. Accessing this variable is not thread safe, so it is only read or @@ -1014,13 +1011,13 @@ public class IpSecService extends IIpSecService.Stub { * * @param context Binder context for this service */ - private IpSecService(Context context, INetworkManagementService networkManager) { - this(context, networkManager, IpSecServiceConfiguration.GETSRVINSTANCE); + private IpSecService(Context context) { + this(context, IpSecServiceConfiguration.GETSRVINSTANCE); } - static IpSecService create(Context context, INetworkManagementService networkManager) + static IpSecService create(Context context) throws InterruptedException { - final IpSecService service = new IpSecService(context, networkManager); + final IpSecService service = new IpSecService(context); service.connectNativeNetdService(); return service; } @@ -1034,11 +1031,9 @@ public class IpSecService extends IIpSecService.Stub { /** @hide */ @VisibleForTesting - public IpSecService(Context context, INetworkManagementService networkManager, - IpSecServiceConfiguration config) { + public IpSecService(Context context, IpSecServiceConfiguration config) { this( context, - networkManager, config, (fd, uid) -> { try { @@ -1052,10 +1047,9 @@ public class IpSecService extends IIpSecService.Stub { /** @hide */ @VisibleForTesting - public IpSecService(Context context, INetworkManagementService networkManager, - IpSecServiceConfiguration config, UidFdTagger uidFdTagger) { + public IpSecService(Context context, IpSecServiceConfiguration config, + UidFdTagger uidFdTagger) { mContext = context; - mNetworkManager = Objects.requireNonNull(networkManager); mSrvConfig = config; mUidFdTagger = uidFdTagger; } @@ -1335,7 +1329,7 @@ public class IpSecService extends IIpSecService.Stub { netd.ipSecAddTunnelInterface(intfName, localAddr, remoteAddr, ikey, okey, resourceId); Binder.withCleanCallingIdentity(() -> { - mNetworkManager.setInterfaceUp(intfName); + NetdUtils.setInterfaceUp(netd, intfName); }); for (int selAddrFamily : ADDRESS_FAMILIES) { diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index a769e88f77d7..713ac73aa9b9 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -113,6 +113,7 @@ import com.android.internal.messages.nano.SystemMessageProto.SystemMessage; import com.android.internal.net.LegacyVpnInfo; import com.android.internal.net.VpnConfig; import com.android.internal.net.VpnProfile; +import com.android.net.module.util.NetdUtils; import com.android.net.module.util.NetworkStackConstants; import com.android.server.DeviceIdleInternal; import com.android.server.LocalServices; @@ -2528,7 +2529,7 @@ public class Vpn { address /* unused */, address /* unused */, network); - mNms.setInterfaceUp(mTunnelIface.getInterfaceName()); + NetdUtils.setInterfaceUp(mNetd, mTunnelIface.getInterfaceName()); mSession = mIkev2SessionCreator.createIkeSession( mContext, diff --git a/services/java/com/android/server/SystemServer.java b/services/java/com/android/server/SystemServer.java index c665ca32bff0..aa222cffc9a3 100644 --- a/services/java/com/android/server/SystemServer.java +++ b/services/java/com/android/server/SystemServer.java @@ -1534,7 +1534,7 @@ public final class SystemServer { t.traceBegin("StartIpSecService"); try { - ipSecService = IpSecService.create(context, networkManagement); + ipSecService = IpSecService.create(context); ServiceManager.addService(Context.IPSEC_SERVICE, ipSecService); } catch (Throwable e) { reportWtf("starting IpSec Service", e); diff --git a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java index c86224a71978..32c95f149979 100644 --- a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java +++ b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java @@ -16,12 +16,16 @@ package com.android.server; +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.system.OsConstants.AF_INET; import static android.system.OsConstants.AF_INET6; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; @@ -36,6 +40,7 @@ import android.content.pm.PackageManager; import android.net.ConnectivityManager; import android.net.INetd; import android.net.InetAddresses; +import android.net.InterfaceConfigurationParcel; import android.net.IpSecAlgorithm; import android.net.IpSecConfig; import android.net.IpSecManager; @@ -48,7 +53,6 @@ import android.net.LinkAddress; import android.net.LinkProperties; import android.net.Network; import android.os.Binder; -import android.os.INetworkManagementService; import android.os.ParcelFileDescriptor; import android.system.Os; import android.test.mock.MockContext; @@ -148,10 +152,17 @@ public class IpSecServiceParameterizedTest { } throw new SecurityException("Unavailable permission requested"); } + + @Override + public int checkCallingOrSelfPermission(String permission) { + if (android.Manifest.permission.NETWORK_STACK.equals(permission)) { + return PERMISSION_GRANTED; + } + throw new UnsupportedOperationException(); + } }; INetd mMockNetd; - INetworkManagementService mNetworkManager; PackageManager mMockPkgMgr; IpSecService.IpSecServiceConfiguration mMockIpSecSrvConfig; IpSecService mIpSecService; @@ -181,10 +192,9 @@ public class IpSecServiceParameterizedTest { @Before public void setUp() throws Exception { mMockNetd = mock(INetd.class); - mNetworkManager = mock(INetworkManagementService.class); mMockPkgMgr = mock(PackageManager.class); mMockIpSecSrvConfig = mock(IpSecService.IpSecServiceConfiguration.class); - mIpSecService = new IpSecService(mMockContext, mNetworkManager, mMockIpSecSrvConfig); + mIpSecService = new IpSecService(mMockContext, mMockIpSecSrvConfig); // Injecting mock netd when(mMockIpSecSrvConfig.getNetdInstance()).thenReturn(mMockNetd); @@ -644,7 +654,10 @@ public class IpSecServiceParameterizedTest { } private IpSecTunnelInterfaceResponse createAndValidateTunnel( - String localAddr, String remoteAddr, String pkgName) { + String localAddr, String remoteAddr, String pkgName) throws Exception { + final InterfaceConfigurationParcel config = new InterfaceConfigurationParcel(); + config.flags = new String[] {IF_STATE_DOWN}; + when(mMockNetd.interfaceGetCfg(anyString())).thenReturn(config); IpSecTunnelInterfaceResponse createTunnelResp = mIpSecService.createTunnelInterface( mSourceAddr, mDestinationAddr, fakeNetwork, new Binder(), pkgName); @@ -674,7 +687,8 @@ public class IpSecServiceParameterizedTest { anyInt(), anyInt(), anyInt()); - verify(mNetworkManager).setInterfaceUp(createTunnelResp.interfaceName); + verify(mMockNetd).interfaceSetCfg(argThat( + config -> Arrays.asList(config.flags).contains(IF_STATE_UP))); } @Test diff --git a/tests/net/java/com/android/server/IpSecServiceRefcountedResourceTest.java b/tests/net/java/com/android/server/IpSecServiceRefcountedResourceTest.java index 788e4efe097e..22a2c94fc194 100644 --- a/tests/net/java/com/android/server/IpSecServiceRefcountedResourceTest.java +++ b/tests/net/java/com/android/server/IpSecServiceRefcountedResourceTest.java @@ -31,7 +31,6 @@ import static org.mockito.Mockito.verify; import android.content.Context; import android.os.Binder; import android.os.IBinder; -import android.os.INetworkManagementService; import android.os.RemoteException; import androidx.test.filters.SmallTest; @@ -62,8 +61,7 @@ public class IpSecServiceRefcountedResourceTest { public void setUp() throws Exception { mMockContext = mock(Context.class); mMockIpSecSrvConfig = mock(IpSecService.IpSecServiceConfiguration.class); - mIpSecService = new IpSecService( - mMockContext, mock(INetworkManagementService.class), mMockIpSecSrvConfig); + mIpSecService = new IpSecService(mMockContext, mMockIpSecSrvConfig); } private void assertResourceState( diff --git a/tests/net/java/com/android/server/IpSecServiceTest.java b/tests/net/java/com/android/server/IpSecServiceTest.java index 536e98327e1f..f97eabf6366d 100644 --- a/tests/net/java/com/android/server/IpSecServiceTest.java +++ b/tests/net/java/com/android/server/IpSecServiceTest.java @@ -42,7 +42,6 @@ import android.net.IpSecManager; import android.net.IpSecSpiResponse; import android.net.IpSecUdpEncapResponse; import android.os.Binder; -import android.os.INetworkManagementService; import android.os.ParcelFileDescriptor; import android.os.Process; import android.system.ErrnoException; @@ -116,7 +115,6 @@ public class IpSecServiceTest { } Context mMockContext; - INetworkManagementService mMockNetworkManager; INetd mMockNetd; IpSecService.IpSecServiceConfiguration mMockIpSecSrvConfig; IpSecService mIpSecService; @@ -124,10 +122,9 @@ public class IpSecServiceTest { @Before public void setUp() throws Exception { mMockContext = mock(Context.class); - mMockNetworkManager = mock(INetworkManagementService.class); mMockNetd = mock(INetd.class); mMockIpSecSrvConfig = mock(IpSecService.IpSecServiceConfiguration.class); - mIpSecService = new IpSecService(mMockContext, mMockNetworkManager, mMockIpSecSrvConfig); + mIpSecService = new IpSecService(mMockContext, mMockIpSecSrvConfig); // Injecting mock netd when(mMockIpSecSrvConfig.getNetdInstance()).thenReturn(mMockNetd); @@ -135,7 +132,7 @@ public class IpSecServiceTest { @Test public void testIpSecServiceCreate() throws InterruptedException { - IpSecService ipSecSrv = IpSecService.create(mMockContext, mMockNetworkManager); + IpSecService ipSecSrv = IpSecService.create(mMockContext); assertNotNull(ipSecSrv); } @@ -608,7 +605,7 @@ public class IpSecServiceTest { public void testOpenUdpEncapSocketTagsSocket() throws Exception { IpSecService.UidFdTagger mockTagger = mock(IpSecService.UidFdTagger.class); IpSecService testIpSecService = new IpSecService( - mMockContext, mMockNetworkManager, mMockIpSecSrvConfig, mockTagger); + mMockContext, mMockIpSecSrvConfig, mockTagger); IpSecUdpEncapResponse udpEncapResp = testIpSecService.openUdpEncapsulationSocket(0, new Binder()); diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index cffd2d1d428f..6576daf5ab9a 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -21,6 +21,8 @@ import static android.content.pm.UserInfo.FLAG_MANAGED_PROFILE; import static android.content.pm.UserInfo.FLAG_PRIMARY; import static android.content.pm.UserInfo.FLAG_RESTRICTED; import static android.net.ConnectivityManager.NetworkCallback; +import static android.net.INetd.IF_STATE_DOWN; +import static android.net.INetd.IF_STATE_UP; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; @@ -62,6 +64,7 @@ import android.net.ConnectivityManager; import android.net.INetd; import android.net.Ikev2VpnProfile; import android.net.InetAddresses; +import android.net.InterfaceConfigurationParcel; import android.net.IpPrefix; import android.net.IpSecManager; import android.net.IpSecTunnelInterfaceResponse; @@ -872,17 +875,28 @@ public class VpnTest { eq(AppOpsManager.MODE_IGNORED)); } - private NetworkCallback triggerOnAvailableAndGetCallback() { + private NetworkCallback triggerOnAvailableAndGetCallback() throws Exception { final ArgumentCaptor<NetworkCallback> networkCallbackCaptor = ArgumentCaptor.forClass(NetworkCallback.class); verify(mConnectivityManager, timeout(TEST_TIMEOUT_MS)) .requestNetwork(any(), networkCallbackCaptor.capture()); + // onAvailable() will trigger onDefaultNetworkChanged(), so NetdUtils#setInterfaceUp will be + // invoked. Set the return value of INetd#interfaceGetCfg to prevent NullPointerException. + final InterfaceConfigurationParcel config = new InterfaceConfigurationParcel(); + config.flags = new String[] {IF_STATE_DOWN}; + when(mNetd.interfaceGetCfg(anyString())).thenReturn(config); final NetworkCallback cb = networkCallbackCaptor.getValue(); cb.onAvailable(TEST_NETWORK); return cb; } + private void verifyInterfaceSetCfgWithFlags(String flag) throws Exception { + // Add a timeout for waiting for interfaceSetCfg to be called. + verify(mNetd, timeout(TEST_TIMEOUT_MS)).interfaceSetCfg(argThat( + config -> Arrays.asList(config.flags).contains(flag))); + } + @Test public void testStartPlatformVpnAuthenticationFailed() throws Exception { final ArgumentCaptor<IkeSessionCallback> captor = @@ -894,6 +908,8 @@ public class VpnTest { final Vpn vpn = startLegacyVpn(createVpn(primaryUser.id), (mVpnProfile)); final NetworkCallback cb = triggerOnAvailableAndGetCallback(); + verifyInterfaceSetCfgWithFlags(IF_STATE_UP); + // Wait for createIkeSession() to be called before proceeding in order to ensure consistent // state verify(mIkev2SessionCreator, timeout(TEST_TIMEOUT_MS)) @@ -912,6 +928,8 @@ public class VpnTest { final Vpn vpn = startLegacyVpn(createVpn(primaryUser.id), mVpnProfile); final NetworkCallback cb = triggerOnAvailableAndGetCallback(); + verifyInterfaceSetCfgWithFlags(IF_STATE_UP); + // Wait for createIkeSession() to be called before proceeding in order to ensure consistent // state verify(mConnectivityManager, timeout(TEST_TIMEOUT_MS)).unregisterNetworkCallback(eq(cb)); |