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