debian/patches/extra/0019-nbd-Fix-regression-on-resiliency-to-port-scan.patch

   1 From 97c7e46a9f8ae03e24df1d18d3b5e9df420f39ce Mon Sep 17 00:00:00 2001
   2 From: Eric Blake <eblake@redhat.com>
   3 Date: Thu, 8 Jun 2017 17:26:17 -0500
   4 Subject: [PATCH 19/23] nbd: Fix regression on resiliency to port scan
   5
   6 Back in qemu 2.5, qemu-nbd was immune to port probes (a transient
   7 server would not quit, regardless of how many probe connections
   8 came and went, until a connection actually negotiated).  But we
   9 broke that in commit ee7d7aa when removing the return value to
  10 nbd_client_new(), although that patch also introduced a bug causing
  11 an assertion failure on a client that fails negotiation.  We then
  12 made it worse during refactoring in commit 1a6245a (a segfault
  13 before we could even assert); the (masked) assertion was cleaned
  14 up in d3780c2 (still in 2.6), and just recently we finally fixed
  15 the segfault ("nbd: Fully intialize client in case of failed
  16 negotiation").  But that still means that ever since we added
  17 TLS support to qemu-nbd, we have been vulnerable to an ill-timed
  18 port-scan being able to cause a denial of service by taking down
  19 qemu-nbd before a real client has a chance to connect.
  20
  21 Since negotiation is now handled asynchronously via coroutines,
  22 we no longer have a synchronous point of return by re-adding a
  23 return value to nbd_client_new().  So this patch instead wires
  24 things up to pass the negotiation status through the close_fn
  25 callback function.
  26
  27 Simple test across two terminals:
  28 $ qemu-nbd -f raw -p 30001 file
  29 $ nmap 127.0.0.1 -p 30001 && \
  30   qemu-io -c 'r 0 512' -f raw nbd://localhost:30001
  31
  32 Note that this patch does not change what constitutes successful
  33 negotiation (thus, a client must enter transmission phase before
  34 that client can be considered as a reason to terminate the server
  35 when the connection ends).  Perhaps we may want to tweak things
  36 in a later patch to also treat a client that uses NBD_OPT_ABORT
  37 as being a 'successful' negotiation (the client correctly talked
  38 the NBD protocol, and informed us it was not going to use our
  39 export after all), but that's a discussion for another day.
  40
  41 Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1451614
  42
  43 Signed-off-by: Eric Blake <eblake@redhat.com>
  44 Message-Id: <20170608222617.20376-1-eblake@redhat.com>
  45 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
  46 ---
  47  blockdev-nbd.c      |  6 +++++-
  48  include/block/nbd.h |  2 +-
  49  nbd/server.c        | 24 +++++++++++++++---------
  50  qemu-nbd.c          |  4 ++--
  51  4 files changed, 23 insertions(+), 13 deletions(-)
  52
  53 diff --git a/blockdev-nbd.c b/blockdev-nbd.c
  54 index 8a11807df3..8d7284ac56 100644
  55 --- a/blockdev-nbd.c
  56 +++ b/blockdev-nbd.c
  57 @@ -27,6 +27,10 @@ typedef struct NBDServerData {
  58
  59  static NBDServerData *nbd_server;
  60
  61 +static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
  62 +{
  63 +    nbd_client_put(client);
  64 +}
  65
  66  static gboolean nbd_accept(QIOChannel *ioc, GIOCondition condition,
  67                             gpointer opaque)
  68 @@ -46,7 +50,7 @@ static gboolean nbd_accept(QIOChannel *ioc, GIOCondition condition,
  69      qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
  70      nbd_client_new(NULL, cioc,
  71                     nbd_server->tlscreds, NULL,
  72 -                   nbd_client_put);
  73 +                   nbd_blockdev_client_closed);
  74      object_unref(OBJECT(cioc));
  75      return TRUE;
  76  }
  77 diff --git a/include/block/nbd.h b/include/block/nbd.h
  78 index 3e373f0498..b69c30d063 100644
  79 --- a/include/block/nbd.h
  80 +++ b/include/block/nbd.h
  81 @@ -160,7 +160,7 @@ void nbd_client_new(NBDExport *exp,
  82                      QIOChannelSocket *sioc,
  83                      QCryptoTLSCreds *tlscreds,
  84                      const char *tlsaclname,
  85 -                    void (*close)(NBDClient *));
  86 +                    void (*close_fn)(NBDClient *, bool));
  87  void nbd_client_get(NBDClient *client);
  88  void nbd_client_put(NBDClient *client);
  89
  90 diff --git a/nbd/server.c b/nbd/server.c
  91 index edfda84d43..a98bb21a0a 100644
  92 --- a/nbd/server.c
  93 +++ b/nbd/server.c
  94 @@ -81,7 +81,7 @@ static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
  95
  96  struct NBDClient {
  97      int refcount;
  98 -    void (*close)(NBDClient *client);
  99 +    void (*close_fn)(NBDClient *client, bool negotiated);
 100
 101      bool no_zeroes;
 102      NBDExport *exp;
 103 @@ -796,7 +796,7 @@ void nbd_client_put(NBDClient *client)
 104      }
 105  }
 106
 107 -static void client_close(NBDClient *client)
 108 +static void client_close(NBDClient *client, bool negotiated)
 109  {
 110      if (client->closing) {
 111          return;
 112 @@ -811,8 +811,8 @@ static void client_close(NBDClient *client)
 113                           NULL);
 114
 115      /* Also tell the client, so that they release their reference.  */
 116 -    if (client->close) {
 117 -        client->close(client);
 118 +    if (client->close_fn) {
 119 +        client->close_fn(client, negotiated);
 120      }
 121  }
 122
 123 @@ -993,7 +993,7 @@ void nbd_export_close(NBDExport *exp)
 124
 125      nbd_export_get(exp);
 126      QTAILQ_FOREACH_SAFE(client, &exp->clients, next, next) {
 127 -        client_close(client);
 128 +        client_close(client, true);
 129      }
 130      nbd_export_set_name(exp, NULL);
 131      nbd_export_set_description(exp, NULL);
 132 @@ -1355,7 +1355,7 @@ done:
 133
 134  out:
 135      nbd_request_put(req);
 136 -    client_close(client);
 137 +    client_close(client, true);
 138      nbd_client_put(client);
 139  }
 140
 141 @@ -1381,7 +1381,7 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
 142      qemu_co_mutex_init(&client->send_lock);
 143
 144      if (nbd_negotiate(data)) {
 145 -        client_close(client);
 146 +        client_close(client, false);
 147          goto out;
 148      }
 149
 150 @@ -1391,11 +1391,17 @@ out:
 151      g_free(data);
 152  }
 153
 154 +/*
 155 + * Create a new client listener on the given export @exp, using the
 156 + * given channel @sioc.  Begin servicing it in a coroutine.  When the
 157 + * connection closes, call @close_fn with an indication of whether the
 158 + * client completed negotiation.
 159 + */
 160  void nbd_client_new(NBDExport *exp,
 161                      QIOChannelSocket *sioc,
 162                      QCryptoTLSCreds *tlscreds,
 163                      const char *tlsaclname,
 164 -                    void (*close_fn)(NBDClient *))
 165 +                    void (*close_fn)(NBDClient *, bool))
 166  {
 167      NBDClient *client;
 168      NBDClientNewData *data = g_new(NBDClientNewData, 1);
 169 @@ -1412,7 +1418,7 @@ void nbd_client_new(NBDExport *exp,
 170      object_ref(OBJECT(client->sioc));
 171      client->ioc = QIO_CHANNEL(sioc);
 172      object_ref(OBJECT(client->ioc));
 173 -    client->close = close_fn;
 174 +    client->close_fn = close_fn;
 175
 176      data->client = client;
 177      data->co = qemu_coroutine_create(nbd_co_client_start, data);
 178 diff --git a/qemu-nbd.c b/qemu-nbd.c
 179 index b44764eb87..483dd77a77 100644
 180 --- a/qemu-nbd.c
 181 +++ b/qemu-nbd.c
 182 @@ -335,10 +335,10 @@ static void nbd_export_closed(NBDExport *exp)
 183
 184  static void nbd_update_server_watch(void);
 185
 186 -static void nbd_client_closed(NBDClient *client)
 187 +static void nbd_client_closed(NBDClient *client, bool negotiated)
 188  {
 189      nb_fds--;
 190 -    if (nb_fds == 0 && !persistent && state == RUNNING) {
 191 +    if (negotiated && nb_fds == 0 && !persistent && state == RUNNING) {
 192          state = TERMINATE;
 193      }
 194      nbd_update_server_watch();
 195 --
 196 2.11.0
 197