Eric Blake
2020-Aug-07 22:00 UTC
[Libguestfs] [nbdkit PATCH 0/3] Content differentiation during --tls=on
Patch 3 still needs tests added, but it is at least working from my simple command line tests. Eric Blake (3): server: Implement nbdkit_is_tls for use during .open server: Expose final thread_model to filter's .get_ready tlsdummy: New filter docs/nbdkit-filter.pod | 21 +- docs/nbdkit-plugin.pod | 34 ++- docs/nbdkit-tls.pod | 8 +- filters/log/nbdkit-log-filter.pod | 2 +- filters/tlsdummy/nbdkit-tlsdummy-filter.pod | 72 ++++++ plugins/sh/nbdkit-sh-plugin.pod | 5 +- include/nbdkit-filter.h | 5 +- include/nbdkit-plugin.h | 1 + configure.ac | 2 + filters/tlsdummy/Makefile.am | 63 ++++++ server/internal.h | 3 +- server/backend.c | 6 +- server/filters.c | 10 +- server/nbdkit.syms | 1 + server/plugins.c | 14 +- server/public.c | 16 ++ plugins/sh/methods.c | 1 + filters/cow/cow.c | 2 +- filters/exitlast/exitlast.c | 2 +- filters/ext2/ext2.c | 2 +- filters/extentlist/extentlist.c | 3 +- filters/gzip/gzip.c | 2 +- filters/limit/limit.c | 2 +- filters/log/log.c | 18 +- filters/partition/partition.c | 2 +- filters/rate/rate.c | 4 +- filters/retry/retry.c | 2 +- filters/stats/stats.c | 2 +- filters/tar/tar.c | 2 +- filters/tlsdummy/tlsdummy.c | 235 ++++++++++++++++++++ filters/truncate/truncate.c | 2 +- filters/xz/xz.c | 2 +- tests/test-layers-filter.c | 4 +- TODO | 13 +- 34 files changed, 504 insertions(+), 59 deletions(-) create mode 100644 filters/tlsdummy/nbdkit-tlsdummy-filter.pod create mode 100644 filters/tlsdummy/Makefile.am create mode 100644 filters/tlsdummy/tlsdummy.c -- 2.28.0
Eric Blake
2020-Aug-07 22:00 UTC
[Libguestfs] [nbdkit PATCH 1/3] server: Implement nbdkit_is_tls for use during .open
Now that we can differentiate content based on export name, we also need to be able to differentiate content for a --tls=on server, since TLS is optional according to whether the client has authenticated. For internal code and filters, this means adding a new parameter; the sh plugin can do likewise. For plugins, we can't add a parameter until the V3 protocol, so in the meantime, we add nbdkit_is_tls(), even though it will be deprecated alongside nbdkit_export_name() when we do get to V3. Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/nbdkit-filter.pod | 12 ++++++++---- docs/nbdkit-plugin.pod | 32 +++++++++++++++++++++++++++---- docs/nbdkit-tls.pod | 5 ++++- filters/log/nbdkit-log-filter.pod | 2 +- plugins/sh/nbdkit-sh-plugin.pod | 5 +++-- include/nbdkit-filter.h | 2 +- include/nbdkit-plugin.h | 1 + server/internal.h | 3 ++- server/backend.c | 6 +++--- server/filters.c | 6 ++++-- server/nbdkit.syms | 1 + server/plugins.c | 14 +++++++++----- server/public.c | 16 ++++++++++++++++ plugins/sh/methods.c | 1 + filters/cow/cow.c | 2 +- filters/exitlast/exitlast.c | 2 +- filters/ext2/ext2.c | 2 +- filters/gzip/gzip.c | 2 +- filters/limit/limit.c | 2 +- filters/log/log.c | 16 +++++++++------- filters/partition/partition.c | 2 +- filters/rate/rate.c | 2 +- filters/retry/retry.c | 2 +- filters/tar/tar.c | 2 +- filters/truncate/truncate.c | 2 +- filters/xz/xz.c | 2 +- tests/test-layers-filter.c | 2 +- TODO | 13 ++++++------- 28 files changed, 109 insertions(+), 50 deletions(-) diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod index 12343dbf..b6ed5504 100644 --- a/docs/nbdkit-filter.pod +++ b/docs/nbdkit-filter.pod @@ -403,7 +403,7 @@ Returns a copy of the C<i>'th export. =head2 C<.open> void * (*open) (nbdkit_next_open *next, void *nxdata, - int readonly, const char *exportname); + int readonly, const char *exportname, int is_tls); This is called when a new client connection is opened and can be used to allocate any per-connection data structures needed by the filter. @@ -420,8 +420,9 @@ error message and return C<NULL>. This callback is optional, but if provided, it must call C<next>, passing C<readonly> and C<exportname> possibly modified according to -how the filter plans to use the plugin. Typically, the filter passes -the same values as it received, or passes readonly=true to provide a +how the filter plans to use the plugin (C<is_tls> is not passed, +because a filter cannot modify it). Typically, the filter passes the +same values as it received, or passes readonly=true to provide a writable layer on top of a read-only backend. However, it is also acceptable to attempt write access to the plugin even if this filter is readonly, such as when a file system mounted read-only still @@ -430,7 +431,10 @@ to be replayed for consistency as part of the mounting process. The C<exportname> string is only guaranteed to be available during the call. If the filter needs to use it (other than immediately passing -it down to the next layer) it must take a copy. +it down to the next layer) it must take a copy. The C<exportname> and +C<is_tls> parameters are provided so that filters do not need to use +the plugin-only interfaces of C<nbdkit_export_name> and +C<nbdkit_is_tls>. The filter should generally call C<next> as its first step, to allocate from the plugin outwards, so that C<.close> running from the diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 9341f282..6237b749 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -728,10 +728,18 @@ connection to be readonly (even if this flag is false) by returning false from the C<.can_write> callback. So if your plugin can only serve read-only, you can ignore this parameter. -This callback is called after the NBD handshake has completed, which -includes TLS authentication (if required). If the plugin defines a -C<.preconnect> callback, then it must be called and return with -success before C<.open> is called. +If the plugin wants to differentiate the content it served based on +client input, then this is the spot to use C<nbdkit_export_name()> to +determine which export the client requested. See also L</EXPORT NAME> +below. + +This callback is called after the NBD handshake has completed; if the +server requires TLS authentication, then that has occurred as well. +But if the server is set up to have optional TLS authentication, you +may check C<nbdkit_is_tls> to learn whether the client has completed +TLS authentication. When running the server in a mode that permits +but not requires TLS, be careful that you do not allow unauthenticated +clients to cause a denial of service against authentication. If there is an error, C<.open> should call C<nbdkit_error> with an error message and return C<NULL>. @@ -1466,6 +1474,22 @@ client data, be cautious when parsing it.> On error, C<nbdkit_error> is called and the call returns C<NULL>. +=head1 AUTHENTICATION + +A server may use C<nbdkit_is_tls> to limit which export names work +until after a client has completed TLS authentication. See +L<nbdkit-tls(1)>. + +=head2 C<nbdkit_is_tls> + + int nbdkit_is_tls (void); + +Return true if the client has completed TLS authentication, or false +if the connection is still plaintext. + +On error (such as calling this function outside of the context of +C<.open>), C<nbdkit_error> is called and the call returns C<-1>. + =head1 PEER NAME It is possible to get the address of the client when you are running diff --git a/docs/nbdkit-tls.pod b/docs/nbdkit-tls.pod index 628512a9..450e0850 100644 --- a/docs/nbdkit-tls.pod +++ b/docs/nbdkit-tls.pod @@ -22,7 +22,10 @@ connect, it will be rejected by the server (in other words, as if the server doesn't support TLS). I<--tls=on> means that the client may choose to connect either with or -without TLS. +without TLS. In this mode, a plugin may wish to serve different +content depending on whether the client has authenticated; the +function C<nbdkit_is_tls()> can be used during the C<.open> callback +for that purpose. Because I<--tls=on> is subject to downgrade attacks where a malicious proxy pretends not to support TLS in order to force either the client diff --git a/filters/log/nbdkit-log-filter.pod b/filters/log/nbdkit-log-filter.pod index 5f690457..721fc118 100644 --- a/filters/log/nbdkit-log-filter.pod +++ b/filters/log/nbdkit-log-filter.pod @@ -74,7 +74,7 @@ before performing a single successful read is: 2020-08-06 02:07:23.080415 ListExports id=1 readonly=0 default_only=0 ... 2020-08-06 02:07:23.080502 ...ListExports id=1 exports=[""] return=0 - 2020-08-06 02:07:23.080712 connection=1 Connect export='' size=0x400 write=1 flush=1 rotational=0 trim=1 zero=2 fua=2 extents=1 cache=2 fast_zero=1 + 2020-08-06 02:07:23.080712 connection=1 Connect export='' tls=0 size=0x400 write=1 flush=1 rotational=0 trim=1 zero=2 fua=2 extents=1 cache=2 fast_zero=1 2020-08-06 02:07:23.080907 connection=1 Read id=1 offset=0x0 count=0x200 ... 2020-08-06 02:07:23.080927 connection=1 ...Read id=1 return=0 (Success) 2020-08-06 02:07:23.081255 connection=1 Disconnect transactions=1 diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod index 678116f2..07d90b57 100644 --- a/plugins/sh/nbdkit-sh-plugin.pod +++ b/plugins/sh/nbdkit-sh-plugin.pod @@ -320,11 +320,12 @@ as a name and no description. =item C<open> - /path/to/script open <readonly> <exportname> + /path/to/script open <readonly> <exportname> <tls> The C<readonly> parameter will be C<true> or C<false>. The C<exportname> parameter, if present, is the export name passed to the -server from the client. +server from the client. The C<tls> parameter, if present, will be +C<true> or C<false> depending on whether the client is using TLS. On success this should print the handle (any string) on stdout and exit with code C<0>. If the handle ends with a newline character then diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h index 08c6abc4..708a1b54 100644 --- a/include/nbdkit-filter.h +++ b/include/nbdkit-filter.h @@ -175,7 +175,7 @@ struct nbdkit_filter { struct nbdkit_exports *exports); void * (*open) (nbdkit_next_open *next, nbdkit_backend *nxdata, - int readonly, const char *exportname); + int readonly, const char *exportname, int is_tls); void (*close) (void *handle); int (*prepare) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h index 86b8565d..e20391b8 100644 --- a/include/nbdkit-plugin.h +++ b/include/nbdkit-plugin.h @@ -146,6 +146,7 @@ struct nbdkit_plugin { extern void nbdkit_set_error (int err); extern const char *nbdkit_export_name (void); +extern int nbdkit_is_tls (void); #define NBDKIT_REGISTER_PLUGIN(plugin) \ NBDKIT_CXX_LANG_C \ diff --git a/server/internal.h b/server/internal.h index f84696ca..d043225a 100644 --- a/server/internal.h +++ b/server/internal.h @@ -367,7 +367,8 @@ struct backend { int (*preconnect) (struct backend *, int readonly); int (*list_exports) (struct backend *, int readonly, int default_only, struct nbdkit_exports *exports); - void *(*open) (struct backend *, int readonly, const char *exportname); + void *(*open) (struct backend *, int readonly, const char *exportname, + int is_tls); int (*prepare) (struct backend *, void *handle, int readonly); int (*finalize) (struct backend *, void *handle); void (*close) (struct backend *, void *handle); diff --git a/server/backend.c b/server/backend.c index 75ca53be..8f4fed9d 100644 --- a/server/backend.c +++ b/server/backend.c @@ -186,8 +186,8 @@ backend_open (struct backend *b, int readonly, const char *exportname) GET_CONN; struct handle *h = get_handle (conn, b->i); - controlpath_debug ("%s: open readonly=%d exportname=\"%s\"", - b->name, readonly, exportname); + controlpath_debug ("%s: open readonly=%d exportname=\"%s\" tls=%d", + b->name, readonly, exportname, conn->using_tls); assert (h->handle == NULL); assert ((h->state & HANDLE_OPEN) == 0); @@ -212,7 +212,7 @@ backend_open (struct backend *b, int readonly, const char *exportname) /* Most filters will call next_open first, resulting in * inner-to-outer ordering. */ - h->handle = b->open (b, readonly, exportname); + h->handle = b->open (b, readonly, exportname, conn->using_tls); controlpath_debug ("%s: open returned handle %p", b->name, h->handle); if (h->handle == NULL) { diff --git a/server/filters.c b/server/filters.c index 20518354..90a9a948 100644 --- a/server/filters.c +++ b/server/filters.c @@ -250,7 +250,8 @@ filter_list_exports (struct backend *b, int readonly, int default_only, } static void * -filter_open (struct backend *b, int readonly, const char *exportname) +filter_open (struct backend *b, int readonly, const char *exportname, + int is_tls) { struct backend_filter *f = container_of (b, struct backend_filter, backend); void *handle; @@ -259,7 +260,8 @@ filter_open (struct backend *b, int readonly, const char *exportname) * inner-to-outer ordering. */ if (f->filter.open) - handle = f->filter.open (backend_open, b->next, readonly, exportname); + handle = f->filter.open (backend_open, b->next, readonly, exportname, + is_tls); else if (backend_open (b->next, readonly, exportname) == -1) handle = NULL; else diff --git a/server/nbdkit.syms b/server/nbdkit.syms index 6cc6ed32..a67669b7 100644 --- a/server/nbdkit.syms +++ b/server/nbdkit.syms @@ -53,6 +53,7 @@ nbdkit_extents_free; nbdkit_extents_new; nbdkit_get_extent; + nbdkit_is_tls; nbdkit_nanosleep; nbdkit_parse_bool; nbdkit_parse_int8_t; diff --git a/server/plugins.c b/server/plugins.c index d4364cd2..bc57623a 100644 --- a/server/plugins.c +++ b/server/plugins.c @@ -292,7 +292,8 @@ plugin_list_exports (struct backend *b, int readonly, int default_only, } static void * -plugin_open (struct backend *b, int readonly, const char *exportname) +plugin_open (struct backend *b, int readonly, const char *exportname, + int is_tls) { GET_CONN; struct backend_plugin *p = container_of (b, struct backend_plugin, backend); @@ -302,11 +303,14 @@ plugin_open (struct backend *b, int readonly, const char *exportname) /* Save the exportname since the lifetime of the string passed in * here is likely to be brief. In addition this provides a place * for nbdkit_export_name to retrieve it if called from the plugin. + * While readonly and the export name can be altered in plugins, the + * tls mode cannot. * - * In API V3 we propose to pass the exportname as an extra parameter - * to the (new) plugin.open and deprecate nbdkit_export_name for V3 - * users. Even then we will still need to save it in the handle - * because of the lifetime issue. + * In API V3 we propose to pass the exportname and tls mode as extra + * parameters to the (new) plugin.open and deprecate + * nbdkit_export_name and nbdkit_is_tls for V3 users. Even then we + * will still need to save the export name in the handle because of + * the lifetime issue. */ if (conn->exportname == NULL) { conn->exportname = strdup (exportname); diff --git a/server/public.c b/server/public.c index c00a5cb6..f682d732 100644 --- a/server/public.c +++ b/server/public.c @@ -670,6 +670,22 @@ nbdkit_export_name (void) return conn->exportname; } +/* This function will be deprecated for API V3 users. The preferred + * approach will be to get the tls mode from .open(). + */ +int +nbdkit_is_tls (void) +{ + struct connection *conn = threadlocal_get_conn (); + + if (!conn) { + nbdkit_error ("no connection in this thread"); + return -1; + } + + return conn->using_tls; +} + int nbdkit_peer_name (struct sockaddr *addr, socklen_t *addrlen) { diff --git a/plugins/sh/methods.c b/plugins/sh/methods.c index 9f247524..c59198ca 100644 --- a/plugins/sh/methods.c +++ b/plugins/sh/methods.c @@ -341,6 +341,7 @@ sh_open (int readonly) { script, method, readonly ? "true" : "false", nbdkit_export_name () ? : "", + nbdkit_is_tls () ? "true" : "false", NULL }; struct sh_handle *h = malloc (sizeof *h); diff --git a/filters/cow/cow.c b/filters/cow/cow.c index 0faf2726..51ca64a4 100644 --- a/filters/cow/cow.c +++ b/filters/cow/cow.c @@ -94,7 +94,7 @@ cow_config (nbdkit_next_config *next, void *nxdata, static void * cow_open (nbdkit_next_open *next, void *nxdata, - int readonly, const char *exportname) + int readonly, const char *exportname, int is_tls) { /* Always pass readonly=1 to the underlying plugin. */ if (next (nxdata, 1, exportname) == -1) diff --git a/filters/exitlast/exitlast.c b/filters/exitlast/exitlast.c index 378cda75..4c879cc9 100644 --- a/filters/exitlast/exitlast.c +++ b/filters/exitlast/exitlast.c @@ -51,7 +51,7 @@ static _Atomic unsigned connections; static void * exitlast_open (nbdkit_next_open *next, nbdkit_backend *nxdata, - int readonly, const char *exportname) + int readonly, const char *exportname, int is_tls) { if (next (nxdata, readonly, exportname) == -1) return NULL; diff --git a/filters/ext2/ext2.c b/filters/ext2/ext2.c index 72b7ac9f..75ac2c4c 100644 --- a/filters/ext2/ext2.c +++ b/filters/ext2/ext2.c @@ -122,7 +122,7 @@ struct handle { /* Create the per-connection handle. */ static void * ext2_open (nbdkit_next_open *next, void *nxdata, - int readonly, const char *exportname) + int readonly, const char *exportname, int is_tls) { struct handle *h; diff --git a/filters/gzip/gzip.c b/filters/gzip/gzip.c index 8323b882..d92e00d9 100644 --- a/filters/gzip/gzip.c +++ b/filters/gzip/gzip.c @@ -75,7 +75,7 @@ gzip_thread_model (void) static void * gzip_open (nbdkit_next_open *next, nbdkit_backend *nxdata, - int readonly, const char *exportname) + int readonly, const char *exportname, int is_tls) { /* Always pass readonly=1 to the underlying plugin. */ if (next (nxdata, 1, exportname) == -1) diff --git a/filters/limit/limit.c b/filters/limit/limit.c index 7c4477eb..fb862df7 100644 --- a/filters/limit/limit.c +++ b/filters/limit/limit.c @@ -91,7 +91,7 @@ limit_preconnect (nbdkit_next_preconnect *next, nbdkit_backend *nxdata, static void * limit_open (nbdkit_next_open *next, nbdkit_backend *nxdata, - int readonly, const char *exportname) + int readonly, const char *exportname, int is_tls) { if (next (nxdata, readonly, exportname) == -1) return NULL; diff --git a/filters/log/log.c b/filters/log/log.c index c89f5001..f8da9ad8 100644 --- a/filters/log/log.c +++ b/filters/log/log.c @@ -135,6 +135,7 @@ struct handle { uint64_t connection; uint64_t id; char *exportname; + int tls; }; /* Compute the next id number on the current connection. */ @@ -283,7 +284,7 @@ log_list_exports (nbdkit_next_list_exports *next, void *nxdata, /* Open a connection. */ static void * log_open (nbdkit_next_open *next, void *nxdata, - int readonly, const char *exportname) + int readonly, const char *exportname, int is_tls) { struct handle *h; @@ -296,9 +297,9 @@ log_open (nbdkit_next_open *next, void *nxdata, return NULL; } - /* Save the exportname in the handle so we can display it in - * log_prepare. We must take a copy because this string has a short - * lifetime. + /* Save the exportname and tls state in the handle so we can display + * it in log_prepare. We must take a copy because this string has a + * short lifetime. */ h->exportname = strdup (exportname); if (h->exportname == NULL) { @@ -306,6 +307,7 @@ log_open (nbdkit_next_open *next, void *nxdata, free (h); return NULL; } + h->tls = is_tls; ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); h->connection = ++connections; @@ -343,9 +345,9 @@ log_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, e < 0 || c < 0 || Z < 0) return -1; - output (h, "Connect", 0, "export='%s' size=0x%" PRIx64 " write=%d flush=%d " - "rotational=%d trim=%d zero=%d fua=%d extents=%d cache=%d " - "fast_zero=%d", exportname, size, w, f, r, t, z, F, e, c, Z); + output (h, "Connect", 0, "export='%s' tls=%d size=0x%" PRIx64 " write=%d " + "flush=%d rotational=%d trim=%d zero=%d fua=%d extents=%d cache=%d " + "fast_zero=%d", exportname, h->tls, size, w, f, r, t, z, F, e, c, Z); return 0; } diff --git a/filters/partition/partition.c b/filters/partition/partition.c index c348664b..849f0cc7 100644 --- a/filters/partition/partition.c +++ b/filters/partition/partition.c @@ -88,7 +88,7 @@ struct handle { /* Open a connection. */ static void * partition_open (nbdkit_next_open *next, void *nxdata, - int readonly, const char *exportname) + int readonly, const char *exportname, int is_tls) { struct handle *h; diff --git a/filters/rate/rate.c b/filters/rate/rate.c index 6e99fcc7..32c47fdf 100644 --- a/filters/rate/rate.c +++ b/filters/rate/rate.c @@ -163,7 +163,7 @@ rate_get_ready (nbdkit_next_get_ready *next, void *nxdata) /* Create the per-connection handle. */ static void * rate_open (nbdkit_next_open *next, void *nxdata, - int readonly, const char *exportname) + int readonly, const char *exportname, int is_tls) { struct rate_handle *h; diff --git a/filters/retry/retry.c b/filters/retry/retry.c index be334c39..a2e57d77 100644 --- a/filters/retry/retry.c +++ b/filters/retry/retry.c @@ -113,7 +113,7 @@ struct retry_handle { static void * retry_open (nbdkit_next_open *next, void *nxdata, - int readonly, const char *exportname) + int readonly, const char *exportname, int is_tls) { struct retry_handle *h; diff --git a/filters/tar/tar.c b/filters/tar/tar.c index c04f09dd..ab041153 100644 --- a/filters/tar/tar.c +++ b/filters/tar/tar.c @@ -113,7 +113,7 @@ struct handle { static void * tar_open (nbdkit_next_open *next, nbdkit_backend *nxdata, - int readonly, const char *exportname) + int readonly, const char *exportname, int is_tls) { struct handle *h; diff --git a/filters/truncate/truncate.c b/filters/truncate/truncate.c index ee384871..00b5d8ce 100644 --- a/filters/truncate/truncate.c +++ b/filters/truncate/truncate.c @@ -125,7 +125,7 @@ struct handle { /* Open a connection. */ static void * truncate_open (nbdkit_next_open *next, void *nxdata, - int readonly, const char *exportname) + int readonly, const char *exportname, int is_tls) { struct handle *h; diff --git a/filters/xz/xz.c b/filters/xz/xz.c index 9154dbeb..26cfa959 100644 --- a/filters/xz/xz.c +++ b/filters/xz/xz.c @@ -90,7 +90,7 @@ struct xz_handle { /* Create the per-connection handle. */ static void * xz_open (nbdkit_next_open *next, void *nxdata, - int readonly, const char *exportname) + int readonly, const char *exportname, int is_tls) { struct xz_handle *h; diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c index 397af575..5c5b3f0f 100644 --- a/tests/test-layers-filter.c +++ b/tests/test-layers-filter.c @@ -117,7 +117,7 @@ test_layers_filter_list_exports (nbdkit_next_list_exports *next, void *nxdata, static void * test_layers_filter_open (nbdkit_next_open *next, void *nxdata, - int readonly, const char *exportname) + int readonly, const char *exportname, int is_tls) { struct handle *h = malloc (sizeof *h); diff --git a/TODO b/TODO index 46f92443..262327fb 100644 --- a/TODO +++ b/TODO @@ -62,11 +62,11 @@ General ideas for improvements continue to keep their non-standard handshake while utilizing nbdkit to prototype new behaviors in serving the kernel. -* When using --tls=on (encryption is supported but not required), - plugins need to be able to differentiate which content they serve to - plaintext clients vs. secure clients. This may require an - alteration of the signature for .list_exports and .open, or a new - utility function nbdkit_tls_mode() similar to nbdkit_export_name(). +* The current signature of .list_exports is awkward; it overloads the + list response to NBD_OPT_LIST with the resolution of the default + export name in .open, and it is missing a tls parameter. It is + probably worth splitting out a new .default_export callback for the + latter purpose, as well as fixing the signature for the former. * Background thread for filters. Some filters (readahead, cache and proposed scan filter - see below) could be more effective if they @@ -308,7 +308,6 @@ using ‘#define NBDKIT_API_VERSION <version>’. and filters, and the type of the values, and both check and parse them for the plugin. -* Modify open() API so it takes an export name parameter, and maybe a - tls mode. +* Modify open() API so it takes an export name and tls parameter. * Change config_help from a variable to a function. -- 2.28.0
Eric Blake
2020-Aug-07 22:00 UTC
[Libguestfs] [nbdkit PATCH 2/3] server: Expose final thread_model to filter's .get_ready
The next patch wants to add a filter that will prevent DoS attacks from a plaintext client; to be successful, the filter must guarantee that nbdkit did not settle on SERIALIZE_CONNECTIONS. The easiest way to solve this is to expose the final thread model to .get_ready, which is after the point where .config_complete may have altered it, and before any connections are permitted. Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/nbdkit-filter.pod | 9 ++++++++- include/nbdkit-filter.h | 3 ++- server/filters.c | 4 ++-- filters/extentlist/extentlist.c | 3 ++- filters/log/log.c | 2 +- filters/rate/rate.c | 2 +- filters/stats/stats.c | 2 +- tests/test-layers-filter.c | 2 +- 8 files changed, 18 insertions(+), 9 deletions(-) diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod index b6ed5504..32db0938 100644 --- a/docs/nbdkit-filter.pod +++ b/docs/nbdkit-filter.pod @@ -298,11 +298,18 @@ with an error message and return C<-1>. =head2 C<.get_ready> - int (*get_ready) (nbdkit_next_get_ready *next, void *nxdata); + int (*get_ready) (nbdkit_next_get_ready *next, void *nxdata, + int thread_model); This intercepts the plugin C<.get_ready> method and can be used by the filter to get ready to serve requests. +The C<thread_model> parameter informs the filter about the final +thread model chosen by nbdkit after considering the results of +C<.thread_model> of all filters in the chain after C<.config>. This +does not need to be passed on to C<next>, as the model can no longer +be altered at this point. + If there is an error, C<.get_ready> should call C<nbdkit_error> with an error message and return C<-1>. diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h index 708a1b54..6aba1aec 100644 --- a/include/nbdkit-filter.h +++ b/include/nbdkit-filter.h @@ -166,7 +166,8 @@ struct nbdkit_filter { nbdkit_backend *nxdata); const char *config_help; int (*thread_model) (void); - int (*get_ready) (nbdkit_next_get_ready *next, nbdkit_backend *nxdata); + int (*get_ready) (nbdkit_next_get_ready *next, nbdkit_backend *nxdata, + int thread_model); int (*after_fork) (nbdkit_next_after_fork *next, nbdkit_backend *nxdata); int (*preconnect) (nbdkit_next_preconnect *next, nbdkit_backend *nxdata, int readonly); diff --git a/server/filters.c b/server/filters.c index 90a9a948..0cfae344 100644 --- a/server/filters.c +++ b/server/filters.c @@ -183,10 +183,10 @@ filter_get_ready (struct backend *b) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - debug ("%s: get_ready", b->name); + debug ("%s: get_ready thread_model=%d", b->name, thread_model); if (f->filter.get_ready) { - if (f->filter.get_ready (next_get_ready, b->next) == -1) + if (f->filter.get_ready (next_get_ready, b->next, thread_model) == -1) exit (EXIT_FAILURE); } else diff --git a/filters/extentlist/extentlist.c b/filters/extentlist/extentlist.c index 3005b790..dfb5e808 100644 --- a/filters/extentlist/extentlist.c +++ b/filters/extentlist/extentlist.c @@ -260,7 +260,8 @@ parse_extentlist (void) } static int -extentlist_get_ready (nbdkit_next_get_ready *next, void *nxdata) +extentlist_get_ready (nbdkit_next_get_ready *next, void *nxdata, + int thread_model) { parse_extentlist (); diff --git a/filters/log/log.c b/filters/log/log.c index f8da9ad8..6a3a9b14 100644 --- a/filters/log/log.c +++ b/filters/log/log.c @@ -100,7 +100,7 @@ log_config_complete (nbdkit_next_config_complete *next, void *nxdata) /* Open the logfile. */ static int -log_get_ready (nbdkit_next_get_ready *next, void *nxdata) +log_get_ready (nbdkit_next_get_ready *next, void *nxdata, int thread_model) { int fd; diff --git a/filters/rate/rate.c b/filters/rate/rate.c index 32c47fdf..325f5657 100644 --- a/filters/rate/rate.c +++ b/filters/rate/rate.c @@ -145,7 +145,7 @@ rate_config (nbdkit_next_config *next, void *nxdata, } static int -rate_get_ready (nbdkit_next_get_ready *next, void *nxdata) +rate_get_ready (nbdkit_next_get_ready *next, void *nxdata, int thread_model) { /* Initialize the global buckets. */ bucket_init (&read_bucket, rate, BUCKET_CAPACITY); diff --git a/filters/stats/stats.c b/filters/stats/stats.c index 688078ec..687dd05b 100644 --- a/filters/stats/stats.c +++ b/filters/stats/stats.c @@ -210,7 +210,7 @@ stats_config_complete (nbdkit_next_config_complete *next, void *nxdata) } static int -stats_get_ready (nbdkit_next_get_ready *next, void *nxdata) +stats_get_ready (nbdkit_next_get_ready *next, void *nxdata, int thread_model) { int fd; diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c index 5c5b3f0f..3f295588 100644 --- a/tests/test-layers-filter.c +++ b/tests/test-layers-filter.c @@ -84,7 +84,7 @@ test_layers_filter_config_complete (nbdkit_next_config_complete *next, static int test_layers_filter_get_ready (nbdkit_next_get_ready *next, - void *nxdata) + void *nxdata, int thread_model) { DEBUG_FUNCTION; return next (nxdata); -- 2.28.0
Take advantage of the fact that we can now detect the type of client during --tls=on in order to provide safe dummy content for plaintext clients without having to rewrite plugins to do so. Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/nbdkit-plugin.pod | 4 +- docs/nbdkit-tls.pod | 5 +- filters/tlsdummy/nbdkit-tlsdummy-filter.pod | 72 ++++++ configure.ac | 2 + filters/tlsdummy/Makefile.am | 63 ++++++ filters/tlsdummy/tlsdummy.c | 235 ++++++++++++++++++++ 6 files changed, 379 insertions(+), 2 deletions(-) create mode 100644 filters/tlsdummy/nbdkit-tlsdummy-filter.pod create mode 100644 filters/tlsdummy/Makefile.am create mode 100644 filters/tlsdummy/tlsdummy.c diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 6237b749..43b0f6f9 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -1478,7 +1478,9 @@ On error, C<nbdkit_error> is called and the call returns C<NULL>. A server may use C<nbdkit_is_tls> to limit which export names work until after a client has completed TLS authentication. See -L<nbdkit-tls(1)>. +L<nbdkit-tls(1)>. It is also possible to use +L<nbdkit-tlsdummy-filter(1)> to automatically ensure that the plugin +is only used with authentication. =head2 C<nbdkit_is_tls> diff --git a/docs/nbdkit-tls.pod b/docs/nbdkit-tls.pod index 450e0850..fd78c21b 100644 --- a/docs/nbdkit-tls.pod +++ b/docs/nbdkit-tls.pod @@ -25,7 +25,9 @@ I<--tls=on> means that the client may choose to connect either with or without TLS. In this mode, a plugin may wish to serve different content depending on whether the client has authenticated; the function C<nbdkit_is_tls()> can be used during the C<.open> callback -for that purpose. +for that purpose. Or, you can opt to use L<nbdkit-tlsdummy-filter(1)> +to provide safe dummy content to plaintext connections, saving the +plugin content only for secure connections. Because I<--tls=on> is subject to downgrade attacks where a malicious proxy pretends not to support TLS in order to force either the client @@ -275,6 +277,7 @@ More information can be found in L<gnutls_priority_init(3)>. =head1 SEE ALSO L<nbdkit(1)>, +L<nbdkit-tlsdummy-filter(1)>, L<gnutls_priority_init(3)>, L<psktool(1)>, L<https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md>, diff --git a/filters/tlsdummy/nbdkit-tlsdummy-filter.pod b/filters/tlsdummy/nbdkit-tlsdummy-filter.pod new file mode 100644 index 00000000..f8eef69f --- /dev/null +++ b/filters/tlsdummy/nbdkit-tlsdummy-filter.pod @@ -0,0 +1,72 @@ +=head1 NAME + +nbdkit-tlsdummy-filter - nbdkit TLS protection filter + +=head1 SYNOPSIS + + nbdkit --tls=on --filter=tlsdummy plugin [plugin-args...] [tlsreadme=MESSAGE] + +=head1 DESCRIPTION + +C<nbdkit-tlsdummy-filter> is designed to be used when offering a +connection that allows but does not require TLS from clients, in order +to offer safe alternative content to plaintext clients, only exposing +the underlying plugin to authenticated users. This may provide a +nicer failure mode for plaintext clients than the harsher C<nbdkit +--tls=require>. + +When this filter detects a plaintext connection, it ignores the +client's export name, and provides a single read-only export with 512 +bytes of data and content that defaults to the message "This NBD +server requires TLS authentication before it will serve useful data." + +When using this filter, it is recommended to place this filter first +in the command line, to reduce the chance that any work done by +C<.open> in later filters or the plugin can be exploited by plaintext +connections as a denial of service attack to starve further +authenticated connections. Note that this plugin will fail to load if +the plugin requests the C<SERIALIZE_CONNECTIONS> thread model, as a +plaintext client holding its connection open indefinitely would be +such a starvation. + +=head1 PARAMETERS + +=over 4 + +=item B<tlsreadme=>MESSAGE + +This optional parameter can be used to use C<MESSAGE> as the contents +of the dummy export exposed to plaintext clients, using trailing NUL +bytes to round the size up to 512 bytes. + +=back + +=head1 FILES + +=over 4 + +=item F<$filterdir/nbdkit-tlsdummy-filter.so> + +The filter. + +Use C<nbdkit --dump-config> to find the location of C<$filterdir>. + +=back + +=head1 VERSION + +C<nbdkit-tlsdummy-filter> first appeared in nbdkit 1.22. + +=head1 SEE ALSO + +L<nbdkit(1)>, +L<nbdkit-tls(1)>, +L<nbdkit-filter(3)>. + +=head1 AUTHORS + +Eric Blake + +=head1 COPYRIGHT + +Copyright (C) 2020 Red Hat Inc. diff --git a/configure.ac b/configure.ac index 28f4a3cd..34730000 100644 --- a/configure.ac +++ b/configure.ac @@ -124,6 +124,7 @@ filters="\ stats \ swab \ tar \ + tlsdummy \ truncate \ xz \ " @@ -1165,6 +1166,7 @@ AC_CONFIG_FILES([Makefile filters/stats/Makefile filters/swab/Makefile filters/tar/Makefile + filters/tlsdummy/Makefile filters/truncate/Makefile filters/xz/Makefile fuzzing/Makefile diff --git a/filters/tlsdummy/Makefile.am b/filters/tlsdummy/Makefile.am new file mode 100644 index 00000000..a1577511 --- /dev/null +++ b/filters/tlsdummy/Makefile.am @@ -0,0 +1,63 @@ +# 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 $(top_srcdir)/common-rules.mk + +EXTRA_DIST = nbdkit-tlsdummy-filter.pod + +filter_LTLIBRARIES = nbdkit-tlsdummy-filter.la + +nbdkit_tlsdummy_filter_la_SOURCES = \ + tlsdummy.c \ + $(top_srcdir)/include/nbdkit-filter.h \ + $(NULL) + +nbdkit_tlsdummy_filter_la_CPPFLAGS = \ + -I$(top_srcdir)/include \ + -I$(top_srcdir)/common/include \ + $(NULL) +nbdkit_tlsdummy_filter_la_CFLAGS = $(WARNINGS_CFLAGS) +nbdkit_tlsdummy_filter_la_LDFLAGS = \ + -module -avoid-version -shared $(SHARED_LDFLAGS) \ + -Wl,--version-script=$(top_srcdir)/filters/filters.syms \ + $(NULL) + +if HAVE_POD + +man_MANS = nbdkit-tlsdummy-filter.1 +CLEANFILES += $(man_MANS) + +nbdkit-tlsdummy-filter.1: nbdkit-tlsdummy-filter.pod + $(PODWRAPPER) --section=1 --man $@ \ + --html $(top_builddir)/html/$@.html \ + $< + +endif HAVE_POD diff --git a/filters/tlsdummy/tlsdummy.c b/filters/tlsdummy/tlsdummy.c new file mode 100644 index 00000000..6e1a7414 --- /dev/null +++ b/filters/tlsdummy/tlsdummy.c @@ -0,0 +1,235 @@ +/* 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 <string.h> +#include <assert.h> + +#include <nbdkit-filter.h> + +/* Needed to shut up newer gcc about use of strncpy on our message buffer */ +#if __GNUC__ >= 8 +#define NONSTRING __attribute__ ((nonstring)) +#else +#define NONSTRING +#endif + +static char message[512] NONSTRING = "This NBD server requires TLS " + "authentication before it will serve useful data.\n"; + +/* Called for each key=value passed on the command line. */ +static int +tlsdummy_config (nbdkit_next_config *next, void *nxdata, + const char *key, const char *value) +{ + if (strcmp (key, "tlsreadme") == 0) { + strncpy (message, value, sizeof message); /* Yes, we really mean strncpy */ + return 0; + } + return next (nxdata, key, value); +} + +#define tlsdummy_config_help \ + "tlsreadme=<MESSAGE> Alternative contents for the plaintext dummy export.\n" + +int +tlsdummy_get_ready (nbdkit_next_get_ready *next, void *nxdata, + int thread_model) +{ + if (thread_model == NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) { + nbdkit_error ("the tlsdummy filter requires parallel connection support"); + return -1; + } + return next (nxdata); +} + +/* TODO: init_exports needs is_tls parameter */ + +/* Helper for determining if this connection is insecure. This works + * because we can treat all handles on a binary basis: secure or + * insecure, which lets .open get away without allocating a more + * complex handle. + */ +#define NOT_TLS (handle == &message) + +static void * +tlsdummy_open (nbdkit_next_open *next, void *nxdata, int readonly, + const char *exportname, int is_tls) +{ + /* Bummer - we really do NOT want to call next() when insecure, + * because we don't know how long it will take. But that requires a + * fix to nbdkit; right now, it asserts if we skip this :( + */ + if (next (nxdata, readonly, exportname) == -1) + return NULL; + if (!is_tls) + return &message; /* See NOT_TLS for this choice of handle */ + return NBDKIT_HANDLE_NOT_NEEDED; +} + +/* When insecure, override ALL plugin .can_FOO to avoid an information leak */ + +static int64_t +tlsdummy_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + if (NOT_TLS) + return sizeof message; + return next_ops->get_size (nxdata); +} + +static int +tlsdummy_can_write (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + if (NOT_TLS) + return 0; + return next_ops->can_write (nxdata); +} + +static int +tlsdummy_can_flush (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + if (NOT_TLS) + return 0; + return next_ops->can_flush (nxdata); +} + +static int +tlsdummy_is_rotational (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + if (NOT_TLS) + return 0; + return next_ops->is_rotational (nxdata); +} + +static int +tlsdummy_can_trim (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + if (NOT_TLS) + return 0; + return next_ops->can_trim (nxdata); +} + +static int +tlsdummy_can_zero (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + if (NOT_TLS) + return NBDKIT_ZERO_NONE; + return next_ops->can_zero (nxdata); +} + +static int +tlsdummy_can_fast_zero (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + if (NOT_TLS) + return 0; + return next_ops->can_fast_zero (nxdata); +} + +static int +tlsdummy_can_extents (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + if (NOT_TLS) + return 0; + return next_ops->can_extents (nxdata); +} + +static int +tlsdummy_can_fua (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + if (NOT_TLS) + return NBDKIT_FUA_NONE; + return next_ops->can_fua (nxdata); +} + +static int +tlsdummy_can_multi_conn (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + if (NOT_TLS) + return 0; + return next_ops->can_multi_conn (nxdata); +} + +static int +tlsdummy_can_cache (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + if (NOT_TLS) + return NBDKIT_CACHE_NONE; + return next_ops->can_cache (nxdata); +} + +static int +tlsdummy_pread (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, void *b, uint32_t count, uint64_t offs, + uint32_t flags, int *err) +{ + if (NOT_TLS) { + memcpy (b, message + offs, count); + return 0; + } + return next_ops->pread (nxdata, b, count, offs, flags, err); +} + +static struct nbdkit_filter filter = { + .name = "tlsdummy", + .longname = "nbdkit tlsdummy filter", + .config = tlsdummy_config, + .config_help = tlsdummy_config_help, + .get_ready = tlsdummy_get_ready, + /* XXX .init_exports needs is_tls parameter */ + .open = tlsdummy_open, + .get_size = tlsdummy_get_size, + .can_write = tlsdummy_can_write, + .can_flush = tlsdummy_can_flush, + .is_rotational = tlsdummy_is_rotational, + .can_trim = tlsdummy_can_trim, + .can_zero = tlsdummy_can_zero, + .can_fast_zero = tlsdummy_can_fast_zero, + .can_extents = tlsdummy_can_extents, + .can_fua = tlsdummy_can_fua, + .can_multi_conn = tlsdummy_can_multi_conn, + .can_cache = tlsdummy_can_cache, + .pread = tlsdummy_pread, +}; + +NBDKIT_REGISTER_FILTER(filter) -- 2.28.0
On 8/7/20 5:00 PM, Eric Blake wrote:> Take advantage of the fact that we can now detect the type of client > during --tls=on in order to provide safe dummy content for plaintext > clients without having to rewrite plugins to do so. > > Signed-off-by: Eric Blake <eblake@redhat.com> > ---I got a test working (although it still shows that we are bit awkward until nbdkit makes it easier for filters to skip calling next_open when it wants to) diff --git c/tests/Makefile.am i/tests/Makefile.am index b5ef96a7..dd756723 100644 --- c/tests/Makefile.am +++ i/tests/Makefile.am @@ -1501,6 +1501,10 @@ EXTRA_DIST += \ test-truncate-extents.sh \ $(NULL) +# tlsdummy filter test. +TESTS += test-tlsdummy.sh +EXTRA_DIST += test-tlsdummy.sh + # xz filter test. LIBGUESTFS_TESTS += test-xz diff --git c/tests/test-tlsdummy.sh i/tests/test-tlsdummy.sh new file mode 100755 index 00000000..40c29602 --- /dev/null +++ i/tests/test-tlsdummy.sh @@ -0,0 +1,98 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2019-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. + +source ./functions.sh +set -e +set -x + +requires nbdsh -c 'exit (not h.supports_tls ())' + +# Does the nbdkit binary support TLS? +if ! nbdkit --dump-config | grep -sq tls=yes; then + echo "$0: nbdkit built without TLS support" + exit 77 +fi + +# Did we create the PSK keys file? +# Probably 'certtool' is missing. +if [ ! -s keys.psk ]; then + echo "$0: PSK keys file was not created by the test harness" + exit 77 +fi + +export sock=`mktemp -u` +pid="tlsdummy.pid" + +files="$sock $pid" +rm -f $files +cleanup_fn rm -f $files + +# Run dual-mode server +start_nbdkit -P $pid -U $sock --tls=on --tls-psk=keys.psk \ + --filter=tlsdummy sh - tlsreadme=$'dummy\n' <<\EOF +if test "$2" = insecure; then + echo 'EINVAL insecure handle' 2>&1; exit 1 +fi +case $1 in + list_exports) echo NAMES; echo hello; echo world ;; + open) if test "$4" != true; then + # nbdkit needs a fix to let tlsdummy skip .open when insecure... + # echo 'EINVAL unexpected tls mode' 2>&1; exit 1 + echo insecure; exit 0 + fi + echo $3 ;; + get_size) echo 6 ;; + pread) echo "$2" | dd skip=$4 count=$3 iflag=skip_bytes,count_bytes ;; + *) exit 2; +esac +EOF + +# Plaintext client sees only dummy volume +nbdsh -c ' +import os +h.set_export_name ("hello") +h.connect_unix (os.environ["sock"]) +assert h.get_size () == 512 +assert h.pread (5, 0) == b"dummy" +' + +# Encrypted client sees desired volumes +nbdsh -c ' +import os +h.set_export_name ("hello") +h.set_tls (nbd.TLS_REQUIRE) +h.set_tls_psk_file ("keys.psk") +h.set_tls_username ("qemu") +h.connect_unix (os.environ["sock"]) +assert h.get_size () == 6 +assert h.pread (5, 0) == b"hello" +' -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Richard W.M. Jones
2020-Aug-10 12:59 UTC
Re: [Libguestfs] [nbdkit PATCH 1/3] server: Implement nbdkit_is_tls for use during .open
Largely a mechanical patch, and the changes at the beginning adding the new API and updating documentation were fine, so ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Richard W.M. Jones
2020-Aug-10 13:01 UTC
Re: [Libguestfs] [nbdkit PATCH 2/3] server: Expose final thread_model to filter's .get_ready
On Fri, Aug 07, 2020 at 05:00:52PM -0500, Eric Blake wrote:> The next patch wants to add a filter that will prevent DoS attacks > from a plaintext client; to be successful, the filter must guarantee > that nbdkit did not settle on SERIALIZE_CONNECTIONS. The easiest way > to solve this is to expose the final thread model to .get_ready, which > is after the point where .config_complete may have altered it, and > before any connections are permitted. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > docs/nbdkit-filter.pod | 9 ++++++++- > include/nbdkit-filter.h | 3 ++- > server/filters.c | 4 ++-- > filters/extentlist/extentlist.c | 3 ++- > filters/log/log.c | 2 +- > filters/rate/rate.c | 2 +- > filters/stats/stats.c | 2 +- > tests/test-layers-filter.c | 2 +- > 8 files changed, 18 insertions(+), 9 deletions(-)This is fine. Is this something that we would also with to add to plugin->get_ready in V3 API? If so it would be a good idea to add this to TODO. ACK Rich.> diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod > index b6ed5504..32db0938 100644 > --- a/docs/nbdkit-filter.pod > +++ b/docs/nbdkit-filter.pod > @@ -298,11 +298,18 @@ with an error message and return C<-1>. > > =head2 C<.get_ready> > > - int (*get_ready) (nbdkit_next_get_ready *next, void *nxdata); > + int (*get_ready) (nbdkit_next_get_ready *next, void *nxdata, > + int thread_model); > > This intercepts the plugin C<.get_ready> method and can be used by the > filter to get ready to serve requests. > > +The C<thread_model> parameter informs the filter about the final > +thread model chosen by nbdkit after considering the results of > +C<.thread_model> of all filters in the chain after C<.config>. This > +does not need to be passed on to C<next>, as the model can no longer > +be altered at this point. > + > If there is an error, C<.get_ready> should call C<nbdkit_error> with > an error message and return C<-1>. > > diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h > index 708a1b54..6aba1aec 100644 > --- a/include/nbdkit-filter.h > +++ b/include/nbdkit-filter.h > @@ -166,7 +166,8 @@ struct nbdkit_filter { > nbdkit_backend *nxdata); > const char *config_help; > int (*thread_model) (void); > - int (*get_ready) (nbdkit_next_get_ready *next, nbdkit_backend *nxdata); > + int (*get_ready) (nbdkit_next_get_ready *next, nbdkit_backend *nxdata, > + int thread_model); > int (*after_fork) (nbdkit_next_after_fork *next, nbdkit_backend *nxdata); > int (*preconnect) (nbdkit_next_preconnect *next, nbdkit_backend *nxdata, > int readonly); > diff --git a/server/filters.c b/server/filters.c > index 90a9a948..0cfae344 100644 > --- a/server/filters.c > +++ b/server/filters.c > @@ -183,10 +183,10 @@ filter_get_ready (struct backend *b) > { > struct backend_filter *f = container_of (b, struct backend_filter, backend); > > - debug ("%s: get_ready", b->name); > + debug ("%s: get_ready thread_model=%d", b->name, thread_model); > > if (f->filter.get_ready) { > - if (f->filter.get_ready (next_get_ready, b->next) == -1) > + if (f->filter.get_ready (next_get_ready, b->next, thread_model) == -1) > exit (EXIT_FAILURE); > } > else > diff --git a/filters/extentlist/extentlist.c b/filters/extentlist/extentlist.c > index 3005b790..dfb5e808 100644 > --- a/filters/extentlist/extentlist.c > +++ b/filters/extentlist/extentlist.c > @@ -260,7 +260,8 @@ parse_extentlist (void) > } > > static int > -extentlist_get_ready (nbdkit_next_get_ready *next, void *nxdata) > +extentlist_get_ready (nbdkit_next_get_ready *next, void *nxdata, > + int thread_model) > { > parse_extentlist (); > > diff --git a/filters/log/log.c b/filters/log/log.c > index f8da9ad8..6a3a9b14 100644 > --- a/filters/log/log.c > +++ b/filters/log/log.c > @@ -100,7 +100,7 @@ log_config_complete (nbdkit_next_config_complete *next, void *nxdata) > > /* Open the logfile. */ > static int > -log_get_ready (nbdkit_next_get_ready *next, void *nxdata) > +log_get_ready (nbdkit_next_get_ready *next, void *nxdata, int thread_model) > { > int fd; > > diff --git a/filters/rate/rate.c b/filters/rate/rate.c > index 32c47fdf..325f5657 100644 > --- a/filters/rate/rate.c > +++ b/filters/rate/rate.c > @@ -145,7 +145,7 @@ rate_config (nbdkit_next_config *next, void *nxdata, > } > > static int > -rate_get_ready (nbdkit_next_get_ready *next, void *nxdata) > +rate_get_ready (nbdkit_next_get_ready *next, void *nxdata, int thread_model) > { > /* Initialize the global buckets. */ > bucket_init (&read_bucket, rate, BUCKET_CAPACITY); > diff --git a/filters/stats/stats.c b/filters/stats/stats.c > index 688078ec..687dd05b 100644 > --- a/filters/stats/stats.c > +++ b/filters/stats/stats.c > @@ -210,7 +210,7 @@ stats_config_complete (nbdkit_next_config_complete *next, void *nxdata) > } > > static int > -stats_get_ready (nbdkit_next_get_ready *next, void *nxdata) > +stats_get_ready (nbdkit_next_get_ready *next, void *nxdata, int thread_model) > { > int fd; > > diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c > index 5c5b3f0f..3f295588 100644 > --- a/tests/test-layers-filter.c > +++ b/tests/test-layers-filter.c > @@ -84,7 +84,7 @@ test_layers_filter_config_complete (nbdkit_next_config_complete *next, > > static int > test_layers_filter_get_ready (nbdkit_next_get_ready *next, > - void *nxdata) > + void *nxdata, int thread_model) > { > DEBUG_FUNCTION; > return next (nxdata); > -- > 2.28.0 > > _______________________________________________ > Libguestfs mailing list > Libguestfs@redhat.com > https://www.redhat.com/mailman/listinfo/libguestfs-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
On 8/7/20 5:00 PM, Eric Blake wrote:> Take advantage of the fact that we can now detect the type of client > during --tls=on in order to provide safe dummy content for plaintext > clients without having to rewrite plugins to do so. > > Signed-off-by: Eric Blake <eblake@redhat.com> > ---After playing with this a bit more, I'm thinking: nbdkit --tls=on --filter=tls-fallback PLUGIN ... reads better than tlsdummy. Renaming the filter is not too difficult of a search-and-replace, so I'll go ahead and make that change before pushing this.> docs/nbdkit-plugin.pod | 4 +- > docs/nbdkit-tls.pod | 5 +- > filters/tlsdummy/nbdkit-tlsdummy-filter.pod | 72 ++++++ > configure.ac | 2 + > filters/tlsdummy/Makefile.am | 63 ++++++ > filters/tlsdummy/tlsdummy.c | 235 ++++++++++++++++++++ > 6 files changed, 379 insertions(+), 2 deletions(-) > create mode 100644 filters/tlsdummy/nbdkit-tlsdummy-filter.pod > create mode 100644 filters/tlsdummy/Makefile.am > create mode 100644 filters/tlsdummy/tlsdummy.c >-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Reasonably Related Threads
- [nbdkit PATCH 2/3] server: Expose final thread_model to filter's .get_ready
- Re: Plans for nbdkit 1.18 release?
- [PATCH nbdkit 1/2] server: Add .after_fork callback, mainly for plugins to create threads.
- Re: [nbdkit PATCH 2/3] server: Expose final thread_model to filter's .get_ready
- [nbdkit PATCH 0/3] Content differentiation during --tls=on