Skip to content

Commit

Permalink
security: states: Fail oldstyle servers when tls==2
Browse files Browse the repository at this point in the history
This test succeeds, which is wrong:

$ nbdsh -c 'h.set_tls(nbd.TLS_REQUIRE)' \
  -c 'h.connect_command(["nbdkit", "-o", "-s", "null"])' \
  -c 'print(h.get_size())'
0

Consider the case of a server that allows, but does not require, TLS
encryption.  A client that wants to only use the server if encrypted
(as evidenced by the request for LIBNBD_TLS_REQUIRE) can be subjected
to a protocol downgrade attack: a man-in-the-middle attacker can
translate the original server's unencrypted newstyle offerings into an
oldstyle server, such that the client is now unaware that it is
sending plain-text rather than the desired encrypted session.

Red Hat security will probably assign this a CVE, but we felt it
reasonable to publish the fix now, in part due to the rarity of
oldstyle servers these days.

Workaround: if the server offers extensions that are only possible in
newstyle connections, a client can check post-connection but before
sending any read or write requests that any of those extensions are
enabled, to ensure that a newstyle connection is in use
(unfortunately, this doesn't help for all servers).  Known witnesses:
- if nbd_can_df(h) returns true
- if the client requested nbd_add_meta_context(h, context) prior to
connection, then after connection nbd_can_meta_context(h, context)
returns true (the most common context is LIBNBD_CONTEXT_BASE_ALLOCATION)
  • Loading branch information
ebblake committed Sep 16, 2019
1 parent 2941878 commit cca3b10
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 0 deletions.
10 changes: 10 additions & 0 deletions generator/states-oldstyle.c
Expand Up @@ -46,6 +46,16 @@
gflags = be16toh (h->sbuf.old_handshake.gflags);
eflags = be16toh (h->sbuf.old_handshake.eflags);

/* Server is unable to upgrade to TLS. If h->tls is not require (2)
* then we can continue unencrypted.
*/
if (h->tls == 2) {
SET_NEXT_STATE (%.DEAD);
set_error (ENOTSUP, "handshake: server is oldstyle, "
"but handle TLS setting is require (2)");
return 0;
}

h->gflags = gflags;
debug (h, "gflags: 0x%" PRIx16, gflags);

Expand Down
17 changes: 17 additions & 0 deletions tests/oldstyle.c
Expand Up @@ -87,6 +87,23 @@ main (int argc, char *argv[])

progname = argv[0];

/* Initial sanity check that we can't require TLS */
nbd = nbd_create ();
if (nbd == NULL) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
if (nbd_set_tls (nbd, LIBNBD_TLS_REQUIRE) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
if (nbd_connect_command (nbd, args) != -1) {
fprintf (stderr, "%s\n", "expected failure");
exit (EXIT_FAILURE);
}
nbd_close (nbd);

/* Now for a working connection */
nbd = nbd_create ();
if (nbd == NULL) {
fprintf (stderr, "%s\n", nbd_get_error ());
Expand Down

0 comments on commit cca3b10

Please sign in to comment.