glib-networking fix CVE-2020-13645 (#66)

This commit is contained in:
Henry Beberman 2020-08-31 12:52:31 -07:00 committed by GitHub
parent 02028eb5ad
commit 3b451070c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 171 additions and 3 deletions

View File

@ -0,0 +1,163 @@
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

View File

@ -1,7 +1,7 @@
Summary: Glib networking modules
Name: glib-networking
Version: 2.59.1
Release: 5%{?dist}
Release: 6%{?dist}
License: GPLv2+ with exceptions
URL: https://gitlab.gnome.org/GNOME/glib-networking/
Group: System Environment/Development
@ -9,6 +9,8 @@ Vendor: Microsoft Corporation
Distribution: Mariner
Source0: http://ftp.gnome.org/pub/GNOME/sources/glib-networking/2.59/%{name}-%{version}.tar.xz
Patch0: CVE-2020-13645.patch
BuildRequires: nettle-devel
BuildRequires: autogen-libopts-devel
BuildRequires: libtasn1-devel
@ -38,6 +40,7 @@ These are the additional language files of glib-networking.
%prep
%setup -q
%patch0 -p1
%build
mkdir build &&
@ -69,8 +72,10 @@ ninja test
%defattr(-,root,root)
%changelog
* Sat May 09 00:20:40 PST 2020 Nick Samson <nisamson@microsoft.com> - 2.59.1-5
- Added %%license line automatically, updated license line
* Tue Aug 18 2020 Henry Beberman <hebeberm@microsoft.com> - 2.59.1-6
- Backport patch for CVE-2020-13645
* Sat May 09 00:20:40 PST 2020 Nick Samson <nisamson@microsoft.com> - 2.59.1-5
- Added %%license line automatically, updated license line
* Wed May 06 2020 Pawel Winogrodzki <pawelwi@microsoft.com> 2.59.1-4
- Removing *Requires for "ca-certificates".
* Thu Apr 23 2020 Andrew Phelps <anphel@microsoft.com> 2.59.1-3