When using --tls=require, we need to consume any payload sent by the client on an option other than NBD_OPT_STARTTLS (for example, NBD_OPT_INFO); otherwise we will be out of sync and unable to read the next NBD_OPT_, most likely preventing us from being able to establish TLS if the client next attempts NBD_OPT_STARTTLS. Also, if the client sends NBD_OPT_EXPORT_NAME, they are expecting us to disconnect on failure, rather than trying to reply with an error (most clients use NBD_OPT_GO these days because of that). While it is unfortunate that a MitM proxy attacker can use this weakness in nbdkit to prevent a TLS connection, it is not quite the same as the prior fix for CVE-2021-3716 (commit 09a13daf) where a plaintext NBD_OPT_STRUCTURED_REPLY could confuse a client; this is because failure to establish TLS is easily detected at the client as a proxy attack, and not a situation where the plaintext injection can break client behavior without detection. This bug has been latent for some time; that is because most people do not try to connect a plaintext client to a server that requires TLS; and even if it is done accidentally, clients like qemu or libnbd that give a payload-free option request of NBD_OPT_STRUCTURED_REPLY first don't tickle the problem, especially if they disconnect on getting the NBD_REP_ERR_TLS_REQD rather than trying to proceed with another NBD_OPT_*. But with an upcoming libnbd release adding a new API nbd_opt_starttls() to write a SELECTIVETLS client, it becomes easier to tickle. See also qemu commit d1129a8a. Fixes: c5e76492 ("Add TLS support.", v1.1.15) --- server/protocol-handshake-newstyle.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index 5ac45efa..4e16b72c 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -424,8 +424,16 @@ negotiate_handshake_newstyle_options (void) */ if (tls == 2 && !conn->using_tls && !(option == NBD_OPT_ABORT || option == NBD_OPT_STARTTLS)) { + if (option == NBD_OPT_EXPORT_NAME) { + debug ("newstyle negotiation: can't reply NBD_REP_ERR_TLS_REQD to %s", + name_of_nbd_opt (option)); + return -1; + } if (send_newstyle_option_reply (option, NBD_REP_ERR_TLS_REQD)) return -1; + if (conn_recv_full (data, optlen, + "waiting for starttls: %m") == -1) + return -1; continue; } -- 2.37.3