Re: [Libguestfs] [PATCH libnbd 1/2] lib/crypto: Use GNUTLS_NO_SIGNAL if available

On 07/27/22 18:30, Richard W.M. Jones wrote: > libnbd has long used MSG_NOSIGNAL to avoid receiving SIGPIPE if we > accidentally write on a closed socket, which is a nice alternative to > using a SIGPIPE signal handler. However with TLS connections, gnutls > did not use this flag and so programs using libnbd + TLS would receive > SIGPIPE in some situations, notably if the server closed the > connection abruptly while we were trying to write something. > > GnuTLS 3.4.2 introduces GNUTLS_NO_SIGNAL which does the same thing. > Use this flag if available. > > RHEL 7 has an older gnutls which lacks this flag. To avoid qemu-nbd > interop tests failing (rarely, but more often with a forthcoming > change to TLS shutdown behaviour), register a SIGPIPE signal handler > in the test if the flag is missing. > --- > configure.ac | 15 +++++++++++++++ > interop/interop.c | 10 ++++++++++ > lib/crypto.c | 7 ++++++- > 3 files changed, 31 insertions(+), 1 deletion(-) > > diff --git a/configure.ac b/configure.ac > index 86c3a08690..b5bae4f1b2 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -182,6 +182,21 @@ AS_IF([test "$GNUTLS_LIBS" != ""],[ > gnutls_session_set_verify_cert \ > gnutls_transport_is_ktls_enabled \ > ]) > + AC_MSG_CHECKING([if gnutls has GNUTLS_NO_SIGNAL]) > + AC_COMPILE_IFELSE( > + [AC_LANG_PROGRAM([ > + #include <gnutls/gnutls.h> > + gnutls_session_t session; > + ], [ > + gnutls_init(&session, GNUTLS_CLIENT|GNUTLS_NO_SIGNAL); > + ]) > + ], [ > + AC_MSG_RESULT([yes]) > + AC_DEFINE([HAVE_GNUTLS_NO_SIGNAL], [1], > + [GNUTLS_NO_SIGNAL found at compile time]) > + ], [ > + AC_MSG_RESULT([no]) > + ]) > LIBS="$old_LIBS" > ]) > > diff --git a/interop/interop.c b/interop/interop.c > index b41f3ca887..036545bd82 100644 > --- a/interop/interop.c > +++ b/interop/interop.c > @@ -84,6 +84,16 @@ main (int argc, char *argv[]) > REQUIRES > #endif > > + /* Ignore SIGPIPE. We only need this for GnuTLS < 3.4.2, since > + * newer GnuTLS has the GNUTLS_NO_SIGNAL flag which adds > + * MSG_NOSIGNAL to each write call. > + */ > +#if !HAVE_GNUTLS_NO_SIGNAL > +#if TLS > + signal (SIGPIPE, SIG_IGN); > +#endif > +#endif > + > /* Create a large sparse temporary file. */ > #ifdef NEEDS_TMPFILE > int fd = mkstemp (TMPFILE); > diff --git a/lib/crypto.c b/lib/crypto.c > index ffc2b4ab5f..ffba4bda9b 100644 > --- a/lib/crypto.c > +++ b/lib/crypto.c > @@ -590,7 +590,12 @@ nbd_internal_crypto_create_session (struct nbd_handle *h, > gnutls_psk_client_credentials_t pskcreds = NULL; > gnutls_certificate_credentials_t xcreds = NULL; > > - err = gnutls_init (&session, GNUTLS_CLIENT|GNUTLS_NONBLOCK); > + err = gnutls_init (&session, > + GNUTLS_CLIENT | GNUTLS_NONBLOCK > +#if HAVE_GNUTLS_NO_SIGNAL > + | GNUTLS_NO_SIGNAL > +#endif > + ); > if (err < 0) { > set_error (errno, "gnutls_init: %s", gnutls_strerror (err)); > return NULL; > I've never seen a single example of SIGPIPE being useful to a program that checks for, and properly handles, write() / send() / sendto() / sendmsg() failures.

Even if we want to die with a SIGPIPE actually (so that the parent see it), we can recognize errno == EPIPE, then reset the disposition of SIGPIPE to SIG_DFLT in a controlled manner, and re-raise the signal. (But this is totally obscure; in most cases, a parent process is satisfied if the child exits with status 1 or so in case the child sees an EPIPE.) So, why not set the disposition of SIGPIPE to SIG_IGN indiscriminately?