Migrate VPN to the public NetworkAgent API.
On top of being a cleanup this is useful for the S Network
Selection project that will need to enrich the Network
Agent API, and as such should not have to support legacy
agents.
Test: FrameworksNetTests NetworkStackTests
Bug: 167544279
Change-Id: Id3e5f6e19829c64074cd6a52c5f950cee56b860b
diff --git a/core/java/android/net/NetworkProvider.java b/core/java/android/net/NetworkProvider.java
index d31218d..a17a498 100644
--- a/core/java/android/net/NetworkProvider.java
+++ b/core/java/android/net/NetworkProvider.java
@@ -51,13 +51,6 @@
public static final int ID_NONE = -1;
/**
- * A hardcoded ID for NetworkAgents representing VPNs. These agents are not created by any
- * provider, so they use this constant for clarity instead of NONE.
- * @hide only used by ConnectivityService.
- */
- public static final int ID_VPN = -2;
-
- /**
* The first providerId value that will be allocated.
* @hide only used by ConnectivityService.
*/
diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java
index 4f5c13d..102672e 100644
--- a/services/core/java/com/android/server/connectivity/Vpn.java
+++ b/services/core/java/com/android/server/connectivity/Vpn.java
@@ -152,6 +152,7 @@
public class Vpn {
private static final String NETWORKTYPE = "VPN";
private static final String TAG = "Vpn";
+ private static final String VPN_AGENT_NAME = "VpnNetworkAgent";
private static final boolean LOGD = true;
// Length of time (in milliseconds) that an app hosting an always-on VPN is placed on
@@ -443,12 +444,39 @@
* Update current state, dispatching event to listeners.
*/
@VisibleForTesting
+ @GuardedBy("this")
protected void updateState(DetailedState detailedState, String reason) {
if (LOGD) Log.d(TAG, "setting state=" + detailedState + ", reason=" + reason);
mLegacyState = LegacyVpnInfo.stateFromNetworkInfo(detailedState);
mNetworkInfo.setDetailedState(detailedState, reason, null);
- if (mNetworkAgent != null) {
- mNetworkAgent.sendNetworkInfo(mNetworkInfo);
+ // TODO : only accept transitions when the agent is in the correct state (non-null for
+ // CONNECTED, DISCONNECTED and FAILED, null for CONNECTED).
+ // This will require a way for tests to pretend the VPN is connected that's not
+ // calling this method with CONNECTED.
+ // It will also require audit of where the code calls this method with DISCONNECTED
+ // with a null agent, which it was doing historically to make sure the agent is
+ // disconnected as this was a no-op if the agent was null.
+ switch (detailedState) {
+ case CONNECTED:
+ if (null != mNetworkAgent) {
+ mNetworkAgent.markConnected();
+ }
+ break;
+ case DISCONNECTED:
+ case FAILED:
+ if (null != mNetworkAgent) {
+ mNetworkAgent.unregister();
+ mNetworkAgent = null;
+ }
+ break;
+ case CONNECTING:
+ if (null != mNetworkAgent) {
+ throw new IllegalStateException("VPN can only go to CONNECTING state when"
+ + " the agent is null.");
+ }
+ break;
+ default:
+ throw new IllegalArgumentException("Illegal state argument " + detailedState);
}
updateAlwaysOnNotification(detailedState);
}
@@ -1016,7 +1044,7 @@
}
mConfig = null;
- updateState(DetailedState.IDLE, "prepare");
+ updateState(DetailedState.DISCONNECTED, "prepare");
setVpnForcedLocked(mLockdown);
} finally {
Binder.restoreCallingIdentity(token);
@@ -1252,7 +1280,7 @@
mNetworkCapabilities.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
mLegacyState = LegacyVpnInfo.STATE_CONNECTING;
- mNetworkInfo.setDetailedState(DetailedState.CONNECTING, null, null);
+ updateState(DetailedState.CONNECTING, "agentConnect");
NetworkAgentConfig networkAgentConfig = new NetworkAgentConfig();
networkAgentConfig.allowBypass = mConfig.allowBypass && !mLockdown;
@@ -1270,20 +1298,24 @@
mNetworkCapabilities.addCapability(NET_CAPABILITY_NOT_METERED);
}
- final long token = Binder.clearCallingIdentity();
- try {
- mNetworkAgent = new NetworkAgent(mLooper, mContext, NETWORKTYPE /* logtag */,
- mNetworkInfo, mNetworkCapabilities, lp,
- ConnectivityConstants.VPN_DEFAULT_SCORE, networkAgentConfig,
- NetworkProvider.ID_VPN) {
- @Override
- public void unwanted() {
- // We are user controlled, not driven by NetworkRequest.
- }
- };
- } finally {
- Binder.restoreCallingIdentity(token);
- }
+ mNetworkAgent = new NetworkAgent(mContext, mLooper, NETWORKTYPE /* logtag */,
+ mNetworkCapabilities, lp,
+ ConnectivityConstants.VPN_DEFAULT_SCORE, networkAgentConfig,
+ new NetworkProvider(mContext, mLooper, VPN_AGENT_NAME)) {
+ @Override
+ public void unwanted() {
+ // We are user controlled, not driven by NetworkRequest.
+ }
+ };
+ Binder.withCleanCallingIdentity(() -> {
+ try {
+ mNetworkAgent.register();
+ } catch (final Exception e) {
+ // If register() throws, don't keep an unregistered agent.
+ mNetworkAgent = null;
+ throw e;
+ }
+ });
mNetworkAgent.setUnderlyingNetworks((mConfig.underlyingNetworks != null)
? Arrays.asList(mConfig.underlyingNetworks) : null);
mNetworkInfo.setIsAvailable(true);
@@ -1301,19 +1333,12 @@
private void agentDisconnect(NetworkAgent networkAgent) {
if (networkAgent != null) {
- NetworkInfo networkInfo = new NetworkInfo(mNetworkInfo);
- networkInfo.setIsAvailable(false);
- networkInfo.setDetailedState(DetailedState.DISCONNECTED, null, null);
- networkAgent.sendNetworkInfo(networkInfo);
+ networkAgent.unregister();
}
}
private void agentDisconnect() {
- if (mNetworkInfo.isConnected()) {
- mNetworkInfo.setIsAvailable(false);
- updateState(DetailedState.DISCONNECTED, "agentDisconnect");
- mNetworkAgent = null;
- }
+ updateState(DetailedState.DISCONNECTED, "agentDisconnect");
}
/**
@@ -1402,6 +1427,8 @@
&& updateLinkPropertiesInPlaceIfPossible(mNetworkAgent, oldConfig)) {
// Keep mNetworkAgent unchanged
} else {
+ // Initialize the state for a new agent, while keeping the old one connected
+ // in case this new connection fails.
mNetworkAgent = null;
updateState(DetailedState.CONNECTING, "establish");
// Set up forwarding and DNS rules.
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 561c6ba..7e8f195 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -1089,6 +1089,10 @@
mMockNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_VPN, lp,
mNetworkCapabilities);
mMockNetworkAgent.waitForIdle(TIMEOUT_MS);
+ verify(mNetworkManagementService, times(1))
+ .addVpnUidRanges(eq(mMockVpn.getNetId()), eq(uids.toArray(new UidRange[0])));
+ verify(mNetworkManagementService, never())
+ .removeVpnUidRanges(eq(mMockVpn.getNetId()), any());
mAgentRegistered = true;
mNetworkCapabilities.set(mMockNetworkAgent.getNetworkCapabilities());
mNetworkAgent = mMockNetworkAgent.getNetworkAgent();
@@ -6922,8 +6926,8 @@
final Set<UidRange> vpnRange = Collections.singleton(UidRange.createForUser(VPN_USER));
mMockVpn.establish(lp, VPN_UID, vpnRange);
- // Connected VPN should have interface rules set up. There are two expected invocations,
- // one during VPN uid update, one during VPN LinkProperties update
+ // A connected VPN should have interface rules set up. There are two expected invocations,
+ // one during the VPN initial connection, one during the VPN LinkProperties update.
ArgumentCaptor<int[]> uidCaptor = ArgumentCaptor.forClass(int[].class);
verify(mMockNetd, times(2)).firewallAddUidInterfaceRules(eq("tun0"), uidCaptor.capture());
assertContainsExactly(uidCaptor.getAllValues().get(0), APP1_UID, APP2_UID);
diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java
index a553b58..11bf7a9 100644
--- a/tests/net/java/com/android/server/connectivity/VpnTest.java
+++ b/tests/net/java/com/android/server/connectivity/VpnTest.java
@@ -100,6 +100,7 @@
import androidx.test.runner.AndroidJUnit4;
import com.android.internal.R;
+import com.android.internal.net.LegacyVpnInfo;
import com.android.internal.net.VpnConfig;
import com.android.internal.net.VpnProfile;
import com.android.server.IpSecService;
@@ -589,7 +590,7 @@
}
@Test
- public void testNotificationShownForAlwaysOnApp() {
+ public void testNotificationShownForAlwaysOnApp() throws Exception {
final UserHandle userHandle = UserHandle.of(primaryUser.id);
final Vpn vpn = createVpn(primaryUser.id);
setMockedUsers(primaryUser);
@@ -619,7 +620,6 @@
@Test
public void testCapabilities() {
- final Vpn vpn = createVpn(primaryUser.id);
setMockedUsers(primaryUser);
final Network mobile = new Network(1);
@@ -1037,7 +1037,7 @@
when(exception.getErrorType())
.thenReturn(IkeProtocolException.ERROR_TYPE_AUTHENTICATION_FAILED);
- final Vpn vpn = startLegacyVpn(mVpnProfile);
+ final Vpn vpn = startLegacyVpn(createVpn(primaryUser.id), (mVpnProfile));
final NetworkCallback cb = triggerOnAvailableAndGetCallback();
// Wait for createIkeSession() to be called before proceeding in order to ensure consistent
@@ -1048,20 +1048,20 @@
ikeCb.onClosedExceptionally(exception);
verify(mConnectivityManager, timeout(TEST_TIMEOUT_MS)).unregisterNetworkCallback(eq(cb));
- assertEquals(DetailedState.FAILED, vpn.getNetworkInfo().getDetailedState());
+ assertEquals(LegacyVpnInfo.STATE_FAILED, vpn.getLegacyVpnInfo().state);
}
@Test
public void testStartPlatformVpnIllegalArgumentExceptionInSetup() throws Exception {
when(mIkev2SessionCreator.createIkeSession(any(), any(), any(), any(), any(), any()))
.thenThrow(new IllegalArgumentException());
- final Vpn vpn = startLegacyVpn(mVpnProfile);
+ final Vpn vpn = startLegacyVpn(createVpn(primaryUser.id), mVpnProfile);
final NetworkCallback cb = triggerOnAvailableAndGetCallback();
// Wait for createIkeSession() to be called before proceeding in order to ensure consistent
// state
verify(mConnectivityManager, timeout(TEST_TIMEOUT_MS)).unregisterNetworkCallback(eq(cb));
- assertEquals(DetailedState.FAILED, vpn.getNetworkInfo().getDetailedState());
+ assertEquals(LegacyVpnInfo.STATE_FAILED, vpn.getLegacyVpnInfo().state);
}
private void setAndVerifyAlwaysOnPackage(Vpn vpn, int uid, boolean lockdownEnabled) {
@@ -1100,8 +1100,7 @@
// a subsequent CL.
}
- public Vpn startLegacyVpn(final VpnProfile vpnProfile) throws Exception {
- final Vpn vpn = createVpn(primaryUser.id);
+ private Vpn startLegacyVpn(final Vpn vpn, final VpnProfile vpnProfile) throws Exception {
setMockedUsers(primaryUser);
// Dummy egress interface
@@ -1118,7 +1117,7 @@
@Test
public void testStartPlatformVpn() throws Exception {
- startLegacyVpn(mVpnProfile);
+ startLegacyVpn(createVpn(primaryUser.id), mVpnProfile);
// TODO: Test the Ikev2VpnRunner started up properly. Relies on utility methods added in
// a subsequent patch.
}
@@ -1153,7 +1152,7 @@
legacyRunnerReady.open();
return new Network(102);
});
- final Vpn vpn = startLegacyVpn(profile);
+ final Vpn vpn = startLegacyVpn(createVpn(primaryUser.id), profile);
final TestDeps deps = (TestDeps) vpn.mDeps;
try {
// udppsk and 1701 are the values for TYPE_L2TP_IPSEC_PSK