[Libguestfs] [libnbd PATCH 1/4] starttls: Skip error payload if falling back to unencrypted

[Eric Blake]

If we tried but did not require TLS, and the NBD server rejected our
attempt at NBD_OPT_STARTTLS but included an error message, then we
MUST capture (or skip) that message off the wire before proceeding on
with attempting structured replies. This requires a new state, as well
as changing the rejection of payload length to only happen on
NBD_REP_ACK.
---
 generator/generator                      |  7 ++++++
 generator/states-newstyle-opt-starttls.c | 27 +++++++++++++++++++++---
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/generator/generator b/generator/generator
index d5b5f5f..3faa272 100755
--- a/generator/generator
+++ b/generator/generator
@@ -352,6 +352,13 @@ and newstyle_opt_starttls_state_machine = [
     external_events = [ NotifyRead, "" ];
   };

+  State {
+    default_state with
+    name = "SKIP_REPLY_PAYLOAD";
+    comment = "Skip newstyle NBD_OPT_STARTTLS reply payload";
+    external_events = [ NotifyRead, "" ];
+  };
+
   State {
     default_state with
     name = "CHECK_REPLY";
diff --git a/generator/states-newstyle-opt-starttls.c b/generator/states-newstyle-opt-starttls.c
index f44b654..f56553c 100644
--- a/generator/states-newstyle-opt-starttls.c
+++ b/generator/states-newstyle-opt-starttls.c
@@ -45,9 +45,23 @@
   return 0;

  NEWSTYLE.OPT_STARTTLS.RECV_REPLY:
+  uint32_t len;
+
   switch (recv_into_rbuf (conn)) {
   case -1: SET_NEXT_STATE (%.DEAD); return -1;
-  case 0: SET_NEXT_STATE (%CHECK_REPLY);
+  case 0:
+    /* Discard the payload if there is one. */
+    len = be32toh (conn->sbuf.or.option_reply.replylen);
+    conn->rbuf = NULL;
+    conn->rlen = len;
+    SET_NEXT_STATE (%SKIP_REPLY_PAYLOAD);
+  }
+  return 0;
+
+ NEWSTYLE.OPT_STARTTLS.SKIP_REPLY_PAYLOAD:
+  switch (recv_into_rbuf (conn)) {
+  case -1: SET_NEXT_STATE (%.DEAD); return -1;
+  case 0:  SET_NEXT_STATE (%CHECK_REPLY);
   }
   return 0;

@@ -62,13 +76,19 @@
   option = be32toh (conn->sbuf.or.option_reply.option);
   reply = be32toh (conn->sbuf.or.option_reply.reply);
   len = be32toh (conn->sbuf.or.option_reply.replylen);
-  if (magic != NBD_REP_MAGIC || option != NBD_OPT_STARTTLS || len != 0) {
+  if (magic != NBD_REP_MAGIC || option != NBD_OPT_STARTTLS) {
     SET_NEXT_STATE (%.DEAD);
-    set_error (0, "handshake: invalid option reply magic, option or length");
+    set_error (0, "handshake: invalid option reply magic or option");
     return -1;
   }
   switch (reply) {
   case NBD_REP_ACK:
+    if (len != 0) {
+      SET_NEXT_STATE (%.DEAD);
+      set_error (0, "handshake: invalid option reply length");
+      return -1;
+    }
+
     new_sock = nbd_internal_crypto_create_session (conn, conn->sock);
     if (new_sock == NULL) {
       SET_NEXT_STATE (%.DEAD);
@@ -99,6 +119,7 @@
     debug (conn->h,
            "server refused TLS (%s), continuing with unencrypted connection",
            reply == NBD_REP_ERR_POLICY ? "policy" : "not supported");
+    /* XXX: capture instead of skip server's payload to NBD_REP_ERR*? */
     SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
     return 0;
   }
-- 
2.20.1