[Libguestfs] [nbdkit PATCH 7/7] nbd: Implement structured replies

[Eric Blake]

On 4/22/19 7:50 PM, Eric Blake wrote:
> Time to enhance the nbd plugin to request structured replies from the
> server. For now, deal only with structured reads. The server can now
> return sparse reads, even though we need nbdkit version 3 before we
> can in turn return sparse reads back to the client.
> 
> In general, we have to assume the server is malicious, and so we must
> sanity check that it sends replies we expect. Thus, we have a choice
> between either implementing bookkeeping to ensure that the server
> sends exactly as many non-overlapping chunks as needed to cover the
> entire read request, or else ensuring that even when the server
> cheats, we do not leak uninitialized memory back to our client. I
> chose for simplicity, with the result that I end up calling memset()
> more frequently than necessary for a compliant server, rather than
> worrying about bookkeeping to detect a non-compliant server that is
> attempting to cause an information leak.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  plugins/nbd/nbd.c | 232 ++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 212 insertions(+), 20 deletions(-)
> 

> +  case NBD_STRUCTURED_REPLY_MAGIC:
> +    if (!h->structured) {
> +      nbdkit_error ("structured response without negotiation");
> +      return nbd_mark_dead (h);
> +    }
> +    if (read_full (h->fd, sizeof rep.simple + (char *) &rep,
> +                   sizeof rep - sizeof rep.simple))
> +      return nbd_mark_dead (h);
> +    rep.structured.flags = be16toh (rep.structured.flags);
> +    rep.structured.type = be16toh (rep.structured.type);
> +    rep.structured.length = be32toh (rep.structured.length);
> +    nbdkit_debug ("received structured reply %s for cookie %#" PRIx64
> +                  ", payload length %" PRId32,
> +                  name_of_nbd_reply_type(rep.structured.type),
> +                  rep.structured.handle, rep.structured.length);
> +    if (rep.structured.length > 64 * 1024 * 1024) {
> +      nbdkit_error ("structured reply length is suspiciously large: %" PRId32,
> +                    rep.structured.length);
> +      return nbd_mark_dead (h);
> +    }
> +    if (rep.structured.length) {
> +      /* Special case for OFFSET_DATA in order to read tail of chunk
> +         directly into final buffer later on */
> +      len = (rep.structured.type == NBD_REPLY_TYPE_OFFSET_DATA &&
> +             rep.structured.length > sizeof offset) ? sizeof offset :
> +        rep.structured.length;
> +      buf = malloc (len);
...

> +    case NBD_REPLY_TYPE_OFFSET_DATA:
> +      if (rep.structured.length <= sizeof offset) {
> +        nbdkit_error ("structured reply OFFSET_DATA too small");
> +        free (buf);
> +        return nbd_mark_dead (h);
> +      }
> +      memcpy (&offset, buf, sizeof offset);
> +      offset = be64toh (offset);
> +      len = rep.structured.length - sizeof offset;
> +      break;

leaks buf

> +    case NBD_REPLY_TYPE_OFFSET_HOLE:
> +      if (rep.structured.length != sizeof offset + sizeof len) {
> +        nbdkit_error ("structured reply OFFSET_HOLE size incorrect");
> +        free (buf);
> +        return nbd_mark_dead (h);
> +      }
> +      memcpy (&offset, buf, sizeof offset);
> +      offset = be64toh (offset);
> +      memcpy (&len, buf, sizeof len);
> +      len = be32toh (len);
> +      if (!len) {
> +        nbdkit_error ("structured reply OFFSET_HOLE length incorrect");
> +        free (buf);
> +        return nbd_mark_dead (h);
> +      }
> +      zero = true;
> +      break;

likewise

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190422/24c12c08/attachment.sig>