[PATCH 4/4] Revert "crypto: implement workaround for GNUTLS thread safety problems"

["Daniel P. Berrangé" <berrange@redhat.com>]

This reverts commit 24ad5e19952b326796c8a3a1595c57ff180dab84.

The workaround for the GNUTLS bug was not correct and it was
decided to rely on GNUTLS being patched by distros instead,
or users manually disabling use of the problematic ciphers.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/tlssession.c         | 92 ++-----------------------------------
 include/crypto/tlssession.h | 14 ------
 meson.build                 |  9 ----
 3 files changed, 3 insertions(+), 112 deletions(-)

diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index 9450c83d9c3..0e9ca9fae0f 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -19,7 +19,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/thread.h"
 #include "crypto/tlssession.h"
 #include "crypto/tlscredsanon.h"
 #include "crypto/tlscredspsk.h"
@@ -53,14 +52,6 @@ struct QCryptoTLSSession {
      */
     Error *rerr;
     Error *werr;
-
-    /*
-     * Used to protect against broken GNUTLS thread safety
-     * https://gitlab.com/gnutls/gnutls/-/issues/1717
-     */
-    bool requireThreadSafety;
-    bool lockEnabled;
-    QemuMutex lock;
 };
 
 
@@ -80,7 +71,6 @@ qcrypto_tls_session_free(QCryptoTLSSession *session)
     g_free(session->authzid);
     qcrypto_tls_creds_box_unref(session->credsbox);
     object_unref(OBJECT(session->creds));
-    qemu_mutex_destroy(&session->lock);
     g_free(session);
 }
 
@@ -96,19 +86,10 @@ qcrypto_tls_session_push(void *opaque, const void *buf, size_t len)
         return -1;
     };
 
-    if (session->lockEnabled) {
-        qemu_mutex_unlock(&session->lock);
-    }
-
     error_free(session->werr);
     session->werr = NULL;
 
     ret = session->writeFunc(buf, len, session->opaque, &session->werr);
-
-    if (session->lockEnabled) {
-        qemu_mutex_lock(&session->lock);
-    }
-
     if (ret == QCRYPTO_TLS_SESSION_ERR_BLOCK) {
         errno = EAGAIN;
         return -1;
@@ -135,16 +116,7 @@ qcrypto_tls_session_pull(void *opaque, void *buf, size_t len)
     error_free(session->rerr);
     session->rerr = NULL;
 
-    if (session->lockEnabled) {
-        qemu_mutex_unlock(&session->lock);
-    }
-
     ret = session->readFunc(buf, len, session->opaque, &session->rerr);
-
-    if (session->lockEnabled) {
-        qemu_mutex_lock(&session->lock);
-    }
-
     if (ret == QCRYPTO_TLS_SESSION_ERR_BLOCK) {
         errno = EAGAIN;
         return -1;
@@ -181,8 +153,6 @@ qcrypto_tls_session_new(QCryptoTLSCreds *creds,
     session->creds = creds;
     object_ref(OBJECT(creds));
 
-    qemu_mutex_init(&session->lock);
-
     if (creds->endpoint != endpoint) {
         error_setg(errp, "Credentials endpoint doesn't match session");
         goto error;
@@ -247,11 +217,6 @@ qcrypto_tls_session_new(QCryptoTLSCreds *creds,
     return NULL;
 }
 
-void qcrypto_tls_session_require_thread_safety(QCryptoTLSSession *sess)
-{
-    sess->requireThreadSafety = true;
-}
-
 static int
 qcrypto_tls_session_check_certificate(QCryptoTLSSession *session,
                                       Error **errp)
@@ -438,17 +403,7 @@ qcrypto_tls_session_write(QCryptoTLSSession *session,
                           size_t len,
                           Error **errp)
 {
-    ssize_t ret;
-
-    if (session->lockEnabled) {
-        qemu_mutex_lock(&session->lock);
-    }
-
-    ret = gnutls_record_send(session->handle, buf, len);
-
-    if (session->lockEnabled) {
-        qemu_mutex_unlock(&session->lock);
-    }
+    ssize_t ret = gnutls_record_send(session->handle, buf, len);
 
     if (ret < 0) {
         if (ret == GNUTLS_E_AGAIN) {
@@ -476,17 +431,7 @@ qcrypto_tls_session_read(QCryptoTLSSession *session,
                          size_t len,
                          Error **errp)
 {
-    ssize_t ret;
-
-    if (session->lockEnabled) {
-        qemu_mutex_lock(&session->lock);
-    }
-
-    ret = gnutls_record_recv(session->handle, buf, len);
-
-    if (session->lockEnabled) {
-        qemu_mutex_unlock(&session->lock);
-    }
+    ssize_t ret = gnutls_record_recv(session->handle, buf, len);
 
     if (ret < 0) {
         if (ret == GNUTLS_E_AGAIN) {
@@ -556,29 +501,8 @@ int
 qcrypto_tls_session_handshake(QCryptoTLSSession *session,
                               Error **errp)
 {
-    int ret;
-    ret = gnutls_handshake(session->handle);
-
+    int ret = gnutls_handshake(session->handle);
     if (!ret) {
-#ifdef CONFIG_GNUTLS_BUG1717_WORKAROUND
-        gnutls_cipher_algorithm_t cipher =
-            gnutls_cipher_get(session->handle);
-
-        /*
-         * Any use of rekeying in TLS 1.3 is unsafe for
-         * a gnutls with bug 1717, however, we know that
-         * QEMU won't initiate manual rekeying. Thus we
-         * only have to protect against automatic rekeying
-         * which doesn't trigger with CHACHA20
-         */
-        if (session->requireThreadSafety &&
-            gnutls_protocol_get_version(session->handle) ==
-            GNUTLS_TLS1_3 &&
-            cipher != GNUTLS_CIPHER_CHACHA20_POLY1305) {
-            session->lockEnabled = true;
-        }
-#endif
-
         session->handshakeComplete = true;
         return QCRYPTO_TLS_HANDSHAKE_COMPLETE;
     }
@@ -616,15 +540,8 @@ qcrypto_tls_session_bye(QCryptoTLSSession *session, Error **errp)
         return 0;
     }
 
-    if (session->lockEnabled) {
-        qemu_mutex_lock(&session->lock);
-    }
     ret = gnutls_bye(session->handle, GNUTLS_SHUT_WR);
 
-    if (session->lockEnabled) {
-        qemu_mutex_unlock(&session->lock);
-    }
-
     if (!ret) {
         return QCRYPTO_TLS_BYE_COMPLETE;
     }
@@ -690,9 +607,6 @@ qcrypto_tls_session_new(QCryptoTLSCreds *creds G_GNUC_UNUSED,
     return NULL;
 }
 
-void qcrypto_tls_session_require_thread_safety(QCryptoTLSSession *sess)
-{
-}
 
 void
 qcrypto_tls_session_free(QCryptoTLSSession *sess G_GNUC_UNUSED)
diff --git a/include/crypto/tlssession.h b/include/crypto/tlssession.h
index 5415503ebab..274f4fd07ac 100644
--- a/include/crypto/tlssession.h
+++ b/include/crypto/tlssession.h
@@ -166,20 +166,6 @@ void qcrypto_tls_session_free(QCryptoTLSSession *sess);
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoTLSSession, qcrypto_tls_session_free)
 
-/**
- * qcrypto_tls_session_require_thread_safety:
- * @sess: the TLS session object
- *
- * Mark that this TLS session will require thread safety
- * for concurrent I/O in both directions. This must be
- * called before the handshake is performed.
- *
- * This will activate a workaround for GNUTLS thread
- * safety issues, where appropriate for the negotiated
- * TLS session parameters.
- */
-void qcrypto_tls_session_require_thread_safety(QCryptoTLSSession *sess);
-
 /**
  * qcrypto_tls_session_check_credentials:
  * @sess: the TLS session object
diff --git a/meson.build b/meson.build
index 48da6cf3e6b..b95fff9fafc 100644
--- a/meson.build
+++ b/meson.build
@@ -1795,18 +1795,11 @@ endif
 
 gnutls = not_found
 gnutls_crypto = not_found
-gnutls_bug1717_workaround = false
 if get_option('gnutls').enabled() or (get_option('gnutls').auto() and have_system)
   gnutls = dependency('gnutls', version: '>=3.7.5',
                       method: 'pkg-config',
                       required: get_option('gnutls'))
   gnutls_crypto = gnutls
-
-  #if gnutls.found() and not get_option('gnutls-bug1717-workaround').disabled()
-    # XXX: when bug 1717 is resolved, add logic to probe for
-    # the GNUTLS fixed version number to handle the 'auto' case
-  #  gnutls_bug1717_workaround = true
-  #endif
 endif
 
 # We prefer use of gnutls for crypto, unless the options
@@ -2533,7 +2526,6 @@ config_host_data.set('CONFIG_KEYUTILS', keyutils.found())
 config_host_data.set('CONFIG_GETTID', has_gettid)
 config_host_data.set('CONFIG_GNUTLS', gnutls.found())
 config_host_data.set('CONFIG_GNUTLS_CRYPTO', gnutls_crypto.found())
-config_host_data.set('CONFIG_GNUTLS_BUG1717_WORKAROUND', gnutls_bug1717_workaround)
 config_host_data.set('CONFIG_TASN1', tasn1.found())
 config_host_data.set('CONFIG_GCRYPT', gcrypt.found())
 config_host_data.set('CONFIG_NETTLE', nettle.found())
@@ -4911,7 +4903,6 @@ summary_info += {'TLS priority':      get_option('tls_priority')}
 summary_info += {'GNUTLS support':    gnutls}
 if gnutls.found()
   summary_info += {'  GNUTLS crypto':   gnutls_crypto.found()}
-  summary_info += {'  GNUTLS bug 1717 workaround': gnutls_bug1717_workaround }
 endif
 summary_info += {'libgcrypt':         gcrypt}
 summary_info += {'nettle':            nettle}
-- 
2.56.0