From 5e6c2e1e2cf28287682022e57c25ea2b2ad7a7fe Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Thu, 29 Apr 2021 13:47:40 -0500 Subject: [PATCH 1/4] gtlsconnection: use a vfunc to implement get_negotiated_protocol() The current code is unsafe to use from multiple threads at once. GIOStream functions like this are supposed to be semi-threadsafe. It's allowed for them to be called on both a reader thread and a writer thread at the same time. Of course, it's still tricky and dangerous, because it's only *really* threadsafe if the handshake has finished, and API users have no plausible way to know that because the API does not require performing an explicit handshake operation. But that's a glib-networking problem. We can at least avoid the most obvious threadsafety issue here in the API layer. Note that we'll need to implement the new vfunc in glib-networking for this to actually work. Fixes #2393 --- gio/gtlsconnection.c | 46 ++++++-------------------------------------- gio/gtlsconnection.h | 4 +++- 2 files changed, 9 insertions(+), 41 deletions(-) diff --git a/gio/gtlsconnection.c b/gio/gtlsconnection.c index 3bd37ae97..0239489b7 100644 --- a/gio/gtlsconnection.c +++ b/gio/gtlsconnection.c @@ -55,12 +55,7 @@ * Since: 2.28 */ -struct _GTlsConnectionPrivate -{ - gchar *negotiated_protocol; -}; - -G_DEFINE_ABSTRACT_TYPE_WITH_PRIVATE (GTlsConnection, g_tls_connection, G_TYPE_IO_STREAM) +G_DEFINE_ABSTRACT_TYPE (GTlsConnection, g_tls_connection, G_TYPE_IO_STREAM) static void g_tls_connection_get_property (GObject *object, guint prop_id, @@ -70,7 +65,6 @@ static void g_tls_connection_set_property (GObject *object, guint prop_id, const GValue *value, GParamSpec *pspec); -static void g_tls_connection_finalize (GObject *object); enum { ACCEPT_CERTIFICATE, @@ -104,7 +98,6 @@ g_tls_connection_class_init (GTlsConnectionClass *klass) gobject_class->get_property = g_tls_connection_get_property; gobject_class->set_property = g_tls_connection_set_property; - gobject_class->finalize = g_tls_connection_finalize; /** * GTlsConnection:base-io-stream: @@ -413,17 +406,6 @@ g_tls_connection_set_property (GObject *object, G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); } -static void -g_tls_connection_finalize (GObject *object) -{ - GTlsConnection *conn = G_TLS_CONNECTION(object); - GTlsConnectionPrivate *priv = g_tls_connection_get_instance_private (conn); - - g_clear_pointer (&priv->negotiated_protocol, g_free); - - G_OBJECT_CLASS (g_tls_connection_parent_class)->finalize (object); -} - /** * g_tls_connection_set_use_system_certdb: * @conn: a #GTlsConnection @@ -871,31 +853,15 @@ g_tls_connection_set_advertised_protocols (GTlsConnection *conn, const gchar * g_tls_connection_get_negotiated_protocol (GTlsConnection *conn) { - GTlsConnectionPrivate *priv; - gchar *protocol; + GTlsConnectionClass *class; g_return_val_if_fail (G_IS_TLS_CONNECTION (conn), NULL); - g_object_get (G_OBJECT (conn), - "negotiated-protocol", &protocol, - NULL); + class = G_TLS_CONNECTION_GET_CLASS (conn); + if (class->get_negotiated_protocol == NULL) + return NULL; - /* - * Cache the property internally so we can return a `const` pointer - * to the caller. - */ - priv = g_tls_connection_get_instance_private (conn); - if (g_strcmp0 (priv->negotiated_protocol, protocol) != 0) - { - g_free (priv->negotiated_protocol); - priv->negotiated_protocol = protocol; - } - else - { - g_free (protocol); - } - - return priv->negotiated_protocol; + return class->get_negotiated_protocol (conn); } /** diff --git a/gio/gtlsconnection.h b/gio/gtlsconnection.h index a40a253a6..90a8db8db 100644 --- a/gio/gtlsconnection.h +++ b/gio/gtlsconnection.h @@ -73,9 +73,11 @@ G_GNUC_BEGIN_IGNORE_DEPRECATIONS GError **error); G_GNUC_END_IGNORE_DEPRECATIONS + const gchar *(*get_negotiated_protocol) (GTlsConnection *conn); + /*< private >*/ /* Padding for future expansion */ - gpointer padding[7]; + gpointer padding[6]; }; GLIB_AVAILABLE_IN_ALL From 09a4203b383533ffae6fee4e0e0938cedf655434 Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Fri, 30 Apr 2021 10:43:22 -0500 Subject: [PATCH 2/4] gtlsconnection: Add doc comment for GTlsConnectionClass --- gio/gtlsconnection.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/gio/gtlsconnection.h b/gio/gtlsconnection.h index 90a8db8db..f46600a81 100644 --- a/gio/gtlsconnection.h +++ b/gio/gtlsconnection.h @@ -43,6 +43,20 @@ struct _GTlsConnection { GTlsConnectionPrivate *priv; }; +/** + * GTlsConnectionClass: + * @parent_class: The parent class. + * @accept_certificate: Check whether to accept a certificate. + * @handshake: Perform a handshake operation. + * @handshake_async: Start an asynchronous handshake operation. + * @handshake_finish: Finish an asynchronous handshake operation. + * @get_binding_data: Retrieve TLS channel binding data + * @get_negotiated_protocol: Get ALPN-negotiated protocol + * + * The class structure for the #GTlsConnection type. + * + * Since: 2.28 + */ struct _GTlsConnectionClass { GIOStreamClass parent_class; From 1ea88f46ff2d9f84c077b7756ad64e9802d82c8a Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Fri, 30 Apr 2021 10:44:16 -0500 Subject: [PATCH 3/4] gdtlsconnection: document get_binding_data vfunc --- gio/gdtlsconnection.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gio/gdtlsconnection.h b/gio/gdtlsconnection.h index 45a16f50a..5ec247f28 100644 --- a/gio/gdtlsconnection.h +++ b/gio/gdtlsconnection.h @@ -46,7 +46,8 @@ typedef struct _GDtlsConnectionInterface GDtlsConnectionInterface; * @shutdown_async: Start an asynchronous shutdown operation. * @shutdown_finish: Finish an asynchronous shutdown operation. * @set_advertised_protocols: Set APLN protocol list - * @get_negotiated_protocol: Retrieve ALPN-negotiated protocol + * @get_negotiated_protocol: Get ALPN-negotiated protocol + * @get_binding_data: Retrieve TLS channel binding data * * Virtual method table for a #GDtlsConnection implementation. * From 2cd187aa5ef144fff6e8a49b360f3c91b68471a1 Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Mon, 21 Jun 2021 09:31:17 -0500 Subject: [PATCH 4/4] gio: add missing Since tags in GTlsConnection/GDtlsConnection --- gio/gdtlsconnection.h | 6 +++--- gio/gtlsconnection.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gio/gdtlsconnection.h b/gio/gdtlsconnection.h index 5ec247f28..8a5ca2723 100644 --- a/gio/gdtlsconnection.h +++ b/gio/gdtlsconnection.h @@ -45,9 +45,9 @@ typedef struct _GDtlsConnectionInterface GDtlsConnectionInterface; * @shutdown: Shut down one or both directions of the connection. * @shutdown_async: Start an asynchronous shutdown operation. * @shutdown_finish: Finish an asynchronous shutdown operation. - * @set_advertised_protocols: Set APLN protocol list - * @get_negotiated_protocol: Get ALPN-negotiated protocol - * @get_binding_data: Retrieve TLS channel binding data + * @set_advertised_protocols: Set APLN protocol list (Since: 2.60) + * @get_negotiated_protocol: Get ALPN-negotiated protocol (Since: 2.60) + * @get_binding_data: Retrieve TLS channel binding data (Since: 2.66) * * Virtual method table for a #GDtlsConnection implementation. * diff --git a/gio/gtlsconnection.h b/gio/gtlsconnection.h index f46600a81..526eb60b5 100644 --- a/gio/gtlsconnection.h +++ b/gio/gtlsconnection.h @@ -50,8 +50,8 @@ struct _GTlsConnection { * @handshake: Perform a handshake operation. * @handshake_async: Start an asynchronous handshake operation. * @handshake_finish: Finish an asynchronous handshake operation. - * @get_binding_data: Retrieve TLS channel binding data - * @get_negotiated_protocol: Get ALPN-negotiated protocol + * @get_binding_data: Retrieve TLS channel binding data (Since: 2.66) + * @get_negotiated_protocol: Get ALPN-negotiated protocol (Since: 2.70) * * The class structure for the #GTlsConnection type. *