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