[Libguestfs] [PATCH] spec: Clarify STARTTLS vs. SET_META_CONTEXT interaction

[Eric Blake]

On Mon, Aug 16, 2021 at 05:31:10PM +0200, Wouter Verhelst wrote:

> > +++ b/doc/proto.md
> > @@ -1165,6 +1165,14 @@ of the newstyle negotiation.
> >      permitted by this document (for example, `NBD_REP_ERR_INVALID` if
> >      the client included data).
> > 
> > +    When this command succeeds, the server MUST NOT preserve any
> > +    per-export state (such as metadata contexts from
> > +    `NBD_OPT_SET_META_CONTEXT`) issued before this command.  The
> > +    server MAY preserve global state such as a client request for
> > +    `NBD_OPT_STRUCTURED_REPLY`; however, a client SHOULD defer other
> > +    stateful option requests until after it determines whether
> > +    encryption is available.
> 
> I'm not sure I think that makes sense. It's safer to require that
> STARTTLS wipes out everything.
> 
> There are two reasonable ways of communicating with a server that
> supports opportunistic TLS: either you enable TLS as soon as possible
> after opening the connection (and then you should do any state
> modification after the STARTTLS command), or you don't do any STARTTLS
> at all, ever (and then all state settings are done in the unencrypted
> connection). Anything else seems like a silly idea.

Concur.

> 
> As such, I think trying to support ways in which you configure things
> before STARTTLS, then do STARTTLS, and then expect things to retain
> state, is bound to invite security issues, and we should not even try to
> support it.

Okay, how about this wording:

+When this command succeeds, the server MUST NOT preserve any
+negotiation state (such as a request for `NBD_OPT_STRUCTURED_REPLY`,
+or metadata contexts from `NBD_OPT_SET_META_CONTEXT`) issued before
+this command.  A client SHOULD defer all stateful option requests
+until after it determines whether encryption is available.

Unfortunately, nbdkit in opportunistic mode does not (yet) obey that:
a request for structured replies prior to starttls is currently
preserved across the jump into encryption - which may result in the
server sending structured replies to a client not expecting them due
to a MitM plaintext injection.  I'll be submitting a patch for that
soon; sounds like we found a CVE in nbdkit.

On the other hand, qemu as NBD client always sends NBD_OPT_STARTTLS
first (disconnecting rather than falling back to plaintext).  libnbd
as NBD client does not (yet) expose any way to attempt starttls after
other negotiation commands: in opportunistic mode, it currently probes
for encryption before giving the user any control over other NBD_OPT_
commands.  At one point, I had the idea of expanding the libnbd API to
make it easier to try even NBD_OPT_STARTTLS under user control (which
would make it easier to test issues like how servers behave with
clients that don't lead off with STARTTLS), but that has not been
implemented yet, so at the moment, we don't have clients in the wild
that were relying on nbdkit's stateful behavior.

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