Similar to the earlier patch for structured reply errors, it's worth logging (at least through debug) any error messages that the server sends alongside NBD_REP_ERR. And given our earlier changes, it doesn't really impact the size of struct nbd_handle. --- generator/states-newstyle-opt-go.c | 5 ++-- .../states-newstyle-opt-set-meta-context.c | 6 +++- generator/states-newstyle-opt-starttls.c | 8 ++--- .../states-newstyle-opt-structured-reply.c | 6 +++- generator/states-newstyle.c | 29 +++++++++++++++++++ lib/internal.h | 1 + 6 files changed, 47 insertions(+), 8 deletions(-) diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c index 3458f09..91a85ef 100644 --- a/generator/states-newstyle-opt-go.c +++ b/generator/states-newstyle-opt-go.c @@ -147,9 +147,10 @@ SET_NEXT_STATE (%^OPT_EXPORT_NAME.START); return 0; default: + if (handle_reply_error (h) == 0) + set_error (0, "handshake: unknown reply from NBD_OPT_GO: 0x%" PRIx32, + reply); SET_NEXT_STATE (%.DEAD); - set_error (0, "handshake: unknown reply from NBD_OPT_GO: 0x%" PRIx32, - reply); return -1; } diff --git a/generator/states-newstyle-opt-set-meta-context.c b/generator/states-newstyle-opt-set-meta-context.c index 94120e2..a00a411 100644 --- a/generator/states-newstyle-opt-set-meta-context.c +++ b/generator/states-newstyle-opt-set-meta-context.c @@ -200,7 +200,11 @@ break; default: /* Anything else is an error, ignore it */ - /* XXX display any error message if NBD_REP_ERR_? */ + if (handle_reply_error (h) == -1) { + SET_NEXT_STATE (%.DEAD); + return -1; + } + debug (h, "handshake: unexpected error from " "NBD_OPT_SET_META_CONTEXT (%" PRIu32 ")", reply); SET_NEXT_STATE (%^OPT_GO.START); diff --git a/generator/states-newstyle-opt-starttls.c b/generator/states-newstyle-opt-starttls.c index e994ffd..61f254f 100644 --- a/generator/states-newstyle-opt-starttls.c +++ b/generator/states-newstyle-opt-starttls.c @@ -83,9 +83,10 @@ return 0; default: - if (!NBD_REP_IS_ERR (reply)) - debug (h, - "server is confused by NBD_OPT_STARTTLS, continuing anyway"); + if (handle_reply_error (h) == -1) { + SET_NEXT_STATE (%.DEAD); + return -1; + } /* Server refused to upgrade to TLS. If h->tls is not require (2) * then we can continue unencrypted. @@ -100,7 +101,6 @@ debug (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; } diff --git a/generator/states-newstyle-opt-structured-reply.c b/generator/states-newstyle-opt-structured-reply.c index d755411..65d5958 100644 --- a/generator/states-newstyle-opt-structured-reply.c +++ b/generator/states-newstyle-opt-structured-reply.c @@ -67,7 +67,11 @@ h->structured_replies = true; break; default: - /* XXX: capture instead of skip server's payload to NBD_REP_ERR*? */ + if (handle_reply_error (h) == -1) { + SET_NEXT_STATE (%.DEAD); + return -1; + } + debug (h, "structured replies are not supported by this server"); h->structured_replies = false; break; diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c index 369df0f..8a4eec6 100644 --- a/generator/states-newstyle.c +++ b/generator/states-newstyle.c @@ -77,6 +77,35 @@ prepare_for_reply_payload (struct nbd_handle *h, uint32_t opt) return 0; } +/* Check an unexpected server reply. If it is an error, log any + * message from the server and return 0; otherwise, return -1. + */ +static int +handle_reply_error (struct nbd_handle *h) +{ + uint32_t len; + uint32_t reply; + + len = be32toh (h->sbuf.or.option_reply.replylen); + reply = be32toh (h->sbuf.or.option_reply.reply); + if (!NBD_REP_IS_ERR (reply)) { + set_error (0, "handshake: unexpected option reply type %d", reply); + return -1; + } + + assert (NBD_MAX_STRING < sizeof h->sbuf.or.payload); + if (len > NBD_MAX_STRING) { + set_error (0, "handshake: option error string too long"); + return -1; + } + + if (len > 0) + debug (h, "handshake: server error message: %.*s", (int) len, + h->sbuf.or.payload.err_msg); + + return 0; +} + /* State machine for parsing the fixed newstyle handshake. */ /* STATE MACHINE */ { diff --git a/lib/internal.h b/lib/internal.h index 5f19279..9acc1b4 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -127,6 +127,7 @@ struct nbd_handle { struct nbd_fixed_new_option_reply_meta_context context; char str[NBD_MAX_STRING]; } __attribute__((packed)) context; + char err_msg[NBD_MAX_STRING]; } payload; } __attribute__((packed)) or; struct nbd_export_name_option_reply export_name_reply; -- 2.20.1