On 9/15/19 9:55 AM, Richard W.M. Jones wrote: > --- Short commit message; at a minimum, I'd probably at least mention that we thought about potential security issues, and didn't see how it could be abused.
> .../reflection/nbdkit-reflection-plugin.pod | 23 ++++- > plugins/reflection/reflection.c | 88 +++++++++++++++++++ > tests/Makefile.am | 2 + > tests/test-reflection-address.sh | 63 +++++++++++++ > 4 files changed, 174 insertions(+), 2 deletions(-) > > diff --git a/plugins/reflection/nbdkit-reflection-plugin.pod > b/plugins/reflection/nbdkit-reflection-plugin.pod > index 1b260b6..7f52c58 100644 > --- a/plugins/reflection/nbdkit-reflection-plugin.pod > +++ b/plugins/reflection/nbdkit-reflection-plugin.pod > @@ -1,10 +1,10 @@ > =head1 NAME > > -nbdkit-reflection-plugin - reflect export name back to the client > +nbdkit-reflection-plugin - reflect client info back to the client Could perhaps squash this hunk into patch 1, but it's also fine here. > +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'? > + > + 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' > +++ 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) > +# 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. 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. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
