[Libguestfs] [PATCH nbdkit] tls: Implement Pre-Shared Keys (PSK) authentication.
Richard W.M. Jones
rjones at redhat.com
Thu Jun 28 16:02:40 UTC 2018
On Thu, Jun 28, 2018 at 10:18:25AM -0500, Eric Blake wrote:
> On 06/25/2018 12:01 PM, Richard W.M. Jones wrote:
> >---
> > docs/nbdkit.pod.in | 45 +++++++++--
> > src/crypto.c | 234 +++++++++++++++++++++++++++++++++++++----------------
> > src/internal.h | 1 +
> > src/main.c | 8 +-
> > 4 files changed, 210 insertions(+), 78 deletions(-)
> >
>
> >+Create a PSK file containing one or more C<username:key> pairs. It is
> >+easiest to use L<psktool(1)> for this:
> >+
> >+ psktool -u rich -p /tmp/psk
> >+
> >+The PSK file contains the hex-encoded random keys in plaintext. Any
> >+client which can read this file will be able to connect to the server.
>
> If I'm understanding correctly, it's also possible for a server to
> create a file that supports multiple users:
>
> c1:1234
> c2:2345
>
> but then hand out a smaller file to client c1 that contains only the
> c2:1234 line, and another small file to client c2, and then a single
> server can not only accept multiple clients but also distinguish
> which client connected.
Correct.
To actually distinguish which client is connected you can call
gnutls_psk_server_get_username(3) in the server to return the user who
connected, although I believe this only works after a successful
handshake has completed (so no good for debugging failed connections
for example).
> Is there any reason for nbdkit as a server
> to additionally limit which resources are served depending on which
> client connected?
It could be done (maybe passed to the plugin?)
At the moment nbdkit doesn't distinguish based on client, eg. with
client certificate CN or IP address.
> >+ err = gnutls_psk_allocate_server_credentials (&psk_creds);
> >+ if (err < 0) {
> >+ print_gnutls_error (err, "allocating PSK credentials");
> >+ exit (EXIT_FAILURE);
>
> Do you care about lint-like efforts to free abs_psk_file before
> exit? (I don't)
Not on abort/exit paths. If it was returning then definitely.
> >+ }
> >+
> >+ /* Note that this function makes a copy of the string so it
> >+ * is safe to free it afterwards.
> >+ */
> >+ gnutls_psk_set_server_credentials_file (psk_creds, abs_psk_file);
> >+
> >+ return 0;
>
> Based on the comment, isn't this a leak of abs_psk_file?
It's marked with CLEANUP_FREE :-)
The comment is a bit confusing though, so I'll try to reword it.
> Overall it looks good. The best part is that you've done
> interoperability testing with your pending qemu patch (I have not
> yet tried to repeat the interoperability testing at this point,
> though).
That's actually very simple. In the nbdkit top build directory, and
assuming you've patched and compiled qemu in ~/d/qemu:
$ make -C tests check PATH=~/d/qemu:$PATH TESTS=test-tls-psk.sh
Thanks for the rest of the review, I'll update the patch based on your
comments.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
More information about the Libguestfs
mailing list