Re: [Libguestfs] [nbdkit RFC PATCH 4/4] exportname: New filter

Add a new filter to make it easier to add exports to a plugin that does advertise them, to avoid advertising where a plugin's list might

be an information leak, and to alter which export name is used in place of "". I would love to be able to have a strict mode enforcing that .open is called only with an export name that the plugin was willing to advertise, but for that, we'll need a way to call the plugin's .list_exports from within the context of .open (since we can't guarantee the guest will call NBD_OPT_LIST). So for now, strict mode requires redundant exportname= command line parameters. It may also be worth adding a way to pass exportname-file=/path/to/file, which then gets parsed the same was as the sh plugin (right now with NAMES, INTERLEAVED, or NAMES+DESCRIPTIONS), to avoid having to do so many export names directly on the command line. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- Compiles, but untested; .open is incomplete, and I need to add tests. At this point, reviewing the documentation to see if the ideas make sense is the best help I can get. .../exportname/nbdkit-exportname-filter.pod | 116 +++++++++++ filters/ext2/nbdkit-ext2-filter.pod | 5 + plugins/file/nbdkit-file-plugin.pod | 6 +- configure.ac | 2 + filters/exportname/Makefile.am | 67 +++++++ filters/exportname/exportname.c | 180 ++++++++++++++++++ TODO | 9 + 7 files changed, 383 insertions(+), 2 deletions(-) create mode 100644 filters/exportname/nbdkit-exportname-filter.pod create mode 100644 filters/exportname/Makefile.am create mode 100644 filters/exportname/exportname.c diff --git a/filters/exportname/nbdkit-exportname-filter.pod b/filters/exportname/nbdkit-exportname-filter.pod new file mode 100644 index 00000000..6b79624e --- /dev/null +++ b/filters/exportname/nbdkit-exportname-filter.pod @@ -0,0 +1,116 @@ +=head1 NAME + +nbdkit-exportname-filter - adjust export names between client and plugin + +=head1 SYNOPSIS + + nbdkit --filter=exportname plugin [default-export=NAME] + [exportname-list=false] [exportname-strict=true] [exportname=NAME]... + +=head1 DESCRIPTION + +Some plugins (such as C<nbdkit-file-plugin(1)> and filters (such as +C<nbdkit-ext2-filter(1)> are able to serve different content based on +the export name requested by the client. The NBD protocol allows a +server to advertise the set of export names it is serving. However, +the list advertised (or absent) from the plugin may not always match +what you want an actual client to see. This filter can be used to +alter the advertised list, as well as configuring which export should +be treated as the default when the client requests the empty string +(C<"">) as an export name. + +=head1 PARAMETERS + +=over 4 + +=item B<default-export=>NAME + +When the client requests the default export name (C<"">), request the +export C<NAME> from the underlying plugin instead of relying on the +plugin's choice of default export. Setting NAME to the empty string +has the same effect as omitting this parameter. + +=item B<exportname-list=false> + +This parameter defaults to true to advertise the modified export list, +although in some cases this can be viewed as an information leak. +Setting this parameter to false tells nbdkit to refuse to answer +C<NBD_OPT_LIST> queries, so that exports are no longer advertised. +This does not prevent a client from connecting to an export name that +it learns through other means. + +=item B<exportname-strict=true> + +Normally, a client can pass whatever export name it wants, regardless +of whether that name is advertised. But setting this parameter to +true will cause the connection to fail if a client requests an export +name that was not included via an B<exportname> parameter. (At this +time, it is not possible to restrict a client to exports advertised by +the plugin without repeating that list via B<exportname>; this +technical limitation may be lifted in the future.) + +=item B<exportname=>NAME + +This parameter adds C<NAME> to the list of advertised exports; it may +be set multiple times. + +=back + +=head1 EXAMPLES + +Suppose that the directory /path/to/dir contains permanent files named +file1, file2, and file3, as well sometimes containing a transient +file4. The following commands show various ways to alter the use of +export names while serving that directory: + +Explicitly set file2 as the default, rather than whatever file +L<readdir(3)> lists first: + + nbdkit --filter=exportname file directory=/path/to/dir default-export=file2 + +Do not advertise any exports; a client must know in advance what +export names to try: + + nbdkit --filter=exportname file directory=/path/to/dir exportname-list=false + +Allow clients to connect to file1 and file3, but not file2: + + nbdkit --filter=exportname file directory=/path/to/dir \ + exportname-strict=true exportname=file1 exportname=file3 + +=head1 FILES + +=over 4 + +=item F<$filterdir/nbdkit-exportname-filter.so> + +The filter. + +Use C<nbdkit --dump-config> to find the location of C<$filterdir>. + +=back + +=head1 VERSION + +C<nbdkit-exportname-filter> first appeared in nbdkit 1.22. + +=head1 SEE ALSO + +L<nbdkit(1)>, +L<nbdkit-filter(3)>, +L<nbdkit-ext2-filter(1)>, +L<nbdkit-extentlist-filter(1)>, +L<nbdkit-fua-filter(1)>, +L<nbdkit-nocache-filter(1)>, +L<nbdkit-noparallel-filter(1)>, +L<nbdkit-nozero-filter(1)>, +L<nbdkit-file-plugin(1)>, +L<nbdkit-info-plugin(1)>. + +=head1 AUTHORS + +Eric Blake + +=head1 COPYRIGHT + +Copyright (C) 2020 Red Hat Inc. diff --git a/filters/ext2/nbdkit-ext2-filter.pod b/filters/ext2/nbdkit-ext2-filter.pod index 78d27f8f..1aef9c2e 100644 --- a/filters/ext2/nbdkit-ext2-filter.pod +++ b/filters/ext2/nbdkit-ext2-filter.pod @@ -65,6 +65,10 @@ exportname passed by the client. Note that this mode allows the client to deduce which files exist within the disk image, which may be a security risk in some situations. +At present, when using this mode, the server does not advertise any +particular exports; however, you may use +L<nbdkit-exportname-filter(1)> to perform that task. + =back =head1 FILES @@ -89,6 +93,7 @@ and removed in nbdkit 1.22. L<nbdkit(1)>, L<nbdkit-plugin(3)>, +L<nbdkit-exportname-filter(1)>, L<nbdkit-partition-filter(1)>, L<nbdkit-guestfs-plugin(1)>, L<http://e2fsprogs.sourceforge.net/>;, diff --git a/plugins/file/nbdkit-file-plugin.pod b/plugins/file/nbdkit-file-plugin.pod index f9ae6e97..998da9bc 100644 --- a/plugins/file/nbdkit-file-plugin.pod +++ b/plugins/file/nbdkit-file-plugin.pod @@ -42,8 +42,9 @@ directory named C<DIRNAME>, including those found by following symbolic links. Other special files in the directory (such as subdirectories, fifos, or Unix sockets) are ignored. When this mode is used, the file to be served is chosen by the export name passed by -the client; a client that requests the default export (C<"">) will be -served whichever file appears first in the L<readdir(3)> listing. For +the client. A client that requests the default export (C<"">) will be +served whichever file appears first in the L<readdir(3)> listing; to +change this behavior, you can use L<nbdkit-exportname-filter(1)>. For security, when using directory mode, this plugin will not accept export names containing slash (C</>). @@ -138,6 +139,7 @@ L<nbdkit-plugin(3)>, L<nbdkit-split-plugin(1)>, L<nbdkit-partitioning-plugin(1)>, L<nbdkit-tmpdisk-plugin(1)>, +L<nbdkit-exportname-filter(1)>, L<nbdkit-fua-filter(1)>, L<nbdkit-noextents-filter(1)>. diff --git a/configure.ac b/configure.ac index 8074a470..c4321489 100644 --- a/configure.ac +++ b/configure.ac @@ -103,6 +103,7 @@ filters="\ delay \ error \ exitlast \ + exportname \ ext2 \ extentlist \ fua \ @@ -1145,6 +1146,7 @@ AC_CONFIG_FILES([Makefile filters/delay/Makefile filters/error/Makefile filters/exitlast/Makefile + filters/exportname/Makefile filters/ext2/Makefile filters/extentlist/Makefile filters/fua/Makefile diff --git a/filters/exportname/Makefile.am b/filters/exportname/Makefile.am new file mode 100644 index 00000000..1bf54518 --- /dev/null +++ b/filters/exportname/Makefile.am @@ -0,0 +1,67 @@ +# nbdkit +# Copyright (C) 2018-2020 Red Hat Inc. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# * Neither the name of Red Hat nor the names of its contributors may be +# used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +include $(top_srcdir)/common-rules.mk + +EXTRA_DIST = nbdkit-exportname-filter.pod + +filter_LTLIBRARIES = nbdkit-exportname-filter.la + +nbdkit_exportname_filter_la_SOURCES = \ + exportname.c \ + $(top_srcdir)/include/nbdkit-filter.h \ + $(NULL) + +nbdkit_exportname_filter_la_CPPFLAGS = \ + -I$(top_srcdir)/include \ + -I$(top_srcdir)/common/include \ + -I$(top_srcdir)/common/utils \ + $(NULL) +nbdkit_exportname_filter_la_CFLAGS = $(WARNINGS_CFLAGS) +nbdkit_exportname_filter_la_LDFLAGS = \ + -module -avoid-version -shared $(SHARED_LDFLAGS) \ + -Wl,--version-script=$(top_srcdir)/filters/filters.syms \ + $(NULL) +nbdkit_exportname_filter_la_LIBADD = \ + $(top_builddir)/common/utils/libutils.la \ + $(NULL) + +if HAVE_POD + +man_MANS = nbdkit-exportname-filter.1 +CLEANFILES += $(man_MANS) + +nbdkit-exportname-filter.1: nbdkit-exportname-filter.pod + $(PODWRAPPER) --section=1 --man $@ \ + --html $(top_builddir)/html/$@.html \ + $< + +endif HAVE_POD diff --git a/filters/exportname/exportname.c b/filters/exportname/exportname.c new file mode 100644 index 00000000..c9404278 --- /dev/null +++ b/filters/exportname/exportname.c @@ -0,0 +1,180 @@ +/* nbdkit + * Copyright (C) 2020 Red Hat Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * * Neither the name of Red Hat nor the names of its contributors may be + * used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include <config.h> + +#include <stdbool.h> +#include <string.h> + +#include <nbdkit-filter.h> + +#include "cleanup.h" + +static const char *default_export; +static bool list = true; +static bool strict; +struct nbdkit_exports *exports; + +static void +exportname_load (void) +{ + exports = nbdkit_exports_new (false); + if (!exports) { + nbdkit_error ("malloc: %m"); + exit (EXIT_FAILURE); + } +} + +static void +exportname_unload (void) +{ + nbdkit_exports_free (exports); +} + +/* Called for each key=value passed on the command line. */ +static int +exportname_config (nbdkit_next_config *next, void *nxdata, + const char *key, const char *value) +{ + int r; + + if (strcmp (key, "default-export") == 0 || + strcmp (key, "default_export") == 0) { + default_export = value; + return 0; + } + if (strcmp (key, "exportname-list") == 0 || + strcmp (key, "exportname_list") == 0) { + r = nbdkit_parse_bool (value); + if (r == -1) + return -1; + list = r; + return 0; + } + if (strcmp (key, "exportname-strict") == 0 || + strcmp (key, "exportname_strict") == 0) { + r = nbdkit_parse_bool (value); + if (r == -1) + return -1; + strict = r; + return 0; + } + if (strcmp (key, "exportname") == 0) + return nbdkit_add_export (exports, value, NULL); + return next (nxdata, key, value); +} + +#define exportname_config_help \ + "default-export=<NAME> Canonical name for the \"\" default export.\n" \ + "exportname-list=<BOOL> Whether to advertise exports (default true).\n" \ + "exportname-strict=<BOOL> Limit clients to approved exports (default false).\n" \ + "exportname=<NAME> Add an approved export name, may be repeated.\n" \ + +static int +exportname_list_exports (nbdkit_next_list_exports *next, void *nxdata, + int readonly, int default_only, + struct nbdkit_exports *exps) +{ + size_t i; + struct nbdkit_exports *source; + CLEANUP_EXPORTS_FREE struct nbdkit_exports *exps2 = NULL; + + /* default_only is set when nbdkit is trying to resolve "" during + * .open, and not when the client requested NBD_OPT_LIST, so it is + * okay to return something even when listing is suppressed. + */ + if (default_only) { + if (default_export) + return nbdkit_add_export (exps, default_export, NULL); + return next (nxdata, readonly, default_only, exps); + } + + if (!list) { + nbdkit_error ("export list restricted by policy"); + return -1; + } + + /* If we have a list, return that instead of the plugin. If not, + * return the plugin's results. However, we must list any default + * first, because nbdkit can cache that rather than calling again + * with default_only. + */ + if (default_export && nbdkit_add_export (exps, "", NULL) == -1) + return -1; + if (nbdkit_exports_count (exports)) + source = exports; + else { + source = exps2 = nbdkit_exports_new (default_only); + if (exps2 == NULL) + return -1; + if (next (nxdata, readonly, default_only, exps2) == -1) + return -1; + } + /* Copy everything, except no need to repeat the default */ + for (i = 0; i < nbdkit_exports_count (source); i++) { + struct nbdkit_export e = nbdkit_get_export (source, i); + + if (default_export && strcmp (default_export, e.name) == 0) + continue; + if (nbdkit_add_export (exps, e.name, e.description) == -1) + return -1; + } + return 0; +} + +static void * +exportname_open (nbdkit_next_open *next, void *nxdata, + int readonly, const char *exportname) +{ + if (!*exportname && default_export) + exportname = default_export; + if (strict) + /* TODO: Check that exportname is in exports... */ + ; + if (next (nxdata, readonly, exportname) == -1) + return NULL; + + return NBDKIT_HANDLE_NOT_NEEDED; +} + +static struct nbdkit_filter filter = { + .name = "exportname", + .longname = "nbdkit exportname filter", + .load = exportname_load, + .unload = exportname_unload, + .config = exportname_config, + .config_help = exportname_config_help, + .list_exports = exportname_list_exports, + .open = exportname_open, +}; + +NBDKIT_REGISTER_FILTER(filter) diff --git a/TODO b/TODO index 46f92443..c9eb0cc7 100644 --- a/TODO +++ b/TODO @@ -237,6 +237,15 @@ nbdkit-extentlist-filter: * make non-read-only access safe by updating the extent list when the filter sees writes and trims +nbdkit-exportname-fitler: + +* find a way to call the plugin's .list_exports during .open, so that + we can enforce exportname-strict=true without command line redundancy + +* add a mode for passing in a file containing exportnames in the same + manner accepted by the sh/eval plugins, rather than one name (and no + description) per config parameter + Filters for security --------------------