Skip to content
GitLab
Switch branch/tag
  1. 07 Mar, 2022 10 commits
  3. 06 Mar, 2022 4 commits
  5. 03 Mar, 2022 10 commits
  7. 13 Jan, 2022 2 commits
  9. 18 Oct, 2021 2 commits
  11. 04 Oct, 2021 1 commit
  13. 03 Sep, 2021 4 commits
    • Eric Blake's avatar
      spec: Relax NBD_OPT_LIST_META_CONTEXTS · 13a4e33a
      Eric Blake authored 
      Using OPT_SET_META_CONTEXTS is stateful (it is documented to wipe out
any previously-requested contexts, and we just tightened the spec to
clarify that starting TLS also wipes it out).  But
OPT_LIST_META_CONTEXTS is not stateful; and in fact, with a
SELECTIVETLS server, it can be handy to list the meta contexts
available on an unencrypted export, then enable encryption, and then
further list what contexts are available on encrypted exports (as the
server is permitted to let those lists differ).  Although such a
client must negotiate structured replies after starttls if it is going
to actually connect to an export, this change permits the client to
shorten the handshake by two commands if it is only being used to list
available exports and their meta contexts before disconnecting.
      13a4e33a
    • Eric Blake's avatar
      client: Don't confuse Unix socket with TLS hostname · d8a6d0b0
      Eric Blake authored 
      When using -u but not -H, we were ending up calling
gnutls_session_set_verify_cert() with the Unix socket's path name,
which is bound to fail (hostnames don't start with /).  Saner is to
only default tlshostname when using TCP sockets.

See also https://gitlab.com/nbdkit/nbdkit/-/issues/1

, as this was
detected during an attempt to prove TLS interoperability between
nbd-client and nbdkit.  Pre-patch, I have to add '-H localhost' to the
nbd-client command line when using nbdkit with a Unix socket, but not
when using a TCP socket; post-patch, I can omit -H and still connect
/dev/nbd0 over TLS using either TCP or Unix.

Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      d8a6d0b0
    • Eric Blake's avatar
      server: Avoid deprecated g_memdup · ecada459
      Eric Blake authored 
      glib now recommends that we use g_memdup2() to avoid accidental 32-bit
truncation bugs on platforms where g_size is larger than guint:

https://discourse.gnome.org/t/port-your-module-from-g-memdup-to-g-memdup2-now/5538

and failure to do so causes noisy compilation due to deprecation
warnings with glib 2.68:

nbd-server.c: In function ‘parse_cfile’:
nbd-server.c:1010:25: warning: ‘g_memdup’ is deprecated: Use 'g_memdup2' instead [-Wdeprecated-declarations]
 1010 |                         SERVER *srv = serve_inc_ref(g_memdup(&s, sizeof(SERVER)));
      |                         ^~~~~~
In file included from /usr/include/glib-2.0/glib.h:82,
                 from nbd-server.c:117:
/usr/include/glib-2.0/glib/gstrfuncs.h:257:23: note: declared here
  257 | gpointer              g_memdup         (gconstpointer mem,
      |                       ^~~~~~~~

Of course, we still want to build on platforms with older glib that
lack g_memdup2().  Thankfully, it's easy enough to audit that all our
current uses of g_memdup() do not overflow 32 bits.
      ecada459
    • Wouter Verhelst's avatar
      Produce a helpful error message when autoconf-archive isn't installed. · 0d937d09
      Wouter Verhelst authored
      0d937d09
  15. 25 Aug, 2021 1 commit
    • Eric Blake's avatar
      spec: Clarify NBD_FLAG_ vs. MitM downgrade attack · ac88ec74
      Eric Blake authored 
      Codify the fact that downgrade attacks are possible not only by
manipulation of NBD_OPT_STARTTLS, but also by manipulation of the
NBD_FLAG[_C]_* handshake flags.  To ensure we don't accidentally
introduce a new MitM attack vector, we want the specification to
clearly document that controlling any new protocol changes prior to
TLS is unwise, and therefore we are unlikely to add any new handshake
flags.

Viewed from another perspective, the 16 bits for handshake flags
control the protocol used during NBD_OPT_*, but what we have with
NBD_FLAG_FIXED_NEWSTYLE is already fairly robust for future extension
(since all but NBD_OPT_EXPORT_NAME encode a length, and we've gone to
great lengths to document what servers and clients should do with
unknown requests).  Meanwhile, any extension that wants to affect the
protocol used by transmission phase, such as structured replies, is
fine waiting until after TLS is started.

The expense of an extra round trip or two during NBD_OPT_ haggling
pales in comparison to the amount of data that will go over the wire
during transmission phase; and if startup efficiency really matters,
we could add a new NBD_OPT_ that does more things in one round trip
(where the fallback is still the older one-at-a-time approach).
      ac88ec74
  17. 16 Aug, 2021 1 commit
    • Eric Blake's avatar
      spec: Clarify STARTTLS vs. STRUCTURED_REPLY · 77e55378
      Eric Blake authored 
      Consider a SELECTIVETLS server and a MitM attacker, under the
following NBD_OPT_ handshake scenario:

 client:                  mitm:                    server:
 > _STARTTLS
                          > _STRUCTURED_REPLY
                                                   < _REP_ACK
                          > _STARTTLS
                                                   < _REP_ACK
                          < _REP_ACK
 > _GO
                          > _GO
                                                   < _REP_ACK
                          < _REP_ACK
 > NBD_CMD_READ

In this scenario, the client is NOT expecting structured replies from
the server, but if the server feels obligated to send them based on
the plaintext negotiation, it may confuse the client.  The MitM
attacker was thus able to corrupt the connection, even without having
any encryption keys.  The only sane approach is to forbid ALL stateful
negotiations from having any effect post-encryption (the MitM's
injected packet is effectively ignored, and the client proceeds
without structured replies).

Unfortunately, nbdkit 1.26.0 is buggy in this regards - a CVE will be
opened against that product.  nbd-server does not yet understand
NBD_OPT_STRUCTURED_REPLY, and qemu as server does not use SELECTIVETLS
mode, so they are immune.
      77e55378
  19. 12 Aug, 2021 1 commit
    • Eric Blake's avatar
      spec: Clarify STARTTLS vs. SET_META_CONTEXT interaction · b7cb250c
      Eric Blake authored 
      Consider a SELECTIVETLS server and a MitM attacker, under the
following NBD_OPT_ handshake scenario:

 client:                  mitm:                    server:
 > _STARTTLS
                          > _SET_META_CONTEXT("A")
                                                   < _REP_META_CONTEXT
                                                   < _REP_ACK
                          > _STARTTLS
                                                   < _REP_ACK
                          < _REP_ACK
 > _SET_META_CONTEXT("B")
                          < _REP_META_CONTEXT
                          < _REP_ACK
 > _GO
                          > _GO
                                                   < _REP_ACK
                          < _REP_ACK
 > NBD_CMD_BLOCK_STATUS

While this scenario requires the MitM to be able to use encryption to
speak to the client (and thus a less likely scenario than a true
protocol downgrade or plaintext buffering attack), it results in a
situation where the client is asking for information on context "B",
but where the server only saw a request for context "A", which may
result in the client interpreting the results of BLOCK_STATUS
incorrectly even though it is coming over an encrypted connection.

The safest fix to this is to require that a server cannot use any meta
context requests from prior to enabling encryption with any successful
NBD_OPT_GO after encryption.  At this point, the spec already states
that the server should then return an error (the client is asking for
block status without proper negotiation), which is better than letting
the client blindly misinterpret a response sent for a different meta
context.

To date, the only known server that has implemented TLS with
SELECTIVETLS mode as well as support for NBD_OPT_SET_META_CONTEXT is
nbdkit (qemu-nbd only has FORCEDTLS mode, and nbd-server lacks meta
context support); thankfully, that implementation is in already line
with this stricter requirement.
      b7cb250c
  21. 11 Aug, 2021 1 commit
    • Eric Blake's avatar
      docs: Link to protocol security considerations in uri docs · f44065cf
      Eric Blake authored 
      Especially useful in light of the recent publishing of
https://nostarttls.secvuln.info/, which documents a variety of
implementations vulnerable to downgrade attacks in SMTP and IMAP, as
well as its caution that that any protocol with a STARTTLS operation
(which includes NBD) needs to be aware of the potential for downgrade
attacks.

The NBD protocol documentation already covers what is necessary to
avoid the effects of a downgrade attack, and all known implementations
of NBD servers and clients with working NBD_OPT_STARTTLS have at least
one mode where TLS is mandatory rather than opportunistic.  So I don't
see this as a CVE against the NBD protocol itself, so much as a worry
about the potential for future poor implementations that disregard the
documentation.
      f44065cf
  23. 10 Aug, 2021 1 commit
  25. 08 Aug, 2021 1 commit
  27. 05 Aug, 2021 1 commit