Add a test for starting the legacy VPN.
The legacy VPN has, among many parameters, a host to connect to.
This host can be specified as a numeric address, or as a hostname.
When it's a name, resolution is required. Currently, name
resolution is performed by the native VPN daemons racoon and
mtpd. When a hostname is used, the framework does not know the
IP address of the VPN server and does not add a throw route for
the VPN server IP address. On older kernels this does not matter
because the legacy PPP kernel code binds the PPP socket to the
right network, but on newer devices that use the upstream PPP
code, this does not work. See b/133797637.
This patch instruments the legacy VPN code so that it can be
run in tests, and uses this instrumentation to simulate passing
a configuration that contains a host, and verifies that the
arguments passed to the mptd and racoon daemons receive the
expected server address, and that the expected throw route is
correctly installed.
It then adds two tests : one specifying the server as a numeric
address, and one as a hostname. As the resolution is currently
broken, the latter of these tests is added disabled, and the
followup fix to the issue enables it.
This test is basic and very targeted, but it's what we need right
now. Also there are plans to remove this entire code path in S, so
the test being ad-hoc is not much of a problem.
Test: this
Bug: 158974172
Change-Id: I96f4bbb9b109e3e5813d083bed1989d88fb156b8
Merged-In: I3c4a94181bd71df68121fa0f71669fa4fa588bdd
(cherry picked from commit dece7f3f74cb67f2a046f3a2a9757b559abc2aac)
diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java
index 1f85d10..476e4b7 100644
--- a/services/core/java/com/android/server/connectivity/Vpn.java
+++ b/services/core/java/com/android/server/connectivity/Vpn.java
@@ -123,6 +123,7 @@
import java.net.Inet4Address;
import java.net.Inet6Address;
import java.net.InetAddress;
+import java.net.UnknownHostException;
import java.nio.charset.StandardCharsets;
import java.security.GeneralSecurityException;
import java.util.ArrayList;
@@ -190,6 +191,7 @@
// automated reconnection
private final Context mContext;
+ @VisibleForTesting final Dependencies mDeps;
private final NetworkInfo mNetworkInfo;
@VisibleForTesting protected String mPackage;
private int mOwnerUID;
@@ -252,17 +254,106 @@
// Handle of the user initiating VPN.
private final int mUserHandle;
+ interface RetryScheduler {
+ void checkInterruptAndDelay(boolean sleepLonger) throws InterruptedException;
+ }
+
+ static class Dependencies {
+ public void startService(final String serviceName) {
+ SystemService.start(serviceName);
+ }
+
+ public void stopService(final String serviceName) {
+ SystemService.stop(serviceName);
+ }
+
+ public boolean isServiceRunning(final String serviceName) {
+ return SystemService.isRunning(serviceName);
+ }
+
+ public boolean isServiceStopped(final String serviceName) {
+ return SystemService.isStopped(serviceName);
+ }
+
+ public File getStateFile() {
+ return new File("/data/misc/vpn/state");
+ }
+
+ public void sendArgumentsToDaemon(
+ final String daemon, final LocalSocket socket, final String[] arguments,
+ final RetryScheduler retryScheduler) throws IOException, InterruptedException {
+ final LocalSocketAddress address = new LocalSocketAddress(
+ daemon, LocalSocketAddress.Namespace.RESERVED);
+
+ // Wait for the socket to connect.
+ while (true) {
+ try {
+ socket.connect(address);
+ break;
+ } catch (Exception e) {
+ // ignore
+ }
+ retryScheduler.checkInterruptAndDelay(true /* sleepLonger */);
+ }
+ socket.setSoTimeout(500);
+
+ final OutputStream out = socket.getOutputStream();
+ for (String argument : arguments) {
+ byte[] bytes = argument.getBytes(StandardCharsets.UTF_8);
+ if (bytes.length >= 0xFFFF) {
+ throw new IllegalArgumentException("Argument is too large");
+ }
+ out.write(bytes.length >> 8);
+ out.write(bytes.length);
+ out.write(bytes);
+ retryScheduler.checkInterruptAndDelay(false /* sleepLonger */);
+ }
+ out.write(0xFF);
+ out.write(0xFF);
+
+ // Wait for End-of-File.
+ final InputStream in = socket.getInputStream();
+ while (true) {
+ try {
+ if (in.read() == -1) {
+ break;
+ }
+ } catch (Exception e) {
+ // ignore
+ }
+ retryScheduler.checkInterruptAndDelay(true /* sleepLonger */);
+ }
+ }
+
+ // TODO : implement and use this.
+ @NonNull
+ public InetAddress resolve(final String endpoint) throws UnknownHostException {
+ try {
+ return InetAddress.parseNumericAddress(endpoint);
+ } catch (IllegalArgumentException e) {
+ Log.e(TAG, "Endpoint is not numeric");
+ }
+ throw new UnknownHostException(endpoint);
+ }
+
+ public boolean checkInterfacePresent(final Vpn vpn, final String iface) {
+ return vpn.jniCheck(iface) == 0;
+ }
+ }
+
public Vpn(Looper looper, Context context, INetworkManagementService netService,
@UserIdInt int userHandle, @NonNull KeyStore keyStore) {
- this(looper, context, netService, userHandle, keyStore,
+ this(looper, context, new Dependencies(), netService, userHandle, keyStore,
new SystemServices(context), new Ikev2SessionCreator());
}
@VisibleForTesting
- protected Vpn(Looper looper, Context context, INetworkManagementService netService,
+ protected Vpn(Looper looper, Context context, Dependencies deps,
+ INetworkManagementService netService,
int userHandle, @NonNull KeyStore keyStore, SystemServices systemServices,
Ikev2SessionCreator ikev2SessionCreator) {
mContext = context;
+ mDeps = deps;
mNetd = netService;
mUserHandle = userHandle;
mLooper = looper;
@@ -2129,7 +2220,8 @@
}
/** This class represents the common interface for all VPN runners. */
- private abstract class VpnRunner extends Thread {
+ @VisibleForTesting
+ abstract class VpnRunner extends Thread {
protected VpnRunner(String name) {
super(name);
@@ -2638,7 +2730,7 @@
} catch (InterruptedException e) {
}
for (String daemon : mDaemons) {
- SystemService.stop(daemon);
+ mDeps.stopService(daemon);
}
}
agentDisconnect();
@@ -2663,13 +2755,13 @@
// Wait for the daemons to stop.
for (String daemon : mDaemons) {
- while (!SystemService.isStopped(daemon)) {
+ while (!mDeps.isServiceStopped(daemon)) {
checkInterruptAndDelay(true);
}
}
// Clear the previous state.
- File state = new File("/data/misc/vpn/state");
+ final File state = mDeps.getStateFile();
state.delete();
if (state.exists()) {
throw new IllegalStateException("Cannot delete the state");
@@ -2696,57 +2788,19 @@
// Start the daemon.
String daemon = mDaemons[i];
- SystemService.start(daemon);
+ mDeps.startService(daemon);
// Wait for the daemon to start.
- while (!SystemService.isRunning(daemon)) {
+ while (!mDeps.isServiceRunning(daemon)) {
checkInterruptAndDelay(true);
}
// Create the control socket.
mSockets[i] = new LocalSocket();
- LocalSocketAddress address = new LocalSocketAddress(
- daemon, LocalSocketAddress.Namespace.RESERVED);
- // Wait for the socket to connect.
- while (true) {
- try {
- mSockets[i].connect(address);
- break;
- } catch (Exception e) {
- // ignore
- }
- checkInterruptAndDelay(true);
- }
- mSockets[i].setSoTimeout(500);
-
- // Send over the arguments.
- OutputStream out = mSockets[i].getOutputStream();
- for (String argument : arguments) {
- byte[] bytes = argument.getBytes(StandardCharsets.UTF_8);
- if (bytes.length >= 0xFFFF) {
- throw new IllegalArgumentException("Argument is too large");
- }
- out.write(bytes.length >> 8);
- out.write(bytes.length);
- out.write(bytes);
- checkInterruptAndDelay(false);
- }
- out.write(0xFF);
- out.write(0xFF);
-
- // Wait for End-of-File.
- InputStream in = mSockets[i].getInputStream();
- while (true) {
- try {
- if (in.read() == -1) {
- break;
- }
- } catch (Exception e) {
- // ignore
- }
- checkInterruptAndDelay(true);
- }
+ // Wait for the socket to connect and send over the arguments.
+ mDeps.sendArgumentsToDaemon(daemon, mSockets[i], arguments,
+ this::checkInterruptAndDelay);
}
// Wait for the daemons to create the new state.
@@ -2754,7 +2808,7 @@
// Check if a running daemon is dead.
for (int i = 0; i < mDaemons.length; ++i) {
String daemon = mDaemons[i];
- if (mArguments[i] != null && !SystemService.isRunning(daemon)) {
+ if (mArguments[i] != null && !mDeps.isServiceRunning(daemon)) {
throw new IllegalStateException(daemon + " is dead");
}
}
@@ -2764,7 +2818,8 @@
// Now we are connected. Read and parse the new state.
String[] parameters = FileUtils.readTextFile(state, 0, null).split("\n", -1);
if (parameters.length != 7) {
- throw new IllegalStateException("Cannot parse the state");
+ throw new IllegalStateException("Cannot parse the state: '"
+ + String.join("', '", parameters) + "'");
}
// Set the interface and the addresses in the config.
@@ -2818,7 +2873,7 @@
checkInterruptAndDelay(false);
// Check if the interface is gone while we are waiting.
- if (jniCheck(mConfig.interfaze) == 0) {
+ if (mDeps.checkInterfacePresent(Vpn.this, mConfig.interfaze)) {
throw new IllegalStateException(mConfig.interfaze + " is gone");
}
@@ -2849,7 +2904,7 @@
while (true) {
Thread.sleep(2000);
for (int i = 0; i < mDaemons.length; i++) {
- if (mArguments[i] != null && SystemService.isStopped(mDaemons[i])) {
+ if (mArguments[i] != null && mDeps.isServiceStopped(mDaemons[i])) {
return;
}
}
diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java
index 4ccf79a..a9313a3 100644
--- a/tests/net/java/com/android/server/connectivity/VpnTest.java
+++ b/tests/net/java/com/android/server/connectivity/VpnTest.java
@@ -30,6 +30,7 @@
import static android.net.NetworkCapabilities.TRANSPORT_VPN;
import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
+import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -49,6 +50,7 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
+import android.annotation.NonNull;
import android.annotation.UserIdInt;
import android.app.AppOpsManager;
import android.app.NotificationManager;
@@ -65,6 +67,7 @@
import android.net.IpPrefix;
import android.net.IpSecManager;
import android.net.LinkProperties;
+import android.net.LocalSocket;
import android.net.Network;
import android.net.NetworkCapabilities;
import android.net.NetworkInfo.DetailedState;
@@ -74,6 +77,7 @@
import android.net.VpnService;
import android.os.Build.VERSION_CODES;
import android.os.Bundle;
+import android.os.ConditionVariable;
import android.os.INetworkManagementService;
import android.os.Looper;
import android.os.Process;
@@ -94,6 +98,7 @@
import com.android.server.IpSecService;
import org.junit.Before;
+import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Answers;
@@ -101,13 +106,20 @@
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
import java.net.Inet4Address;
+import java.net.InetAddress;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;
/**
@@ -133,7 +145,8 @@
managedProfileA.profileGroupId = primaryUser.id;
}
- static final String TEST_VPN_PKG = "com.dummy.vpn";
+ static final String EGRESS_IFACE = "wlan0";
+ static final String TEST_VPN_PKG = "com.testvpn.vpn";
private static final String TEST_VPN_SERVER = "1.2.3.4";
private static final String TEST_VPN_IDENTITY = "identity";
private static final byte[] TEST_VPN_PSK = "psk".getBytes();
@@ -1012,31 +1025,191 @@
// a subsequent CL.
}
- @Test
- public void testStartLegacyVpn() throws Exception {
+ public Vpn startLegacyVpn(final VpnProfile vpnProfile) throws Exception {
final Vpn vpn = createVpn(primaryUser.id);
setMockedUsers(primaryUser);
// Dummy egress interface
- final String egressIface = "DUMMY0";
final LinkProperties lp = new LinkProperties();
- lp.setInterfaceName(egressIface);
+ lp.setInterfaceName(EGRESS_IFACE);
final RouteInfo defaultRoute = new RouteInfo(new IpPrefix(Inet4Address.ANY, 0),
- InetAddresses.parseNumericAddress("192.0.2.0"), egressIface);
+ InetAddresses.parseNumericAddress("192.0.2.0"), EGRESS_IFACE);
lp.addRoute(defaultRoute);
- vpn.startLegacyVpn(mVpnProfile, mKeyStore, lp);
+ vpn.startLegacyVpn(vpnProfile, mKeyStore, lp);
+ return vpn;
+ }
+ @Test
+ public void testStartPlatformVpn() throws Exception {
+ startLegacyVpn(mVpnProfile);
// TODO: Test the Ikev2VpnRunner started up properly. Relies on utility methods added in
- // a subsequent CL.
+ // a subsequent patch.
+ }
+
+ @Test
+ public void testStartRacoonNumericAddress() throws Exception {
+ startRacoon("1.2.3.4", "1.2.3.4");
+ }
+
+ @Test
+ @Ignore("b/158974172") // remove when the bug is fixed
+ public void testStartRacoonHostname() throws Exception {
+ startRacoon("hostname", "5.6.7.8"); // address returned by deps.resolve
+ }
+
+ public void startRacoon(final String serverAddr, final String expectedAddr)
+ throws Exception {
+ final ConditionVariable legacyRunnerReady = new ConditionVariable();
+ final VpnProfile profile = new VpnProfile("testProfile" /* key */);
+ profile.type = VpnProfile.TYPE_L2TP_IPSEC_PSK;
+ profile.name = "testProfileName";
+ profile.username = "userName";
+ profile.password = "thePassword";
+ profile.server = serverAddr;
+ profile.ipsecIdentifier = "id";
+ profile.ipsecSecret = "secret";
+ profile.l2tpSecret = "l2tpsecret";
+ when(mConnectivityManager.getAllNetworks())
+ .thenReturn(new Network[] { new Network(101) });
+ when(mConnectivityManager.registerNetworkAgent(any(), any(), any(), any(),
+ anyInt(), any(), anyInt())).thenAnswer(invocation -> {
+ // The runner has registered an agent and is now ready.
+ legacyRunnerReady.open();
+ return new Network(102);
+ });
+ final Vpn vpn = startLegacyVpn(profile);
+ final TestDeps deps = (TestDeps) vpn.mDeps;
+ try {
+ // udppsk and 1701 are the values for TYPE_L2TP_IPSEC_PSK
+ assertArrayEquals(
+ new String[] { EGRESS_IFACE, expectedAddr, "udppsk",
+ profile.ipsecIdentifier, profile.ipsecSecret, "1701" },
+ deps.racoonArgs.get(10, TimeUnit.SECONDS));
+ // literal values are hardcoded in Vpn.java for mtpd args
+ assertArrayEquals(
+ new String[] { EGRESS_IFACE, "l2tp", expectedAddr, "1701", profile.l2tpSecret,
+ "name", profile.username, "password", profile.password,
+ "linkname", "vpn", "refuse-eap", "nodefaultroute", "usepeerdns",
+ "idle", "1800", "mtu", "1400", "mru", "1400" },
+ deps.mtpdArgs.get(10, TimeUnit.SECONDS));
+ // Now wait for the runner to be ready before testing for the route.
+ legacyRunnerReady.block(10_000);
+ // In this test the expected address is always v4 so /32
+ final RouteInfo expectedRoute = new RouteInfo(new IpPrefix(expectedAddr + "/32"),
+ RouteInfo.RTN_THROW);
+ assertTrue("Routes lack the expected throw route (" + expectedRoute + ") : "
+ + vpn.mConfig.routes,
+ vpn.mConfig.routes.contains(expectedRoute));
+ } finally {
+ // Now interrupt the thread, unblock the runner and clean up.
+ vpn.mVpnRunner.exitVpnRunner();
+ deps.getStateFile().delete(); // set to delete on exit, but this deletes it earlier
+ vpn.mVpnRunner.join(10_000); // wait for up to 10s for the runner to die and cleanup
+ }
+ }
+
+ private static final class TestDeps extends Vpn.Dependencies {
+ public final CompletableFuture<String[]> racoonArgs = new CompletableFuture();
+ public final CompletableFuture<String[]> mtpdArgs = new CompletableFuture();
+ public final File mStateFile;
+
+ private final HashMap<String, Boolean> mRunningServices = new HashMap<>();
+
+ TestDeps() {
+ try {
+ mStateFile = File.createTempFile("vpnTest", ".tmp");
+ mStateFile.deleteOnExit();
+ } catch (final IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ @Override
+ public void startService(final String serviceName) {
+ mRunningServices.put(serviceName, true);
+ }
+
+ @Override
+ public void stopService(final String serviceName) {
+ mRunningServices.put(serviceName, false);
+ }
+
+ @Override
+ public boolean isServiceRunning(final String serviceName) {
+ return mRunningServices.getOrDefault(serviceName, false);
+ }
+
+ @Override
+ public boolean isServiceStopped(final String serviceName) {
+ return !isServiceRunning(serviceName);
+ }
+
+ @Override
+ public File getStateFile() {
+ return mStateFile;
+ }
+
+ @Override
+ public void sendArgumentsToDaemon(
+ final String daemon, final LocalSocket socket, final String[] arguments,
+ final Vpn.RetryScheduler interruptChecker) throws IOException {
+ if ("racoon".equals(daemon)) {
+ racoonArgs.complete(arguments);
+ } else if ("mtpd".equals(daemon)) {
+ writeStateFile(arguments);
+ mtpdArgs.complete(arguments);
+ } else {
+ throw new UnsupportedOperationException("Unsupported daemon : " + daemon);
+ }
+ }
+
+ private void writeStateFile(final String[] arguments) throws IOException {
+ mStateFile.delete();
+ mStateFile.createNewFile();
+ mStateFile.deleteOnExit();
+ final BufferedWriter writer = new BufferedWriter(
+ new FileWriter(mStateFile, false /* append */));
+ writer.write(EGRESS_IFACE);
+ writer.write("\n");
+ // addresses
+ writer.write("10.0.0.1/24\n");
+ // routes
+ writer.write("192.168.6.0/24\n");
+ // dns servers
+ writer.write("192.168.6.1\n");
+ // search domains
+ writer.write("vpn.searchdomains.com\n");
+ // endpoint - intentionally empty
+ writer.write("\n");
+ writer.flush();
+ writer.close();
+ }
+
+ @Override
+ @NonNull
+ public InetAddress resolve(final String endpoint) {
+ try {
+ // If a numeric IP address, return it.
+ return InetAddress.parseNumericAddress(endpoint);
+ } catch (IllegalArgumentException e) {
+ // Otherwise, return some token IP to test for.
+ return InetAddress.parseNumericAddress("5.6.7.8");
+ }
+ }
+
+ @Override
+ public boolean checkInterfacePresent(final Vpn vpn, final String iface) {
+ return true;
+ }
}
/**
* Mock some methods of vpn object.
*/
private Vpn createVpn(@UserIdInt int userId) {
- return new Vpn(Looper.myLooper(), mContext, mNetService,
+ return new Vpn(Looper.myLooper(), mContext, new TestDeps(), mNetService,
userId, mKeyStore, mSystemServices, mIkev2SessionCreator);
}
