Re: [Libguestfs] [PATCH nbdkit 4/4] reflection: Enhance plugin to support client address mode.
On Mon, Sep 16, 2019 at 11:01:49AM -0500, Eric Blake wrote:
On 9/15/19 9:55 AM, Richard W.M. Jones wrote:
> +Another use for the reflection plugin is to send back the client's IP
> +address:
> +
> + $ nbdkit reflection mode=address
> + $ nbdsh -u 'nbd://localhost' -c 'print(h.pread(h.get_size(), 0))'
> +
> +which will print something like:
> +
> + b'[::1]:58912'
Do we want a mode that attempts to do DNS lookup to convert an address
back to a name, so that this could result in b'localhost:58912'?
I suppose this could reveal too much information about the server's
(DNS?) configuration?
> +
> + case AF_UNIX:
> + /* We don't want to expose the socket path because it's a host
> + * filesystem name. The client might not really be running on the
> + * same machine (eg. it using a proxy). However it doesn't even
missing 'is'
Will fix.
> +++ b/tests/test-reflection-address.sh
> @@ -0,0 +1,63 @@
> +# Test the relection plugin with mode=address.
reflection (hmm, I missed that typo in patch 1, where this was copied from)
Oops - fixed in all 3 places.
> +# Run nbdkit.
> +start_nbdkit -P reflection-address.pid -U $sock \
> + reflection mode=address
Is it worth also trying to test a TCP socket (although we have to worry
about finding a free port for the server, as well as heavily filtering
the result as it might be [::1] or 127.0.0.1, and most certainly will
have a different client port number per test run.
Yes it's because of this problem that I'm only testing a Unix domain
socket here.
We do have a few tests which use localhost, but they occasionally fail
in Koji especially because our algorithm to pick a free port is racy:
https://github.com/libguestfs/nbdkit/blob/03a2cc3d766edb17011dafe939e53d0...
As far as I know there's not a good way to fix this except to have
nbdkit choose a port, but that way is problematic in other ways.
Perhaps there is some way to have an external process hang on to a
port and pass it to nbdkit (using -s?)
But overall, I think this is a useful thing to add to the plugin,
and
I'm not seeing any security holes in letting the client know the
client's IP address as seen by the server.
OK I'll fix up all the things you have mentioned and push it, thanks.
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