Here's changes to the file plugin (which I'm happy with) and a new exportname filter (which is still at RFC stage; I need to finish implementing strict mode in .open, and add tests). I also discovered that we really want .list_exports and .open to know when they are used on plaintext vs. tls clients for --tls=on, and we may want to split out a new .default_export callback rather than overloading .list_exports(default_only=true). Ah well, more to play with tomorrow. Eric Blake (4): file: Forbid non-regular, non-block file names file: Add .list_exports support file: Use dirent.dt_type for speed exportname: New filter .../exportname/nbdkit-exportname-filter.pod | 116 +++++++++++ filters/ext2/nbdkit-ext2-filter.pod | 5 + plugins/file/nbdkit-file-plugin.pod | 30 ++- configure.ac | 3 + filters/exportname/Makefile.am | 67 +++++++ tests/Makefile.am | 4 +- plugins/file/file.c | 135 +++++++++++-- filters/exportname/exportname.c | 180 ++++++++++++++++++ tests/test-file-dir.sh | 143 ++++++++++++++ TODO | 9 + 10 files changed, 669 insertions(+), 23 deletions(-) create mode 100644 filters/exportname/nbdkit-exportname-filter.pod create mode 100644 filters/exportname/Makefile.am create mode 100644 filters/exportname/exportname.c create mode 100755 tests/test-file-dir.sh -- 2.28.0
Eric Blake
2020-Aug-07 02:23 UTC
[Libguestfs] [nbdkit PATCH 1/4] file: Forbid non-regular, non-block file names
Serving a directory, char device, fifo, socket, or other unusual file
type is likely to cause unusual problems down the road when trying to
use .pread; let's move the failure sooner to .open.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
plugins/file/file.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/plugins/file/file.c b/plugins/file/file.c
index dc99f992..e049864a 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -189,7 +189,16 @@ file_open (int readonly)
return NULL;
}
- h->is_block_device = S_ISBLK (statbuf.st_mode);
+ if (S_ISBLK (statbuf.st_mode))
+ h->is_block_device = true;
+ else if (S_ISREG (statbuf.st_mode))
+ h->is_block_device = false;
+ else {
+ nbdkit_error ("file is not regular or block device: %s",
filename);
+ close (h->fd);
+ free (h);
+ return NULL;
+ }
h->sector_size = 4096; /* Start with safe guess */
#ifdef BLKSSZGET
--
2.28.0
Eric Blake
2020-Aug-07 02:23 UTC
[Libguestfs] [nbdkit PATCH 2/4] file: Add .list_exports support
Add a new mode to the file plugin, using directory=DIR instead of
[file=]FILE, to allow it to serve all regular/block files in a given
directory, as well as advertising the names of those files it will be
serving.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
plugins/file/nbdkit-file-plugin.pod | 28 +++++-
tests/Makefile.am | 4 +-
plugins/file/file.c | 119 +++++++++++++++++++----
tests/test-file-dir.sh | 143 ++++++++++++++++++++++++++++
4 files changed, 271 insertions(+), 23 deletions(-)
create mode 100755 tests/test-file-dir.sh
diff --git a/plugins/file/nbdkit-file-plugin.pod
b/plugins/file/nbdkit-file-plugin.pod
index dac673ae..f9ae6e97 100644
--- a/plugins/file/nbdkit-file-plugin.pod
+++ b/plugins/file/nbdkit-file-plugin.pod
@@ -6,27 +6,47 @@ nbdkit-file-plugin - nbdkit file plugin
nbdkit file [file=]FILENAME
+ nbdkit file directory=DIRNAME
+
=head1 DESCRIPTION
C<nbdkit-file-plugin> is a file serving plugin for L<nbdkit(1)>.
It serves the named C<FILENAME> over NBD. Local block devices
-(eg. F</dev/sda>) may also be served.
+(eg. F</dev/sda>) may also be served. It may also be used to serve
+any file within a given C<DIRECTORY>, according to which export name
+the guest requests.
=head1 PARAMETERS
+One of B<file> or B<directory> must be given to determine which
mode
+the server will use.
+
=over 4
=item [B<file=>]FILENAME
Serve the file named C<FILENAME>. A local block device name can also
-be used here.
-
-This parameter is required.
+be used here. When this mode is used, the export name requested by
+the client is ignored.
C<file=> is a magic config key and may be omitted in most cases.
See L<nbdkit(1)/Magic parameters>.
+=item B<directory=>DIRNAME
+
+(nbdkt E<ge> 1.22)
+
+Serve all regular files and block devices located directly within the
+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
+security, when using directory mode, this plugin will not accept
+export names containing slash (C</>).
+
=back
=head1 NOTES
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b5ef96a7..d9074ba9 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -636,8 +636,8 @@ test_file_block_SOURCES = test-file-block.c test.h
test_file_block_CFLAGS = $(WARNINGS_CFLAGS) $(LIBGUESTFS_CFLAGS)
test_file_block_LDADD = libtest.la $(LIBGUESTFS_LIBS)
-TESTS += test-file-extents.sh
-EXTRA_DIST += test-file-extents.sh
+TESTS += test-file-extents.sh test-file-dir.sh
+EXTRA_DIST += test-file-extents.sh test-file-dir.sh
# floppy plugin test.
TESTS += test-floppy.sh
diff --git a/plugins/file/file.c b/plugins/file/file.c
index e049864a..4afcad11 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -43,6 +43,7 @@
#include <sys/stat.h>
#include <sys/ioctl.h>
#include <errno.h>
+#include <dirent.h>
#include <pthread.h>
@@ -66,9 +67,11 @@
#endif
static char *filename = NULL;
+static char *directory = NULL;
+DIR *dir = NULL;
-/* Any callbacks using lseek must be protected by this lock. */
-static pthread_mutex_t lseek_lock = PTHREAD_MUTEX_INITIALIZER;
+/* Any callbacks using readdir or lseek must be protected by this lock. */
+static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
/* to enable: -D file.zero=1 */
int file_debug_zero;
@@ -83,10 +86,14 @@ static void
file_unload (void)
{
free (filename);
+ free (directory);
+ if (dir)
+ closedir (dir);
}
/* Called for each key=value passed on the command line. This plugin
- * only accepts file=<filename>, which is required.
+ * only accepts file=<filename> and directory=<dirname>, where
exactly
+ * one is required.
*/
static int
file_config (const char *key, const char *value)
@@ -98,6 +105,12 @@ file_config (const char *key, const char *value)
if (!filename)
return -1;
}
+ else if (strcmp (key, "directory") == 0) {
+ free (directory);
+ directory = nbdkit_realpath (value);
+ if (!directory)
+ return -1;
+ }
else if (strcmp (key, "rdelay") == 0 ||
strcmp (key, "wdelay") == 0) {
nbdkit_error ("add --filter=delay on the command line");
@@ -111,13 +124,19 @@ file_config (const char *key, const char *value)
return 0;
}
-/* Check the user did pass a file=<FILENAME> parameter. */
+/* Check the user did pass exactly one parameter. */
static int
file_config_complete (void)
{
- if (filename == NULL) {
- nbdkit_error ("you must supply the file=<FILENAME> parameter
"
- "after the plugin name on the command line");
+ if (!filename == !directory) {
+ nbdkit_error ("you must supply exactly one file=<FILENAME> or
"
+ "directory=<DIRNAME> parameter after the plugin
name "
+ "on the command line");
+ return -1;
+ }
+
+ if (directory && (dir = opendir (directory)) == NULL) {
+ nbdkit_error ("opendir: %m");
return -1;
}
@@ -125,7 +144,8 @@ file_config_complete (void)
}
#define file_config_help \
- "file=<FILENAME> (required) The filename to serve." \
+ "file=<FILENAME> The filename to serve." \
+ "directory=<DIRNAME> A directory containing files to
serve." \
/* Print some extra information about how the plugin was compiled. */
static void
@@ -145,8 +165,47 @@ file_dump_plugin (void)
#endif
}
+static int file_list_exports (int readonly, int default_only,
+ struct nbdkit_exports *exports)
+{
+ struct dirent *entry;
+ struct stat sb;
+ int fd;
+
+ if (!directory)
+ return nbdkit_add_export (exports, "", NULL);
+
+ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
+ rewinddir (dir);
+ fd = dirfd (dir);
+ if (fd == -1) {
+ nbdkit_error ("dirfd: %m");
+ return -1;
+ }
+ errno = 0;
+ while ((entry = readdir (dir)) != NULL) {
+ /* TODO: Optimize with d_type and/or statx when present? */
+ if (fstatat (fd, entry->d_name, &sb, 0) == 0 &&
+ (S_ISREG (sb.st_mode) || S_ISBLK (sb.st_mode))) {
+ if (nbdkit_add_export (exports, entry->d_name, NULL) == -1) {
+ close (fd);
+ return -1;
+ }
+ }
+ errno = 0;
+ }
+ if (errno) {
+ nbdkit_error ("readdir: %m");
+ close (fd);
+ return -1;
+ }
+ close (fd);
+ return 0;
+}
+
/* The per-connection handle. */
struct handle {
+ char *file;
int fd;
bool is_block_device;
int sector_size;
@@ -170,21 +229,44 @@ file_open (int readonly)
return NULL;
}
+ if (directory) {
+ const char *exportname = nbdkit_export_name ();
+
+ if (strchr (exportname, '/')) {
+ nbdkit_error ("exportname cannot contain /");
+ errno = EINVAL;
+ free (h);
+ return NULL;
+ }
+ if (asprintf (&h->file, "%s/%s", directory, exportname) ==
-1) {
+ nbdkit_error ("asprintf: %m");
+ free (h);
+ return NULL;
+ }
+ }
+ else
+ h->file = strdup (filename);
+ if (h->file == NULL) {
+ nbdkit_error ("malloc: %m");
+ free (h);
+ return NULL;
+ }
+
flags = O_CLOEXEC|O_NOCTTY;
if (readonly)
flags |= O_RDONLY;
else
flags |= O_RDWR;
- h->fd = open (filename, flags);
+ h->fd = open (h->file, flags);
if (h->fd == -1) {
- nbdkit_error ("open: %s: %m", filename);
+ nbdkit_error ("open: %s: %m", h->file);
free (h);
return NULL;
}
if (fstat (h->fd, &statbuf) == -1) {
- nbdkit_error ("fstat: %s: %m", filename);
+ nbdkit_error ("fstat: %s: %m", h->file);
free (h);
return NULL;
}
@@ -194,7 +276,8 @@ file_open (int readonly)
else if (S_ISREG (statbuf.st_mode))
h->is_block_device = false;
else {
- nbdkit_error ("file is not regular or block device: %s",
filename);
+ nbdkit_error ("file is not regular or block device: %s",
h->file);
+ free (h->file);
close (h->fd);
free (h);
return NULL;
@@ -204,7 +287,7 @@ file_open (int readonly)
#ifdef BLKSSZGET
if (h->is_block_device) {
if (ioctl (h->fd, BLKSSZGET, &h->sector_size))
- nbdkit_debug ("cannot get sector size: %s: %m", filename);
+ nbdkit_debug ("cannot get sector size: %s: %m", h->file);
}
#endif
@@ -232,6 +315,7 @@ file_close (void *handle)
{
struct handle *h = handle;
+ free (h->file);
close (h->fd);
free (h);
}
@@ -239,7 +323,7 @@ file_close (void *handle)
#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL
/* For block devices, stat->st_size is not the true size. The caller
- * grabs the lseek_lock.
+ * grabs the lock.
*/
static int64_t
block_device_size (int fd)
@@ -262,7 +346,7 @@ file_get_size (void *handle)
struct handle *h = handle;
if (h->is_block_device) {
- ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lseek_lock);
+ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
return block_device_size (h->fd);
} else {
/* Regular file. */
@@ -554,7 +638,7 @@ file_can_extents (void *handle)
/* A simple test to see whether SEEK_HOLE etc is likely to work on
* the current filesystem.
*/
- ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lseek_lock);
+ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
r = lseek (h->fd, 0, SEEK_HOLE);
if (r == -1) {
nbdkit_debug ("extents disabled: lseek: SEEK_HOLE: %m");
@@ -628,7 +712,7 @@ static int
file_extents (void *handle, uint32_t count, uint64_t offset,
uint32_t flags, struct nbdkit_extents *extents)
{
- ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lseek_lock);
+ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
return do_extents (handle, count, offset, flags, extents);
}
#endif /* SEEK_HOLE */
@@ -662,6 +746,7 @@ static struct nbdkit_plugin plugin = {
.config_help = file_config_help,
.magic_config_key = "file",
.dump_plugin = file_dump_plugin,
+ .list_exports = file_list_exports,
.open = file_open,
.close = file_close,
.get_size = file_get_size,
diff --git a/tests/test-file-dir.sh b/tests/test-file-dir.sh
new file mode 100755
index 00000000..efe3b6cd
--- /dev/null
+++ b/tests/test-file-dir.sh
@@ -0,0 +1,143 @@
+#!/usr/bin/env bash
+# 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.
+
+# Test the use of the directory mode of the file plugin.
+
+source ./functions.sh
+set -e
+set -x
+
+# Hack: This rejects libnbd 1.3.9, but we really need libnbd >= 1.3.11,
+# which does not have its own decent witness...
+requires nbdsh -c 'print (h.get_list_export_description)'
+
+requires nbdinfo --version
+requires jq --version
+
+files="file-dir file-dir.out file-dir.witness"
+rm -rf $files
+cleanup_fn rm -rf $files
+fail=0
+
+# do_nbdkit_list [--no-sort] EXPOUT
+# Check that the advertised list of exports matches EXPOUT
+do_nbdkit_list ()
+{
+ sort=' | sort'
+ if [ "$1" = --no-sort ]; then
+ sort+ shift
+ fi
+ nbdkit -U - -v file directory=file-dir \
+ --run 'nbdinfo --list --json "$uri"' >file-dir.out
+ cat file-dir.out
+ diff -u <(jq -c
'[.exports[]."export-name"]'"$sort" file-dir.out) \
+ <(printf %s\\n "$1") || fail=1
+}
+
+# do_nbdkit_fail NAME
+# Check that attempting to connect to export NAME fails
+do_nbdkit_fail ()
+{
+ # The --run script occurs only if nbdkit gets past .config_complete;
+ # testing for witness proves that our failure was during .open and
+ # not at some earlier point
+ rm -f file-dir.witness
+ nbdkit -U - -v -e "$1" file directory=file-dir \
+ --run 'touch file-dir.witness; nbdsh -u "$uri" -c
"quit()"' && fail=1
+ test -f file-dir.witness || fail=1
+}
+
+# do_nbdkit_pass NAME DATA
+# Check that export NAME serves DATA as its first byte
+do_nbdkit_pass ()
+{
+ out=$(nbdkit -U - -v -e "$1" file directory=file-dir \
+ --run 'nbdsh -u "$uri" -c "print (h.pread (1, 0).decode
(\"utf-8\"))"')
+ test "$out" = "$2" || fail=1
+}
+
+# Not possible to serve a missing directory
+nbdkit -vf file directory=nosuchdir && fail=1
+
+# Serving an empty directory
+mkdir file-dir
+do_nbdkit_list '[]'
+do_nbdkit_fail ''
+do_nbdkit_fail 'a'
+do_nbdkit_fail '..'
+do_nbdkit_fail '/'
+
+# Serving a directory with one file
+echo 1 > file-dir/a
+do_nbdkit_list '["a"]'
+do_nbdkit_pass '' 1
+do_nbdkit_pass a 1
+do_nbdkit_fail b
+
+# Serving a directory with multiple files.
+# Use 'find' to match readdir's raw order (a is not always first!)
+echo 2 > file-dir/b
+raw=$(find file-dir -type f | xargs echo)
+exp=$(echo $raw | sed 's,file-dir/\(.\),"\1",g; s/ /,/')
+do_nbdkit_list --no-sort "[$exp]"
+do_nbdkit_list '["a","b"]'
+case $raw in
+ file-dir/a*) byte=1 ;;
+ file-dir/b*) byte=2 ;;
+ *) fail=1 ;;
+esac
+do_nbdkit_pass '' $byte
+do_nbdkit_pass 'a' 1
+do_nbdkit_pass 'b' 2
+do_nbdkit_fail 'c'
+
+# Serving a directory with non-regular files
+ln -s b file-dir/c
+mkfifo file-dir/d
+mkdir file-dir/e
+ln -s /dev/null file-dir/f
+ln -s . file-dir/g
+ln -s dangling file-dir/h
+do_nbdkit_list '["a","b","c"]'
+do_nbdkit_pass 'a' 1
+do_nbdkit_pass 'b' 2
+do_nbdkit_pass 'c' 2
+do_nbdkit_fail 'd'
+do_nbdkit_fail 'e'
+do_nbdkit_fail 'f'
+do_nbdkit_fail 'g'
+do_nbdkit_fail 'h'
+do_nbdkit_fail './a'
+do_nbdkit_fail '../file-dir/a'
+
+exit $fail
--
2.28.0
Eric Blake
2020-Aug-07 02:23 UTC
[Libguestfs] [nbdkit PATCH 3/4] file: Use dirent.dt_type for speed
No need to fstatat() when .d_type is reliable. However, as it is not
portable, I've done this as a separate patch.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
configure.ac | 1 +
plugins/file/file.c | 21 ++++++++++++++-------
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/configure.ac b/configure.ac
index 28f4a3cd..8074a470 100644
--- a/configure.ac
+++ b/configure.ac
@@ -323,6 +323,7 @@ AC_CHECK_FUNCS([\
pipe2 \
ppoll \
posix_fadvise])
+AC_CHECK_MEMBERS([struct dirent.d_type], [], [], [[#include <dirent.h>]])
dnl Check whether printf("%m") works
AC_CACHE_CHECK([whether the printf family supports %m],
diff --git a/plugins/file/file.c b/plugins/file/file.c
index 4afcad11..445d5686 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -184,13 +184,20 @@ static int file_list_exports (int readonly, int
default_only,
}
errno = 0;
while ((entry = readdir (dir)) != NULL) {
- /* TODO: Optimize with d_type and/or statx when present? */
- if (fstatat (fd, entry->d_name, &sb, 0) == 0 &&
- (S_ISREG (sb.st_mode) || S_ISBLK (sb.st_mode))) {
- if (nbdkit_add_export (exports, entry->d_name, NULL) == -1) {
- close (fd);
- return -1;
- }
+ int r = -1;
+#if HAVE_STRUCT_DIRENT_D_TYPE
+ if (entry->d_type == DT_BLK || entry->d_type == DT_REG)
+ r = 1;
+ else if (entry->d_type != DT_LNK && entry->d_type !=
DT_UNKNOWN)
+ r = 0;
+#endif
+ /* TODO: when chasing symlinks, is statx any nicer than fstatat? */
+ if (r < 0 && fstatat (fd, entry->d_name, &sb, 0) == 0
&&
+ (S_ISREG (sb.st_mode) || S_ISBLK (sb.st_mode)))
+ r = 1;
+ if (r == 1 && nbdkit_add_export (exports, entry->d_name, NULL)
== -1) {
+ close (fd);
+ return -1;
}
errno = 0;
}
--
2.28.0
Eric Blake
2020-Aug-07 02:23 UTC
[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@redhat.com>
---
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
--------------------
--
2.28.0
Richard W.M. Jones
2020-Aug-07 11:43 UTC
Re: [Libguestfs] [nbdkit PATCH 1/4] file: Forbid non-regular, non-block file names
On Thu, Aug 06, 2020 at 09:23:45PM -0500, Eric Blake wrote:> Serving a directory, char device, fifo, socket, or other unusual file > type is likely to cause unusual problems down the road when trying to > use .pread; let's move the failure sooner to .open. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > plugins/file/file.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/plugins/file/file.c b/plugins/file/file.c > index dc99f992..e049864a 100644 > --- a/plugins/file/file.c > +++ b/plugins/file/file.c > @@ -189,7 +189,16 @@ file_open (int readonly) > return NULL; > } > > - h->is_block_device = S_ISBLK (statbuf.st_mode); > + if (S_ISBLK (statbuf.st_mode)) > + h->is_block_device = true; > + else if (S_ISREG (statbuf.st_mode)) > + h->is_block_device = false; > + else { > + nbdkit_error ("file is not regular or block device: %s", filename); > + close (h->fd); > + free (h); > + return NULL; > + } > h->sector_size = 4096; /* Start with safe guess */ > > #ifdef BLKSSZGETACK, we should take this patch soon. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Richard W.M. Jones
2020-Aug-07 11:57 UTC
Re: [Libguestfs] [nbdkit PATCH 2/4] file: Add .list_exports support
On Thu, Aug 06, 2020 at 09:23:46PM -0500, Eric Blake wrote:> + if (!filename == !directory) {A bit tricksy. In plugins/nbd/nbd.c I used: int c = !!sockname + !!hostname + !!uri + (command.size > 0) + (socket_fd >= 0) + !!raw_cid; /* Check the user passed exactly one connection parameter. */ if (c > 1) { nbdkit_error ("cannot mix Unix ‘socket’, TCP ‘hostname’/‘port’, ‘vsock’, " "‘command’, ‘socket-fd’ and ‘uri’ parameters"); return -1; } if (c == 0) { nbdkit_error ("exactly one of ‘socket’, ‘hostname’, ‘vsock’, ‘command’, " "‘socket-fd’ and ‘uri’ parameters must be specified"); return -1; } which might be longer but a bit clearer?> + if (directory && (dir = opendir (directory)) == NULL) { > + nbdkit_error ("opendir: %m"); > return -1; > }What happens if the contents of the directory change while the file plugin is running? I guess we're using rewinddir so that's fine. But maybe we want to defer opening this until we actually do list_exports? I can see arguments both ways TBH, one obvious one being that it'd be nice to fail early if directory doesn't exist at all. However if we defer opening the directory til it is needed then we would have to use nbdkit_realpath on the directory name, and that should check that the directory (or something) exists.> + if (directory) { > + const char *exportname = nbdkit_export_name (); > + > + if (strchr (exportname, '/')) { > + nbdkit_error ("exportname cannot contain /"); > + errno = EINVAL; > + free (h); > + return NULL; > + } > + if (asprintf (&h->file, "%s/%s", directory, exportname) == -1) { > + nbdkit_error ("asprintf: %m"); > + free (h); > + return NULL; > + } > + } > + else > + h->file = strdup (filename); > + if (h->file == NULL) { > + nbdkit_error ("malloc: %m"); > + free (h); > + return NULL; > + } > + > flags = O_CLOEXEC|O_NOCTTY; > if (readonly) > flags |= O_RDONLY; > else > flags |= O_RDWR; > > - h->fd = open (filename, flags); > + h->fd = open (h->file, flags);Maybe openat is safer than trying to concatenate filenames? I would say that Windows is part of the argument here, but we don't support it, and Windows doesn't have openat. Also there's an argument that you want the full filename for error messages, but do we in fact want to echo the exportname (client controlled) to log files?> +# Hack: This rejects libnbd 1.3.9, but we really need libnbd >= 1.3.11, > +# which does not have its own decent witness... > +requires nbdsh -c 'print (h.get_list_export_description)'Some variation on: $ nbdsh -c 'print(h.get_version())' 1.3.11 $ nbdsh -c 'from packaging import version ; print(version.parse(h.get_version()) >= version.parse("1.3.11"))' True However 1.3.10 was only present for a short time and I already got rid of it in Rawhide because it affected other nbdkit tests, so probably what you have already is fine in practice. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Richard W.M. Jones
2020-Aug-07 11:59 UTC
Re: [Libguestfs] [nbdkit RFC PATCH 4/4] exportname: New filter
On Thu, Aug 06, 2020 at 09:23:48PM -0500, Eric Blake wrote:> 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 mightdoes *not* advertise them(?)> 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@redhat.com> > --- > > 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 > --------------------All looks pretty sensible to me. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Eric Blake
2020-Aug-07 13:08 UTC
Re: [Libguestfs] [nbdkit PATCH 2/4] file: Add .list_exports support
On 8/6/20 9:23 PM, Eric Blake wrote:> Add a new mode to the file plugin, using directory=DIR instead of > [file=]FILE, to allow it to serve all regular/block files in a given > directory, as well as advertising the names of those files it will be > serving. > > Signed-off-by: Eric Blake <eblake@redhat.com> > ---In addition to your review, I'm also addressing:> > +static int file_list_exports (int readonly, int default_only, > + struct nbdkit_exports *exports)Formatting - this file prefers: static int file_list_exports (...)> +{ > + struct dirent *entry; > + struct stat sb; > + int fd; > + > + if (!directory) > + return nbdkit_add_export (exports, "", NULL); > + > + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); > + rewinddir (dir); > + fd = dirfd (dir); > + if (fd == -1) { > + nbdkit_error ("dirfd: %m"); > + return -1; > + } > + errno = 0; > + while ((entry = readdir (dir)) != NULL) { > + /* TODO: Optimize with d_type and/or statx when present? */ > + if (fstatat (fd, entry->d_name, &sb, 0) == 0 && > + (S_ISREG (sb.st_mode) || S_ISBLK (sb.st_mode))) { > + if (nbdkit_add_export (exports, entry->d_name, NULL) == -1) { > + close (fd);Bug - the fd obtained from dirfd() is closed by closedir(); closing it ourselves is a double-close which may affect further operation on the DIR or interfere with other code. We weren't tripping on the bug because most clients don't call NBD_OPT_LIST more than once, but such a client would make it more obvious. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org