[glib-networking/mcatanzaro/client-auth-request-fail: 2/2] gnutls: don't attempt to connect to server if TLS interaction fails

[Michael Catanzaro <mcatanzaro src gnome org>]

commit e4e65c07e0e3e5cf78e6c0fb08fc2d4e2da5ca6c
Author: Michael Catanzaro <mcatanzaro igalia com>
Date:   Fri May 17 11:24:26 2019 -0500

    gnutls: don't attempt to connect to server if TLS interaction fails
    
    If we fail to get a client certificate from the TLS interaction, we can
    either go ahead and try to connect to the server or not. Originally, we
    didn't. At some point in the past, I arbitrarily changed this to allow
    trying to connect. The theory is that some servers might request a
    client certificate, but allow connecting anyway even if it's not
    provided, and trying to connect would allow for compatibility with such
    servers.
    
    However, I've never heard of any such server existing. So, for
    simplicity, let's just fail if the TLS interaction has failed. This is
    what the code originally did, anyway, and nobody ever requested a
    change. We can always revisit in the future in the very unlikely event
    somebody wants to connect to such a server without a client certificate.
    
    Now, this causes a somewhat undesirable change in our test output. The
    test server is no longer able to fail on G_TLS_ERROR_CERTIFICATE_REQUIRED,
    because the server is no longer choosing to terminate the connection:
    it's the client choosing to terminate the connection now. So instead the
    error will be a generic handshake error. And we use
    G_TLS_ERROR_NOT_TLS for all handshake errors (see: #63).

 tls/gnutls/gtlsclientconnection-gnutls.c | 20 ++++----------------
 tls/tests/connection.c                   |  4 ++--
 2 files changed, 6 insertions(+), 18 deletions(-)
---
diff --git a/tls/gnutls/gtlsclientconnection-gnutls.c b/tls/gnutls/gtlsclientconnection-gnutls.c
index 12ea5f0..51dfa05 100644
--- a/tls/gnutls/gtlsclientconnection-gnutls.c
+++ b/tls/gnutls/gtlsclientconnection-gnutls.c
@@ -356,26 +356,14 @@ g_tls_client_connection_gnutls_retrieve_function (gnutls_session_t
 
       if (g_tls_connection_base_request_certificate (tls, g_tls_connection_base_get_certificate_error (tls)))
         g_tls_connection_gnutls_get_certificate (conn, pcert, pcert_length, pkey);
-
-      if (*pcert_length == 0)
-        {
-          g_tls_certificate_gnutls_copy_free (*pcert, *pcert_length, *pkey);
-
-          /* If there is still no client certificate, this connection will
-           * probably fail, but no reason to give up: let's try anyway.
-           */
-          g_tls_connection_base_set_missing_requested_client_certificate (tls);
-          return 0;
-        }
     }
 
-  if (*pkey == NULL)
+  /* If no client certificate was provided, or the provided certificate does not
+   * have a private key, then it's time to fail.
+   */
+  if (*pcert_length == 0 || *pkey == NULL)
     {
       g_tls_certificate_gnutls_copy_free (*pcert, *pcert_length, *pkey);
-
-      /* No private key. GnuTLS expects it to be non-null if pcert_length is
-       * nonzero, so we have to abort now.
-       */
       g_tls_connection_base_set_missing_requested_client_certificate (tls);
       return -1;
     }
diff --git a/tls/tests/connection.c b/tls/tests/connection.c
index 9c9d7f3..0d62d28 100644
--- a/tls/tests/connection.c
+++ b/tls/tests/connection.c
@@ -1073,7 +1073,7 @@ test_client_auth_failure (TestConnection *test,
   wait_until_server_finished (test);
 
   g_assert_error (test->read_error, G_TLS_ERROR, G_TLS_ERROR_CERTIFICATE_REQUIRED);
-  g_assert_error (test->server_error, G_TLS_ERROR, G_TLS_ERROR_CERTIFICATE_REQUIRED);
+  g_assert_error (test->server_error, G_TLS_ERROR, G_TLS_ERROR_NOT_TLS);
 
   g_assert_true (accepted_changed);
 
@@ -1263,7 +1263,7 @@ test_client_auth_request_fail (TestConnection *test,
    * when the GTlsInteraction's certificate request fails.
    */
   g_assert_error (test->read_error, G_FILE_ERROR, G_FILE_ERROR_ACCES);
-  g_assert_error (test->server_error, G_TLS_ERROR, G_TLS_ERROR_CERTIFICATE_REQUIRED);
+  g_assert_error (test->server_error, G_TLS_ERROR, G_TLS_ERROR_NOT_TLS);
 
   g_io_stream_close (test->server_connection, NULL, NULL);
   g_io_stream_close (test->client_connection, NULL, NULL);