devices/vsock: added connection handshake

Changed the host-initiated vsock connection protocol to include a
trivial handshake.

The new protocol looks like this:
- [host] CONNECT <port><LF>
- [guest/success] OK <assigned_host_port><LF>

On connection failure, the host host connection is reset without any
accompanying message, as before.

This allows host software to more easily detect connection failures, for
instance when attempting to connect to a guest server that may have not
yet started listening for client connections.

Signed-off-by: Dan Horobeanu <[email protected]>

Author: dhrgit

CHANGELOG.md

@@ -39,6 +39,12 @@
   appear to support multiple vsock devices. Any subsequent calls to this API
   endpoint will override the previous vsock device configuration.
 - Removed unused 'Halting' and 'Halted' instance states.
+- Vsock host-initiated connections now implement a trivial handshake protocol.
+  See the [vsock doc](docs/vsock.md#host-initiated-connections) for details.
+  Related to:
+  [#1253](https://github.com/firecracker-microvm/firecracker/issues/1253),
+  [#1432](https://github.com/firecracker-microvm/firecracker/issues/1432),
+  [#1443](https://github.com/firecracker-microvm/firecracker/pull/1443)
 
 ### Fixed
 

docs/vsock.md

@@ -39,7 +39,10 @@ send a connect command, in text form, specifying the destination AF_VSOCK port:
 "CONNECT PORT\n". Where PORT is the decimal port number, and "\n" is EOL (ASCII
 0x0A). Following that, the same connection will be forwarded by Firecracker to
 the guest software listening on that port, thus establishing the requested
-channel. If no one is listening, Firecracker will terminate the host
+channel. If the connection has been established, Firecracker will send an
+acknowledgement message to the connecting end (host-side), in the form
+"OK PORT\n", where `PORT` is the vsock port number assigned to
+the host end. If no one is listening, Firecracker will terminate the host
 connection.
 
 1. Host: At VM configuration time, add a virtio-vsock device, with some path
@@ -48,6 +51,7 @@ connection.
 3. Host: `connect()` to AF_UNIX at `uds_path`.
 4. Host: `send()` "CONNECT <port_num>\n".
 5. Guest: `accept()` the new connection.
+6. Host: `read()` "OK <assigned_hostside_port>\n".
 
 The channel is established between the sockets obtained at steps 3 (host)
 and 5 (guest).
@@ -126,7 +130,12 @@ port:
 ```bash
 $ socat - UNIX-CONNECT:./v.sock
 CONNECT 52
+```
+
+`socat` will display the connection acknowledgement message:
 
+```
+OK 1073741824
 ```
 
 The connection should now be established (in the above example, between

src/devices/src/virtio/vsock/csm/connection.rs

@@ -541,7 +541,7 @@ where
     ///
     /// Raw data can either be sent straight to the host stream, or to our TX buffer, if the
     /// former fails.
-    fn send_bytes(&mut self, buf: &[u8]) -> Result<()> {
+    pub fn send_bytes(&mut self, buf: &[u8]) -> Result<()> {
         // If there is data in the TX buffer, that means we're already registered for EPOLLOUT
         // events on the underlying stream. Therefore, there's no point in attempting a write
         // at this point. `self.notify()` will get called when EPOLLOUT arrives, and it will
@@ -576,6 +576,11 @@ where
         Ok(())
     }
 
+    /// Return the connections state.
+    pub fn state(&self) -> ConnState {
+        self.state
+    }
+
     /// Check if the credit information the peer has last received from us is outdated.
     fn peer_needs_credit_update(&self) -> bool {
         (self.fwd_cnt - self.last_fwd_cnt_to_peer).0 as usize >= defs::CONN_CREDIT_UPDATE_THRESHOLD

src/devices/src/virtio/vsock/csm/mod.rs

@@ -36,7 +36,7 @@ pub enum Error {
 type Result<T> = std::result::Result<T, Error>;
 
 /// A vsock connection state.
-#[derive(Debug, PartialEq)]
+#[derive(Clone, Copy, Debug, PartialEq)]
 pub enum ConnState {
     /// The connection has been initiated by the host end, but is yet to be confirmed by the guest.
     LocalInit,

src/devices/src/virtio/vsock/unix/muxer.rs

@@ -35,6 +35,7 @@ use std::io::Read;
 use std::os::unix::io::{AsRawFd, RawFd};
 use std::os::unix::net::{UnixListener, UnixStream};
 
+use super::super::csm::ConnState;
 use super::super::defs::uapi;
 use super::super::packet::VsockPacket;
 use super::super::{
@@ -625,9 +626,20 @@ impl VsockMuxer {
         if let Some(conn) = self.conn_map.get_mut(&key) {
             let had_rx = conn.has_pending_rx();
             let was_expiring = conn.will_expire();
+            let prev_state = conn.state();
 
             mut_fn(conn);
 
+            // If this is a host-initiated connection that has just become established, we'll have
+            // to send an ack message to the host end.
+            if prev_state == ConnState::LocalInit && conn.state() == ConnState::Established {
+                conn.send_bytes(format!("OK {}\n", key.local_port).as_bytes())
+                    .unwrap_or_else(|err| {
+                        conn.kill();
+                        warn!("vsock: unable to ack host connection: {:?}", err);
+                    });
+            }
+
             // If the connection wasn't previously scheduled for RX, add it to our RX queue.
             if !had_rx && conn.has_pending_rx() {
                 self.rxq.push(MuxerRx::ConnRx(key));
@@ -885,6 +897,10 @@ mod tests {
             self.init_pkt(local_port, peer_port, uapi::VSOCK_OP_RESPONSE);
             self.send();
 
+            let mut buf = vec![0u8; 32];
+            let len = stream.read(&mut buf[..]).unwrap();
+            assert_eq!(&buf[..len], format!("OK {}\n", local_port).as_bytes());
+
             (stream, local_port)
         }
     }

tests/integration_tests/functional/test_vsock.py

@@ -19,6 +19,7 @@
 from select import select
 from socket import socket, AF_UNIX, SOCK_STREAM
 from threading import Thread, Event
+import re
 
 from host_tools.network import SSHConnection
 
@@ -306,4 +307,7 @@ def _vsock_connect_to_guest(uds_path, port):
     buf = bytearray("CONNECT {}\n".format(port).encode("utf-8"))
     sock.send(buf)
 
+    ack_buf = sock.recv(32)
+    assert re.match("^OK [0-9]+\n$", ack_buf.decode('utf-8')) is not None
+
     return sock