[PATCH 0/3] nbd: Try for cleaner TLS shutdown

Eric Blake posted 3 patches 4 years ago
Test docker-mingw@fedora passed
Test docker-quick@centos7 failed
Test checkpatch passed
Test FreeBSD passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200327161936.2225989-1-eblake@redhat.com
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Markus Armbruster <armbru@redhat.com>, Eric Blake <eblake@redhat.com>
qapi/crypto.json            | 15 +++++++++++++++
include/crypto/tlssession.h | 24 ++++++++++++++++++++++++
block/nbd.c                 |  1 +
crypto/tlssession.c         | 27 +++++++++++++++++++++++++++
io/channel-tls.c            | 27 ++++++++++++++++++++++++++-
nbd/client.c                |  3 ++-
nbd/server.c                |  4 ++++
7 files changed, 99 insertions(+), 2 deletions(-)
[PATCH 0/3] nbd: Try for cleaner TLS shutdown
Posted by Eric Blake 4 years ago
I'm currently trying to debug a hang in 'nbdkit nbd' as a client when
TLS is in use, when dealing with three processes:
nbdkit example1[1] <=tls=> nbdkit nbd[2] <=plain=> qemu-nbd --list[3]

In theory, when process [3] exits, the server side of process [2] is
supposed to disconnect as client from process [1].  But we were
hitting sporadic hangs where process [2] was stuck in a poll() while
in gnutls cleanup code, which appears to be because it did not
recognize a clean shutdown from process [3].

I do not yet have a strong reproducible test case (which might include
adding strategic sleep()s in one-off compilations) to prove that the
problem is an unclean TLS shutdown, but at the same time, the gnutls
documentation is clear that without proper use of gnutls_bye(), a
client cannot distinguish between clean end of communications and a
malicious interruption in service.  Shutting down the write side of a
bi-directional socket as soon as possible makes it easier for a client
to begin its own shutdown actions, and can help in avoiding deadlocks
where both sides of a connection are waiting for the other side to
close() the socket first.  If I can come up with a reliable test case,
this series may be a candidate for 5.0; but for now, I'm only
proposing it for 5.1 (we've had more than one release where qemu was
not exploiting the full power of gnutls_bye(), and even if that
triggers poor interaction with other endpoints such as nbdkit, going
one more release with the same behavior isn't making things worse).

Eric Blake (3):
  crypto: Add qcrypto_tls_shutdown()
  io: Support shutdown of TLS channel
  nbd: Use shutdown(SHUT_WR) after last item sent

 qapi/crypto.json            | 15 +++++++++++++++
 include/crypto/tlssession.h | 24 ++++++++++++++++++++++++
 block/nbd.c                 |  1 +
 crypto/tlssession.c         | 27 +++++++++++++++++++++++++++
 io/channel-tls.c            | 27 ++++++++++++++++++++++++++-
 nbd/client.c                |  3 ++-
 nbd/server.c                |  4 ++++
 7 files changed, 99 insertions(+), 2 deletions(-)

-- 
2.26.0.rc2


Re: [PATCH 0/3] nbd: Try for cleaner TLS shutdown
Posted by no-reply@patchew.org 4 years ago
Patchew URL: https://patchew.org/QEMU/20200327161936.2225989-1-eblake@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

io/channel-tls.o: In function `qio_channel_tls_shutdown':
/tmp/qemu-test/src/io/channel-tls.c:373: undefined reference to `qcrypto_tls_session_shutdown'
/tmp/qemu-test/src/io/channel-tls.c:376: undefined reference to `qcrypto_tls_session_shutdown'
collect2: error: ld returned 1 exit status
make: *** [scsi/qemu-pr-helper] Error 1
make: *** Waiting for unfinished jobs....
io/channel-tls.o: In function `qio_channel_tls_shutdown':
/tmp/qemu-test/src/io/channel-tls.c:373: undefined reference to `qcrypto_tls_session_shutdown'
/tmp/qemu-test/src/io/channel-tls.c:376: undefined reference to `qcrypto_tls_session_shutdown'
collect2: error: ld returned 1 exit status
make: *** [qemu-nbd] Error 1
io/channel-tls.o: In function `qio_channel_tls_shutdown':
/tmp/qemu-test/src/io/channel-tls.c:373: undefined reference to `qcrypto_tls_session_shutdown'
/tmp/qemu-test/src/io/channel-tls.c:376: undefined reference to `qcrypto_tls_session_shutdown'
collect2: error: ld returned 1 exit status
make: *** [qemu-io] Error 1
io/channel-tls.o: In function `qio_channel_tls_shutdown':
/tmp/qemu-test/src/io/channel-tls.c:373: undefined reference to `qcrypto_tls_session_shutdown'
/tmp/qemu-test/src/io/channel-tls.c:376: undefined reference to `qcrypto_tls_session_shutdown'
collect2: error: ld returned 1 exit status
make: *** [qemu-storage-daemon] Error 1
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
    sys.exit(main())
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=89d889bd9dd84a6592e526b967f6a8cc', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-xtkcxxmy/src/docker-src.2020-03-27-14.42.04.29238:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=89d889bd9dd84a6592e526b967f6a8cc
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-xtkcxxmy/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    2m13.487s
user    0m8.419s


The full log is available at
http://patchew.org/logs/20200327161936.2225989-1-eblake@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com