164 строки
6.3 KiB
Diff
164 строки
6.3 KiB
Diff
From 926d242e20526db7a8793991450e643a0945435f Mon Sep 17 00:00:00 2001
|
|
From: Henry Beberman <henry.beberman@microsoft.com>
|
|
Date: Tue, 18 Aug 2020 12:26:55 -0700
|
|
Subject: [PATCH] Backport CVE-2020-13645 fix
|
|
|
|
This is a backport of the fix for CVE-2020-13645
|
|
From 29513946809590c4912550f6f8620468f9836d94 Mon Sep 17 00:00:00 2001
|
|
From: Michael Catanzaro <mcatanzaro@gnome.org>
|
|
Date: Mon, 4 May 2020 17:47:28 -0500
|
|
Subject: [PATCH] Return bad identity error if identity is unset
|
|
|
|
When the server-identity property of GTlsClientConnection is unset, the
|
|
documentation sasy we need to fail the certificate verification with
|
|
G_TLS_CERTIFICATE_BAD_IDENTITY. This is important because otherwise,
|
|
it's easy for applications to fail to specify server identity.
|
|
|
|
Unfortunately, we did not correctly implement the intended, documented
|
|
behavior. When server identity is missing, we check the validity of the
|
|
TLS certificate, but do not check if it corresponds to the expected
|
|
server (since we have no expected server). Then we assume the identity
|
|
is good, instead of returning bad identity, as documented. This means,
|
|
for example, that evil.com can present a valid certificate issued to
|
|
evil.com, and we would happily accept it for paypal.com
|
|
---
|
|
tls/gnutls/gtlsconnection-gnutls.c | 20 ++++----
|
|
tls/tests/connection.c | 77 ++++++++++++++++++++++++++++++
|
|
2 files changed, 88 insertions(+), 9 deletions(-)
|
|
|
|
diff --git a/tls/gnutls/gtlsconnection-gnutls.c b/tls/gnutls/gtlsconnection-gnutls.c
|
|
index 3200079..56be51d 100644
|
|
--- a/tls/gnutls/gtlsconnection-gnutls.c
|
|
+++ b/tls/gnutls/gtlsconnection-gnutls.c
|
|
@@ -1741,21 +1741,23 @@ verify_peer_certificate (GTlsConnectionGnutls *gnutls,
|
|
GTlsCertificate *peer_certificate)
|
|
{
|
|
GTlsConnection *conn = G_TLS_CONNECTION (gnutls);
|
|
- GSocketConnectable *peer_identity;
|
|
+ GSocketConnectable *peer_identity = NULL;
|
|
GTlsDatabase *database;
|
|
- GTlsCertificateFlags errors;
|
|
+ GTlsCertificateFlags errors = 0;
|
|
gboolean is_client;
|
|
|
|
is_client = G_IS_TLS_CLIENT_CONNECTION (gnutls);
|
|
|
|
- if (!is_client)
|
|
- peer_identity = NULL;
|
|
- else if (!g_tls_connection_gnutls_is_dtls (gnutls))
|
|
- peer_identity = g_tls_client_connection_get_server_identity (G_TLS_CLIENT_CONNECTION (gnutls));
|
|
- else
|
|
- peer_identity = g_dtls_client_connection_get_server_identity (G_DTLS_CLIENT_CONNECTION (gnutls));
|
|
+ if (is_client)
|
|
+ {
|
|
+ if (!g_tls_connection_gnutls_is_dtls (gnutls))
|
|
+ peer_identity = g_tls_client_connection_get_server_identity (G_TLS_CLIENT_CONNECTION (gnutls));
|
|
+ else
|
|
+ peer_identity = g_dtls_client_connection_get_server_identity (G_DTLS_CLIENT_CONNECTION (gnutls));
|
|
|
|
- errors = 0;
|
|
+ if (!peer_identity)
|
|
+ errors |= G_TLS_CERTIFICATE_BAD_IDENTITY;
|
|
+ }
|
|
|
|
database = g_tls_connection_get_database (conn);
|
|
if (database == NULL)
|
|
diff --git a/tls/tests/connection.c b/tls/tests/connection.c
|
|
index fe0f124..679dce5 100644
|
|
--- a/tls/tests/connection.c
|
|
+++ b/tls/tests/connection.c
|
|
@@ -2066,6 +2066,81 @@ test_readwrite_after_connection_destroyed (TestConnection *test,
|
|
g_object_unref (ostream);
|
|
}
|
|
|
|
+static void
|
|
+wait_until_server_finished (TestConnection *test)
|
|
+{
|
|
+ WAIT_UNTIL_UNSET (test->server_running);
|
|
+}
|
|
+
|
|
+static void
|
|
+test_connection_missing_server_identity (TestConnection *test,
|
|
+ gconstpointer data)
|
|
+{
|
|
+ GIOStream *connection;
|
|
+ GError *error = NULL;
|
|
+
|
|
+ test->database = g_tls_file_database_new (tls_test_file_path ("ca-roots.pem"), &error);
|
|
+ g_assert_no_error (error);
|
|
+ g_assert_nonnull (test->database);
|
|
+
|
|
+ /* We pass NULL instead of test->identity when creating the client
|
|
+ * connection. This means verification must fail with
|
|
+ * G_TLS_CERTIFICATE_BAD_IDENTITY.
|
|
+ */
|
|
+ connection = start_async_server_and_connect_to_it (test, G_TLS_AUTHENTICATION_NONE);
|
|
+ test->client_connection = g_tls_client_connection_new (connection, NULL, &error);
|
|
+ g_assert_no_error (error);
|
|
+ g_assert_nonnull (test->client_connection);
|
|
+ g_object_unref (connection);
|
|
+
|
|
+ g_tls_connection_set_database (G_TLS_CONNECTION (test->client_connection), test->database);
|
|
+
|
|
+ /* All validation in this test */
|
|
+ g_tls_client_connection_set_validation_flags (G_TLS_CLIENT_CONNECTION (test->client_connection),
|
|
+ G_TLS_CERTIFICATE_VALIDATE_ALL);
|
|
+
|
|
+ read_test_data_async (test);
|
|
+ g_main_loop_run (test->loop);
|
|
+ wait_until_server_finished (test);
|
|
+
|
|
+ g_assert_error (test->read_error, G_TLS_ERROR, G_TLS_ERROR_BAD_CERTIFICATE);
|
|
+
|
|
+#ifdef BACKEND_IS_GNUTLS
|
|
+ g_assert_error (test->server_error, G_TLS_ERROR, G_TLS_ERROR_NOT_TLS);
|
|
+#elif defined(BACKEND_IS_OPENSSL)
|
|
+ /* FIXME: This is not OK. There should be a NOT_TLS errors. But some times
|
|
+ * we either get no error or BROKEN_PIPE
|
|
+ */
|
|
+#endif
|
|
+
|
|
+ g_clear_error (&test->read_error);
|
|
+ g_clear_error (&test->server_error);
|
|
+
|
|
+ g_clear_object (&test->client_connection);
|
|
+ g_clear_object (&test->server_connection);
|
|
+
|
|
+ /* Now do the same thing again, this time ignoring bad identity. */
|
|
+
|
|
+ connection = start_async_server_and_connect_to_it (test, G_TLS_AUTHENTICATION_NONE);
|
|
+ test->client_connection = g_tls_client_connection_new (connection, NULL, &error);
|
|
+ g_assert_no_error (error);
|
|
+ g_assert_nonnull (test->client_connection);
|
|
+ g_object_unref (connection);
|
|
+
|
|
+ g_tls_connection_set_database (G_TLS_CONNECTION (test->client_connection), test->database);
|
|
+
|
|
+ g_tls_client_connection_set_validation_flags (G_TLS_CLIENT_CONNECTION (test->client_connection),
|
|
+ G_TLS_CERTIFICATE_VALIDATE_ALL & ~G_TLS_CERTIFICATE_BAD_IDENTITY);
|
|
+
|
|
+ read_test_data_async (test);
|
|
+ g_main_loop_run (test->loop);
|
|
+ wait_until_server_finished (test);
|
|
+
|
|
+ g_assert_no_error (test->read_error);
|
|
+ g_assert_no_error (test->server_error);
|
|
+}
|
|
+
|
|
+
|
|
int
|
|
main (int argc,
|
|
char *argv[])
|
|
@@ -2137,6 +2212,8 @@ main (int argc,
|
|
setup_connection, test_garbage_database, teardown_connection);
|
|
g_test_add ("/tls/connection/readwrite-after-connection-destroyed", TestConnection, NULL,
|
|
setup_connection, test_readwrite_after_connection_destroyed, teardown_connection);
|
|
+ g_test_add ("/tls/connection/missing-server-identity", TestConnection, NULL,
|
|
+ setup_connection, test_connection_missing_server_identity, teardown_connection);
|
|
|
|
ret = g_test_run ();
|
|
|
|
--
|
|
2.25.1
|
|
|