[Nbd] [PATCH v2 6/6] server: Read client's TLS length data before next option

[Eric Blake <eblake@...696...>]

Any client attempting to probe support for a new option, such as
NBD_OPT_STARTTLS or NBD_OPT_GO, with plans to do a graceful
fallback to older methods, will fail in its attempt if the server
does not consume the length field and potential payload of the
unrecognized (or rejected) option, because the server will then
be reading out of sync and not see the client's magic for the
next option.  While it is true that sane clients are unlikely to
send more than one NBD_OPT_STARTTLS, and thus never trigger some
of the paths in this patch, it is still better to be robust for
all clients.

Furthermore, even if the server requires TLS, and rejects all but
NBD_OPT_STARTTLS as the first valid option, it should still honor
NBD_OPT_ABORT.

Signed-off-by: Eric Blake <eblake@...696...>
---
 nbd-server.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/nbd-server.c b/nbd-server.c
index 4b0692d..d5b2013 100644
--- a/nbd-server.c
+++ b/nbd-server.c
@@ -1507,6 +1507,11 @@ CLIENT* negotiate(int net, GArray* servers, const gchar* tlsdir) {
 				// so must do hard close
 				goto hard_close;
 			}
+			if(opt == NBD_OPT_ABORT) {
+				// handled below
+				break;
+			}
+			consume_len(client);
 			send_reply(client, opt, NBD_REP_ERR_TLS_REQD, -1, "TLS is required on this server");
 			continue;
 		}
@@ -1529,11 +1534,14 @@ CLIENT* negotiate(int net, GArray* servers, const gchar* tlsdir) {
 			break;
 		case NBD_OPT_STARTTLS:
 			if(client->tls_session != NULL) {
+				consume_len(client);
 				send_reply(client, opt, NBD_REP_ERR_INVALID, -1, "Invalid STARTTLS request: TLS has already been negotiated!");
 				continue;
 			}
 			if(tlsdir == NULL) {
+				consume_len(client);
 				send_reply(client, opt, NBD_REP_ERR_POLICY, -1, "TLS not allowed on this server");
+				continue;
 			}
 			if(handle_starttls(client, opt, servers, cflags, tlsdir) == NULL) {
 				// can't recover from failed TLS negotiation.
-- 
2.7.4