Transfer client id on connection open

Whenever a port redirection is enabled (typically through "adb
reverse"), connect() will succeed even if the relay server is not
listening on the other side of the tunnel.

Commit 301867e provided a workaround,
assuming that the port forwarding was local: the client attempted to
read from the socket with the smallest possible timeout. Since there was
nothing to read, a read error indicated that no server was listening on
the other side of the tunnel, while the connection was considered ok on
timeout.

But this does not generally fix the problem. For example, if the relay
server is executed remotely and exposed through a SSH local port
forwarding, then connect() would still succeed (a server was listening
on the other side of the "adb reverse" tunnel) even if the relay server
was not listening behind the SSH tunnel.

Therefore, modify the protocol between the client and the relay server:
when a client connects, the relay server immediately writes its id (an
integer) to the TCP socket. The client considers itself connected only
once it has received this number. Thus, it will be able to detect
connection failure before switching its state to CONNECTED.

DEVELOP.md

@@ -137,6 +137,14 @@ Each [`Client`] manages a TCP socket, used to transmit raw IP packets from and
 to the _Gnirehtet_ Android client. Thus, these IP packets are encapsulated into
 TCP (they are transmitted as the TCP payload).
 
+When a client connects, the relay server assigns an integer id to it, which it
+writes to the TCP socket. The client considers itself connected to the relay
+server only once it has received this number. This allows to detect any
+end-to-end connection issue immediately. For instance, a TCP _connect_ initiated
+by a client succeeds whenever a port redirection is enabled (typically through
+`adb reverse`), even if the relay server is not listening. In that case, the
+first _read_ will fail.
+
 The [`IPv4Packet`] class provides a structured view to read and write packet
 data, which is physically stored in the buffers (the little squares on the
 schema). Since we handle one packet at a time with asynchronous I/O, there is no

app/src/main/java/com/genymobile/gnirehtet/RelayTunnel.java

@@ -22,9 +22,8 @@
 import java.io.IOException;
 import java.net.Inet4Address;
 import java.net.InetSocketAddress;
-import java.net.Socket;
-import java.net.SocketTimeoutException;
 import java.nio.ByteBuffer;
+import java.nio.channels.ReadableByteChannel;
 import java.nio.channels.SocketChannel;
 
 public final class RelayTunnel implements Tunnel {
@@ -48,7 +47,7 @@ public static RelayTunnel open(VpnService vpnService) throws IOException {
 
     public void connect() throws IOException {
         channel.connect(new InetSocketAddress(Inet4Address.getLocalHost(), DEFAULT_PORT));
-        fakeRead(channel.socket());
+        readClientId(channel);
     }
 
     /**
@@ -60,30 +59,30 @@ public void connect() throws IOException {
      * As a consequence, the connection state of the relay server would be invalid temporarily (we
      * would switch to CONNECTED state then switch back to DISCONNECTED).
      * <p>
-     * To avoid this problem, we must actually try to read from the server, so that an error occurs
+     * To avoid this problem, we must actually read from the server, so that an error occurs
      * immediately if the relay server is not accessible.
      * <p>
-     * To do so, we set the lowest timeout possible (1) that is not "infinity" (0), try to read,
-     * then reset the timeout option. Since the client is always the first peer to send data, there
-     * is never anything to read.
+     * Therefore, the relay server immediately sends the client id: consume it and log it.
      *
-     * @param socket the socket to read
+     * @param channel the channel to communicate with the relay server
      * @throws IOException if an I/O error occurs
      */
-    private static void fakeRead(Socket socket) throws IOException {
-        int timeout = socket.getSoTimeout();
-        socket.setSoTimeout(1);
-        boolean eof = false;
-        try {
-            eof = socket.getInputStream().read() == -1;
-        } catch (SocketTimeoutException e) {
-            // ignore, expected timeout
-        } finally {
-            socket.setSoTimeout(timeout);
-        }
-        if (eof) {
-            throw new IOException("Cannot connect to the relay server");
-        }
+    private static void readClientId(ReadableByteChannel channel) throws IOException {
+        Log.d(TAG, "Requesting client id");
+        int clientId = readInt(channel);
+        Log.d(TAG, "Connected to the relay server as #" + Binary.unsigned(clientId));
+    }
+
+    private static int readInt(ReadableByteChannel channel) throws IOException {
+        final int intSize = 4;
+        ByteBuffer buffer = ByteBuffer.allocate(intSize);
+        do {
+            if (channel.read(buffer) == -1) {
+                throw new IOException("Cannot read from channel");
+            }
+        } while (buffer.hasRemaining());
+        buffer.flip();
+        return buffer.getInt();
     }
 
     @Override

relay/src/main/java/com/genymobile/relay/Client.java

@@ -17,6 +17,7 @@
 package com.genymobile.relay;
 
 import java.io.IOException;
+import java.nio.ByteBuffer;
 import java.nio.channels.ClosedChannelException;
 import java.nio.channels.SelectionKey;
 import java.nio.channels.Selector;
@@ -42,10 +43,14 @@ public class Client {
 
     private final List<PacketSource> pendingPacketSources = new ArrayList<>();
 
+    // store the remaining bytes of "id" to send to the client before relaying any data
+    private ByteBuffer pendingIdBuffer;
+
     public Client(Selector selector, SocketChannel clientChannel, RemoveHandler<Client> removeHandler) throws ClosedChannelException {
         id = nextId++;
         this.clientChannel = clientChannel;
         router = new Router(this, selector);
+        pendingIdBuffer = createIntBuffer(id);
 
         SelectionHandler selectionHandler = (selectionKey) -> {
             if (selectionKey.isValid() && selectionKey.isWritable()) {
@@ -58,12 +63,20 @@ public Client(Selector selector, SocketChannel clientChannel, RemoveHandler<Clie
                 updateInterests();
             }
         };
-        // on start, we are interested only in reading (there is nothing to write)
-        selectionKey = clientChannel.register(selector, SelectionKey.OP_READ, selectionHandler);
+        // on start, we are interested only in writing (we must first send the client id)
+        selectionKey = clientChannel.register(selector, SelectionKey.OP_WRITE, selectionHandler);
 
         this.removeHandler = removeHandler;
     }
 
+    private static ByteBuffer createIntBuffer(int value) {
+        final int intSize = 4;
+        ByteBuffer buffer = ByteBuffer.allocate(intSize);
+        buffer.putInt(value);
+        buffer.flip();
+        return buffer;
+    }
+
     public int getId() {
         return id;
     }
@@ -77,6 +90,12 @@ private void processReceive() {
     }
 
     private void processSend() {
+        if (mustSendId()) {
+            if (!sendId()) {
+                destroy();
+            }
+            return;
+        }
         if (!write()) {
             destroy();
             return;
@@ -102,6 +121,29 @@ private boolean write() {
         }
     }
 
+    private boolean mustSendId() {
+        return pendingIdBuffer != null && pendingIdBuffer.hasRemaining();
+    }
+
+    private boolean sendId() {
+        assert mustSendId();
+        try {
+            if (clientChannel.write(pendingIdBuffer) == -1) {
+                Log.w(TAG, "Cannot write client id #" + id + " (EOF)");
+                return false;
+            }
+            if (!pendingIdBuffer.hasRemaining()) {
+                // we don't need this buffer anymore, release it
+                Log.d(TAG, "Client id #" + id + " sent to client");
+                pendingIdBuffer = null;
+            }
+            return true;
+        } catch (IOException e) {
+            Log.e(TAG, "Cannot write client id #" + id, e);
+            return false;
+        }
+    }
+
     private void pushToNetwork() {
         IPv4Packet packet;
         while ((packet = clientToNetwork.asIPv4Packet()) != null) {