[Libguestfs] [nbdkit PATCH 1/2] filters: Add .export_description wrappers

[Eric Blake]

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