VPN: Hook up the new control protocol and network state.

1. No more End-Of-Arguments.
2. Daemons close the control socket after they are initialized.
3. No more system properties.
4. ip-up-vpn now creates state to pass the configuration.
5. JNI methods are split again for legacy VPN.

Change-Id: I02fafdf01d425c965345ef712b2bd5fdee3a0cab

2 files changed, 173 insertions, 93 deletions

diff --git a/services/java/com/android/server/connectivity/Vpn.java b/services/java/com/android/server/connectivity/Vpn.java
index c185012..55ba8e2 100644
--- a/services/java/com/android/server/connectivity/Vpn.java
+++ b/services/java/com/android/server/connectivity/Vpn.java
@@ -41,6 +41,9 @@ import com.android.internal.net.LegacyVpnInfo;
 import com.android.internal.net.VpnConfig;
 import com.android.server.ConnectivityService.VpnCallback;
 
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.Charsets;
 import java.util.Arrays;
@@ -192,10 +195,15 @@ public class Vpn extends INetworkManagementEventObserver.Stub {
         }
 
         // Configure the interface. Abort if any of these steps fails.
-        ParcelFileDescriptor tun = ParcelFileDescriptor.adoptFd(
-                jniConfigure(config.mtu, config.addresses, config.routes));
+        ParcelFileDescriptor tun = ParcelFileDescriptor.adoptFd(jniCreate(config.mtu));
         try {
             String interfaze = jniGetName(tun.getFd());
+            if (jniSetAddresses(interfaze, config.addresses) < 1) {
+                throw new IllegalArgumentException("At least one address must be specified");
+            }
+            if (config.routes != null) {
+                jniSetRoutes(interfaze, config.routes);
+            }
             if (mInterface != null && !mInterface.equals(interfaze)) {
                 jniReset(mInterface);
             }
@@ -279,8 +287,10 @@ public class Vpn extends INetworkManagementEventObserver.Stub {
         }
     }
 
-    private native int jniConfigure(int mtu, String addresses, String routes);
+    private native int jniCreate(int mtu);
     private native String jniGetName(int tun);
+    private native int jniSetAddresses(String interfaze, String addresses);
+    private native int jniSetRoutes(String interfaze, String routes);
     private native void jniReset(String interfaze);
     private native int jniCheck(String interfaze);
     private native void jniProtect(int socket, String interfaze);
@@ -323,7 +333,6 @@ public class Vpn extends INetworkManagementEventObserver.Stub {
      */
     private class LegacyVpnRunner extends Thread {
         private static final String TAG = "LegacyVpnRunner";
-        private static final String NONE = "--";
 
         private final VpnConfig mConfig;
         private final String[] mDaemons;
@@ -346,10 +355,10 @@ public class Vpn extends INetworkManagementEventObserver.Stub {
 
         public void exit() {
             // We assume that everything is reset after the daemons die.
+            interrupt();
             for (String daemon : mDaemons) {
                 SystemProperties.set("ctl.stop", daemon);
             }
-            interrupt();
         }
 
         public LegacyVpnInfo getInfo() {
@@ -380,7 +389,7 @@ public class Vpn extends INetworkManagementEventObserver.Stub {
                 Thread.sleep(yield ? 200 : 1);
             } else {
                 mInfo.state = LegacyVpnInfo.STATE_TIMEOUT;
-                throw new IllegalStateException("time is up");
+                throw new IllegalStateException("Time is up");
             }
         }
 
@@ -404,12 +413,11 @@ public class Vpn extends INetworkManagementEventObserver.Stub {
                     }
                 }
 
-                // Reset the properties.
-                SystemProperties.set("vpn.dns", NONE);
-                SystemProperties.set("vpn.via", NONE);
-                while (!NONE.equals(SystemProperties.get("vpn.dns")) ||
-                        !NONE.equals(SystemProperties.get("vpn.via"))) {
-                    checkpoint(true);
+                // Clear the previous state.
+                File state = new File("/data/misc/vpn/state");
+                state.delete();
+                if (state.exists()) {
+                    throw new IllegalStateException("Cannot delete the state");
                 }
 
                 // Check if we need to restart any of the daemons.
@@ -461,29 +469,34 @@ public class Vpn extends INetworkManagementEventObserver.Stub {
                     OutputStream out = socket.getOutputStream();
                     for (String argument : arguments) {
                         byte[] bytes = argument.getBytes(Charsets.UTF_8);
-                        if (bytes.length >= 0xFFFF) {
-                            throw new IllegalArgumentException("argument is too large");
+                        if (bytes.length > 0xFFFF) {
+                            throw new IllegalArgumentException("Argument is too large");
                         }
                         out.write(bytes.length >> 8);
                         out.write(bytes.length);
                         out.write(bytes);
                         checkpoint(false);
                     }
-
-                    // Send End-Of-Arguments.
-                    out.write(0xFF);
-                    out.write(0xFF);
                     out.flush();
+                    socket.shutdownOutput();
+
+                    // Wait for End-of-File.
+                    InputStream in = socket.getInputStream();
+                    while (true) {
+                        try {
+                            if (in.read() == -1) {
+                                break;
+                            }
+                        } catch (Exception e) {
+                            // ignore
+                        }
+                        checkpoint(true);
+                    }
                     socket.close();
                 }
 
-                // Now here is the beast from the old days. We check few
-                // properties to figure out the current status. Ideally we
-                // can read things back from the sockets and get rid of the
-                // properties, but we have no time...
-                while (NONE.equals(SystemProperties.get("vpn.dns")) ||
-                        NONE.equals(SystemProperties.get("vpn.via"))) {
-
+                // Wait for the daemons to create the new state.
+                while (!state.exists()) {
                     // Check if a running daemon is dead.
                     for (int i = 0; i < mDaemons.length; ++i) {
                         String daemon = mDaemons[i];
@@ -495,20 +508,45 @@ public class Vpn extends INetworkManagementEventObserver.Stub {
                     checkpoint(true);
                 }
 
-                // Now we are connected. Get the interface.
-                mConfig.interfaze = SystemProperties.get("vpn.via");
+                // Now we are connected. Read and parse the new state.
+                byte[] buffer = new byte[(int) state.length()];
+                if (new FileInputStream(state).read(buffer) != buffer.length) {
+                    throw new IllegalStateException("Cannot read the state");
+                }
+                String[] parameters = new String(buffer, Charsets.UTF_8).split("\n", -1);
+                if (parameters.length != 6) {
+                    throw new IllegalStateException("Cannot parse the state");
+                }
+
+                // Set the interface and the addresses in the config.
+                mConfig.interfaze = parameters[0].trim();
+                mConfig.addresses = parameters[1].trim();
 
-                // Get the DNS servers if they are not set in config.
+                // Set the routes if they are not set in the config.
+                if (mConfig.routes == null || mConfig.routes.isEmpty()) {
+                    mConfig.routes = parameters[2].trim();
+                }
+
+                // Set the DNS servers if they are not set in the config.
                 if (mConfig.dnsServers == null || mConfig.dnsServers.size() == 0) {
-                    String dnsServers = SystemProperties.get("vpn.dns").trim();
+                    String dnsServers = parameters[3].trim();
                     if (!dnsServers.isEmpty()) {
                         mConfig.dnsServers = Arrays.asList(dnsServers.split(" "));
                     }
                 }
 
-                // TODO: support search domains from ISAKMP mode config.
+                // Set the search domains if they are not set in the config.
+                if (mConfig.searchDomains == null || mConfig.searchDomains.size() == 0) {
+                    String searchDomains = parameters[4].trim();
+                    if (!searchDomains.isEmpty()) {
+                        mConfig.searchDomains = Arrays.asList(searchDomains.split(" "));
+                    }
+                }
+
+                // Set the routes.
+                jniSetRoutes(mConfig.interfaze, mConfig.routes);
 
-                // The final step must be synchronized.
+                // Here is the last step and it must be done synchronously.
                 synchronized (Vpn.this) {
                     // Check if the thread is interrupted while we are waiting.
                     checkpoint(false);
diff --git a/services/jni/com_android_server_connectivity_Vpn.cpp b/services/jni/com_android_server_connectivity_Vpn.cpp
index 5f920f1..d28a6b4 100644
--- a/services/jni/com_android_server_connectivity_Vpn.cpp
+++ b/services/jni/com_android_server_connectivity_Vpn.cpp
@@ -18,7 +18,6 @@
 
 #define LOG_TAG "VpnJni"
 #include <cutils/log.h>
-#include <cutils/properties.h>
 
 #include <stdio.h>
 #include <string.h>
@@ -54,7 +53,7 @@ static inline in_addr_t *as_in_addr(sockaddr *sa) {
 #define SYSTEM_ERROR -1
 #define BAD_ARGUMENT -2
 
-static int create_interface(char *name, int *index, int mtu)
+static int create_interface(int mtu)
 {
     int tun = open("/dev/tun", O_RDWR | O_NONBLOCK);
 
@@ -82,14 +81,6 @@ static int create_interface(char *name, int *index, int mtu)
         goto error;
     }
 
-    // Get interface index.
-    if (ioctl(inet4, SIOGIFINDEX, &ifr4)) {
-        LOGE("Cannot get index of %s: %s", ifr4.ifr_name, strerror(errno));
-        goto error;
-    }
-
-    strncpy(name, ifr4.ifr_name, IFNAMSIZ);
-    *index = ifr4.ifr_ifindex;
     return tun;
 
 error:
@@ -97,12 +88,40 @@ error:
     return SYSTEM_ERROR;
 }
 
-static int set_addresses(const char *name, int index, const char *addresses)
+static int get_interface_name(char *name, int tun)
+{
+    ifreq ifr4;
+    if (ioctl(tun, TUNGETIFF, &ifr4)) {
+        LOGE("Cannot get interface name: %s", strerror(errno));
+        return SYSTEM_ERROR;
+    }
+    strncpy(name, ifr4.ifr_name, IFNAMSIZ);
+    return 0;
+}
+
+static int get_interface_index(const char *name)
 {
     ifreq ifr4;
+    strncpy(ifr4.ifr_name, name, IFNAMSIZ);
+    if (ioctl(inet4, SIOGIFINDEX, &ifr4)) {
+        LOGE("Cannot get index of %s: %s", name, strerror(errno));
+        return SYSTEM_ERROR;
+    }
+    return ifr4.ifr_ifindex;
+}
+
+static int set_addresses(const char *name, const char *addresses)
+{
+    int index = get_interface_index(name);
+    if (index < 0) {
+        return index;
+    }
+
+    ifreq ifr4;
     memset(&ifr4, 0, sizeof(ifr4));
     strncpy(ifr4.ifr_name, name, IFNAMSIZ);
     ifr4.ifr_addr.sa_family = AF_INET;
+    ifr4.ifr_netmask.sa_family = AF_INET;
 
     in6_ifreq ifr6;
     memset(&ifr6, 0, sizeof(ifr6));
@@ -146,7 +165,7 @@ static int set_addresses(const char *name, int index, const char *addresses)
             }
 
             in_addr_t mask = prefix ? (~0 << (32 - prefix)) : 0;
-            *as_in_addr(&ifr4.ifr_addr) = htonl(mask);
+            *as_in_addr(&ifr4.ifr_netmask) = htonl(mask);
             if (ioctl(inet4, SIOCSIFNETMASK, &ifr4)) {
                 count = (errno == EINVAL) ? BAD_ARGUMENT : SYSTEM_ERROR;
                 break;
@@ -168,8 +187,13 @@ static int set_addresses(const char *name, int index, const char *addresses)
     return count;
 }
 
-static int set_routes(const char *name, int index, const char *routes)
+static int set_routes(const char *name, const char *routes)
 {
+    int index = get_interface_index(name);
+    if (index < 0) {
+        return index;
+    }
+
     rtentry rt4;
     memset(&rt4, 0, sizeof(rt4));
     rt4.rt_dev = (char *)name;
@@ -253,17 +277,6 @@ static int set_routes(const char *name, int index, const char *routes)
     return count;
 }
 
-static int get_interface_name(char *name, int tun)
-{
-    ifreq ifr4;
-    if (ioctl(tun, TUNGETIFF, &ifr4)) {
-        LOGE("Cannot get interface name: %s", strerror(errno));
-        return SYSTEM_ERROR;
-    }
-    strncpy(name, ifr4.ifr_name, IFNAMSIZ);
-    return 0;
-}
-
 static int reset_interface(const char *name)
 {
     ifreq ifr4;
@@ -309,63 +322,90 @@ static void throwException(JNIEnv *env, int error, const char *message)
     }
 }
 
-static jint configure(JNIEnv *env, jobject thiz,
-        jint mtu, jstring jAddresses, jstring jRoutes)
+static jint create(JNIEnv *env, jobject thiz, jint mtu)
 {
-    char name[IFNAMSIZ];
-    int index;
-    int tun = create_interface(name, &index, mtu);
+    int tun = create_interface(mtu);
     if (tun < 0) {
         throwException(env, tun, "Cannot create interface");
         return -1;
     }
+    return tun;
+}
 
+static jstring getName(JNIEnv *env, jobject thiz, jint tun)
+{
+    char name[IFNAMSIZ];
+    if (get_interface_name(name, tun) < 0) {
+        throwException(env, SYSTEM_ERROR, "Cannot get interface name");
+        return NULL;
+    }
+    return env->NewStringUTF(name);
+}
+
+static jint setAddresses(JNIEnv *env, jobject thiz, jstring jName,
+        jstring jAddresses)
+{
+    const char *name = NULL;
     const char *addresses = NULL;
-    const char *routes = NULL;
-    int count;
+    int count = -1;
 
-    // At least one address must be set.
+    name = jName ? env->GetStringUTFChars(jName, NULL) : NULL;
+    if (!name) {
+        jniThrowNullPointerException(env, "name");
+        goto error;
+    }
     addresses = jAddresses ? env->GetStringUTFChars(jAddresses, NULL) : NULL;
     if (!addresses) {
-        jniThrowNullPointerException(env, "address");
+        jniThrowNullPointerException(env, "addresses");
         goto error;
     }
-    count = set_addresses(name, index, addresses);
-    env->ReleaseStringUTFChars(jAddresses, addresses);
-    if (count <= 0) {
+    count = set_addresses(name, addresses);
+    if (count < 0) {
         throwException(env, count, "Cannot set address");
-        goto error;
+        count = -1;
     }
-    LOGD("Configured %d address(es) on %s", count, name);
-
-    // On the contrary, routes are optional.
-    routes = jRoutes ? env->GetStringUTFChars(jRoutes, NULL) : NULL;
-    if (routes) {
-        count = set_routes(name, index, routes);
-        env->ReleaseStringUTFChars(jRoutes, routes);
-        if (count < 0) {
-            throwException(env, count, "Cannot set route");
-            goto error;
-        }
-        LOGD("Configured %d route(s) on %s", count, name);
-    }
-
-    return tun;
 
 error:
-    close(tun);
-    LOGD("%s is destroyed", name);
-    return -1;
+    if (name) {
+        env->ReleaseStringUTFChars(jName, name);
+    }
+    if (addresses) {
+        env->ReleaseStringUTFChars(jAddresses, addresses);
+    }
+    return count;
 }
 
-static jstring getName(JNIEnv *env, jobject thiz, jint tun)
+static jint setRoutes(JNIEnv *env, jobject thiz, jstring jName,
+        jstring jRoutes)
 {
-    char name[IFNAMSIZ];
-    if (get_interface_name(name, tun) < 0) {
-        throwException(env, SYSTEM_ERROR, "Cannot get interface name");
-        return NULL;
+    const char *name = NULL;
+    const char *routes = NULL;
+    int count = -1;
+
+    name = jName ? env->GetStringUTFChars(jName, NULL) : NULL;
+    if (!name) {
+        jniThrowNullPointerException(env, "name");
+        goto error;
     }
-    return env->NewStringUTF(name);
+    routes = jRoutes ? env->GetStringUTFChars(jRoutes, NULL) : NULL;
+    if (!routes) {
+        jniThrowNullPointerException(env, "routes");
+        goto error;
+    }
+    count = set_routes(name, routes);
+    if (count < 0) {
+        throwException(env, count, "Cannot set route");
+        count = -1;
+    }
+
+error:
+    if (name) {
+        env->ReleaseStringUTFChars(jName, name);
+    }
+    if (routes) {
+        env->ReleaseStringUTFChars(jRoutes, routes);
+    }
+    return count;
 }
 
 static void reset(JNIEnv *env, jobject thiz, jstring jName)
@@ -409,8 +449,10 @@ static void protect(JNIEnv *env, jobject thiz, jint socket, jstring jName)
 //------------------------------------------------------------------------------
 
 static JNINativeMethod gMethods[] = {
-    {"jniConfigure", "(ILjava/lang/String;Ljava/lang/String;)I", (void *)configure},
+    {"jniCreate", "(I)I", (void *)create},
     {"jniGetName", "(I)Ljava/lang/String;", (void *)getName},
+    {"jniSetAddresses", "(Ljava/lang/String;Ljava/lang/String;)I", (void *)setAddresses},
+    {"jniSetRoutes", "(Ljava/lang/String;Ljava/lang/String;)I", (void *)setRoutes},
     {"jniReset", "(Ljava/lang/String;)V", (void *)reset},
     {"jniCheck", "(Ljava/lang/String;)I", (void *)check},
     {"jniProtect", "(ILjava/lang/String;)V", (void *)protect},