[Libguestfs] [nbdkit PATCH] server: CVE-2021-???? reset structured replies on starttls

[Eric Blake]

On Mon, Aug 16, 2021 at 01:50:46PM -0500, Eric Blake wrote:
> https://nostarttls.secvuln.info/ pointed out a common implementation
> flaw in various SMTP and IMAP servers with regards to improperly
> caching plaintext state across the STARTTLS encryption boundary.  It
> turns out that nbdkit has the same vulnerability in regards to the NBD
> protocol: an attacker is able to inject a plaintext
> NBD_OPT_STRUCTURED_REPLY before proxying everything else a client
> sends to the server; if the server then acts on that plaintext request
> (as nbdkit did before this patch), then the server ends up sending
> structured replies to at least NBD_CMD_READ, even though the client
> was not expecting them.  The NBD spec has been recently tightened to
> declare the nbdkit behavior to be a security hole.
> 
> ---
> 
> [NB: I'm still in the process of getting a CVE assigned; there is no
> embargo since the issue is already public, but I may wait to apply
> this patch until the commit message can be tweaked]
> ---
>  server/protocol-handshake-newstyle.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c
> index a2c89c9a..7e6b7b1b 100644
> --- a/server/protocol-handshake-newstyle.c
> +++ b/server/protocol-handshake-newstyle.c
> @@ -495,7 +495,8 @@ negotiate_handshake_newstyle_options (void)
>            return -1;
>          conn->using_tls = true;
>          debug ("using TLS on this connection");
> -        /* Wipe out any cached default export name. */
> +        /* Wipe out any cached state. */
> +        conn->structured_replies = false;
>          for_each_backend (b) {
>            free (conn->default_exportname[b->i]);
>            conn->default_exportname[b->i] = NULL;

While backporting this patch, I found one more piece of state that
nbdkit accidentally preserved across STARTTLS:
conn->meta_context_base_allocation.  Fortunately, this one does not
change the protocol the server speaks to a compliant client: since a
client should never use NBD_CMD_BLOCK_STATUS unless it first used
NBD_OPT_SET_META_CONTEXT beforehand, an injection of SET_META_CONTEXT
by MitM will either never be noticed (the client doesn't want to use
block status) or will be overwritten (when the client itself
negotiates meta context after the encryption is set up).  Thus, while
related to the CVE, it is not in the same category as the
STRUCTURED_REPLY injection (where a completely compliant client still
gets unexpected traffic from the server).  I'll go ahead and patch it
upstream, and backport it as well.  But it missed the 1.27.7, 1.26.5,
and 1.24.6 releases, yet I don't see it as a show-stopper worth
cutting another release.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org