[glib-networking/glib-2-62] Return bad identity error if identity is unset
- From: Michael Catanzaro <mcatanzaro src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [glib-networking/glib-2-62] Return bad identity error if identity is unset
- Date: Wed, 6 May 2020 14:53:03 +0000 (UTC)
commit 29513946809590c4912550f6f8620468f9836d94
Author: Michael Catanzaro <mcatanzaro gnome org>
Date: Mon May 4 17:47:28 2020 -0500
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.
Fixes #135
tls/base/gtlsconnection-base.c | 20 ++++++------
tls/tests/connection.c | 70 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 81 insertions(+), 9 deletions(-)
---
diff --git a/tls/base/gtlsconnection-base.c b/tls/base/gtlsconnection-base.c
index d1c4638..74f9786 100644
--- a/tls/base/gtlsconnection-base.c
+++ b/tls/base/gtlsconnection-base.c
@@ -1170,21 +1170,23 @@ verify_peer_certificate (GTlsConnectionBase *tls,
GTlsCertificate *peer_certificate)
{
GTlsConnectionBaseClass *tls_class = G_TLS_CONNECTION_BASE_GET_CLASS (tls);
- GSocketConnectable *peer_identity;
+ GSocketConnectable *peer_identity = NULL;
GTlsDatabase *database;
- GTlsCertificateFlags errors;
+ GTlsCertificateFlags errors = 0;
gboolean is_client;
is_client = G_IS_TLS_CLIENT_CONNECTION (tls);
- if (!is_client)
- peer_identity = NULL;
- else if (!g_tls_connection_base_is_dtls (tls))
- peer_identity = g_tls_client_connection_get_server_identity (G_TLS_CLIENT_CONNECTION (tls));
- else
- peer_identity = g_dtls_client_connection_get_server_identity (G_DTLS_CLIENT_CONNECTION (tls));
+ if (is_client)
+ {
+ if (!g_tls_connection_base_is_dtls (tls))
+ peer_identity = g_tls_client_connection_get_server_identity (G_TLS_CLIENT_CONNECTION (tls));
+ else
+ peer_identity = g_dtls_client_connection_get_server_identity (G_DTLS_CLIENT_CONNECTION (tls));
- errors = 0;
+ if (!peer_identity)
+ errors |= G_TLS_CERTIFICATE_BAD_IDENTITY;
+ }
database = g_tls_connection_get_database (G_TLS_CONNECTION (tls));
if (!database)
diff --git a/tls/tests/connection.c b/tls/tests/connection.c
index ff7e8cb..8df8ef2 100644
--- a/tls/tests/connection.c
+++ b/tls/tests/connection.c
@@ -2427,6 +2427,74 @@ test_socket_timeout (TestConnection *test,
g_assert_error (test->server_error, G_TLS_ERROR, G_TLS_ERROR_NOT_TLS);
}
+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[])
@@ -2515,6 +2583,8 @@ main (int argc,
setup_connection, test_sync_op_during_handshake, teardown_connection);
g_test_add ("/tls/" BACKEND "/connection/socket-timeout", TestConnection, NULL,
setup_connection, test_socket_timeout, teardown_connection);
+ g_test_add ("/tls/" BACKEND "/connection/missing-server-identity", TestConnection, NULL,
+ setup_connection, test_connection_missing_server_identity, teardown_connection);
ret = g_test_run ();
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]