1:43 p.m.
On 9/1/20 9:30 AM, Richard W.M. Jones wrote:
On Thu, Aug 27, 2020 at 05:03:45PM -0500, Eric Blake wrote:
> When extracting an obvious subset of a larger container (ext2, gzip,
> partition, tar, xz), it's fairly easy to add a nice updated
> description for what the client is seeing. Not all clients request
> the description, but if you are worried about this leaking too much
> information, it can be silenced with nbdkit-exportname-filter.
No?
The trouble with this is it leaks configuration information of the
server which most users of nbdkit would not really be aware of. NBD
protocol export descriptions are a pretty obscure thing - even I would
have to look at the qemu-nbd manual to work out how to read them from
an NBD server.
Also I don't really see the benefit of this to anyone. The server
admin already knows this, and the client doesn't care unless they're
looking for a way to attack the server.
Okay, that's worth thinking about. A server is unlikely to need to
provide descriptions when it serves the same content to everyone; it is
more a useful tactic to servers that offer multiple exports, and wants
to aid the client in making that choice. For example, when using 'file
dir=/path/to/nbdkit', we could advertise:
"aclocal.m4", "-rw-rw-r--. 1 eblake eblake 54764 Sep 1 11:03
aclocal.m4"
"BENCHMARKING", "-rw-rw-r--. 1 eblake eblake 5324 Aug 19 08:46
BENCHMARKING"
...
Now, if we wrap the ext2 filter in front of that, either:
ext2 is hardcoded to extracting a specific filename from the underlying
image, but the client is choosing which image to extract it from (seeing
the original description is now a bit off, because we are now exposing
only a subset of the file),
or ext2 is honoring export names directly, and calling next_open(""), in
which case the description of the default export in the file plugin
makes even less sense.
The simplest course of action is to just have the ext2 filter always
advertise NULL as the description (any description from the plugin is
lost, because by the time we extract a file from an ext2 image, the
original image's description is no longer fully applicable to the subset
we are exposing). Or maybe we could do:
if plugin reports NULL, we also report NULL
if plugin reports string, we prefix a hint that we are extracting an
ext2 file out of the plugin's description string
> And maybe this is an excuse for adding nbdkit_[v]printf_intern()...
This could well be useful.
That part is now done.
> +/* Description. */
> +static const char *
> +ext2_export_description (struct nbdkit_next_ops *next_ops, void *nxdata,
> + void *handle)
> +{
> + struct handle *h = handle;
> + const char *fname = file ?: h->exportname;
> + const char *slash = fname[0] == '/' ? "" : "/";
> + const char *base = next_ops->export_description (nxdata);
> + CLEANUP_FREE char *desc = NULL;
> +
> + if (base)
> + asprintf (&desc, "embedded '%s%s' from within ext2 image:
%s",
> + slash, fname, base);
This is the case where a prefix might be nice...
> + else
> + asprintf (&desc, "embedded '%s%s' from within ext2 image",
slash, fname);
...where this is a case where since the original plugin had no
description, we are better off not providing one ourselves, either.
But you are also right that providing a prefix is somewhat redundant -
the person that invoked nbdkit to set up the server already knows what
is being served, and the client is less likely to care about the
description of what is being served, compared to the actual content.
So like you, I'm now wavering on whether we actually want this patch, or
one that just provides an .export_description that always returns NULL,
or something else.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org