[Libguestfs] [libnbd PATCH] api: Fix block status assertion under set_strict bypass

[Eric Blake]

A compliant server should not send NBD_REPLY_TYPE_BLOCK_STATUS unless
we successfully negotiated a meta context.  And our default strictness
settings refuse to let us send NBD_CMD_BLOCK_STATUS unless we
negotiated a meta context.  But when you mix non-default settings
(using nbd_set_strict to disable STRICT_COMMANDS) to send a block
status without having negotiated it, coupled with a non-compliant
server that responds with status anyways, we can then hit the
assertion failure where h->meta_valid is not set during the
REPLY.CHUNK_REPLY.RECV_BS_ENTRIES state.

Demonstration of the assertion failure can be done with a quick patch
to nbdkit (here, on top of v1.35.6):

| diff --git i/server/protocol.c w/server/protocol.c
| index cb530e65..6b115d99 100644
| --- i/server/protocol.c
| +++ w/server/protocol.c
| @@ -190,7 +190,7 @@ validate_request (uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count,
|    }
|
|    /* Block status allowed? */
| -  if (cmd == NBD_CMD_BLOCK_STATUS) {
| +  if (cmd == NBD_CMD_BLOCK_STATUS && 0) {
|      if (!conn->structured_replies) {
|        nbdkit_error ("invalid request: "
|                      "%s: structured replies was not negotiated",
| @@ -536,7 +536,7 @@ send_structured_reply_block_status (uint64_t cookie,
|    size_t i;
|    int r;
|
| -  assert (conn->meta_context_base_allocation);
| +  //  assert (conn->meta_context_base_allocation);
|    assert (cmd == NBD_CMD_BLOCK_STATUS);
|
|    blocks = extents_to_block_descriptors (extents, flags, count, offset,

plus this sequence:

$ patched/nbdkit memory 1M
$ ./run nbdsh --opt-mode -u nbd://localhost
nbd> h.set_request_meta_context(False)
nbd> h.set_export_name('a')
nbd> def c(arg):
...   pass
...
nbd> h.opt_set_meta_context_queries(['base:allocation'], c)
1
nbd> h.set_export_name('')
nbd> h.opt_go()
nbd> h.set_strict_mode(0)
nbd> h.block_status(1024*1024, 0, c)
nbdsh: generator/states-reply-chunk.c:425: enter_STATE_REPLY_CHUNK_REPLY_RECV_BS_ENTRIES: Assertion `h->meta_valid' failed.
Aborted (core dumped)

As this is not a typical setup, and is trivially avoided by sticking
to default settings (or more safely, by using TLS connections to
trusted servers that don't reply to a spurious block status call),
this did not warrant a CVE.  However, since it is a case where a
server's reply can prompt a libnbd denial of service crash, I will be
sending a security announcement and upcoming patch be to the
libnbd-security man page once backports are in place.

Fixes: 55b09667 ("api: Fix nbd_can_meta_context if NBD_OPT_SET_META_CONTEXT fails", v1.15.3)
Signed-off-by: Eric Blake <eblake at redhat.com>
---
 generator/states-reply-chunk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/generator/states-reply-chunk.c b/generator/states-reply-chunk.c
index 02c65414..26b8a6b0 100644
--- a/generator/states-reply-chunk.c
+++ b/generator/states-reply-chunk.c
@@ -101,6 +101,7 @@  REPLY.CHUNK_REPLY.START:

   case NBD_REPLY_TYPE_BLOCK_STATUS:
     if (cmd->type != NBD_CMD_BLOCK_STATUS ||
+        !h->meta_valid || h->meta_contexts.len == 0 ||
         length < 12 || ((length-4) & 7) != 0)
       goto resync;
     assert (CALLBACK_IS_NOT_NULL (cmd->cb.fn.extent));
-- 
2.41.0