240 lines
9.0 KiB
Diff
240 lines
9.0 KiB
Diff
From 05a88c3a8e961e1394b9ccd269f03c7230004498 Mon Sep 17 00:00:00 2001
|
|
From: Tomas Popela <tpopela@redhat.com>
|
|
Date: Tue, 20 Jun 2017 13:03:59 +0200
|
|
Subject: [PATCH 1/3] Rework some of the SoupAuthNegotiate internals
|
|
|
|
There are several problems with the current state. The main problem is
|
|
that the SoupMessage can outlive the SoupAuthNegotiate object and if the
|
|
SoupMessage signals handlers are active then they could be called with invalid
|
|
SoupAuthNegotiate object. To avoid that use the g_signal_connect_data() and
|
|
increase the reference on the SoupAuthNegotiate object. Also rework how
|
|
we are connecting the 'got_headers' signal handler to the SoupMessage object, so
|
|
they are really connected only once, even if the GSS mechanism involves
|
|
multiple rounds.
|
|
|
|
The whole concept of how we are working with the
|
|
SoupAuthNegotiateConnectionState is also wrong. When the connection state is
|
|
created it's saved to the private structure and then accessed from
|
|
there. The problem is that another state for different message could be
|
|
created in the mean time and that one would overwrite the currently set (or if
|
|
one would be freed then it would erase the one that is currently set). To solve
|
|
this expose the SoupConnectionAuth's get_connection_state_for_message() and
|
|
call it when we need the connection state object.
|
|
---
|
|
libsoup/soup-auth-negotiate.c | 56 +++++++++++-------------------------------
|
|
libsoup/soup-connection-auth.c | 29 ++++++++++++++++++----
|
|
libsoup/soup-connection-auth.h | 4 +++
|
|
3 files changed, 43 insertions(+), 46 deletions(-)
|
|
|
|
diff --git a/libsoup/soup-auth-negotiate.c b/libsoup/soup-auth-negotiate.c
|
|
index 94863d68..78c56b83 100644
|
|
--- a/libsoup/soup-auth-negotiate.c
|
|
+++ b/libsoup/soup-auth-negotiate.c
|
|
@@ -68,11 +68,6 @@ typedef struct {
|
|
|
|
typedef struct {
|
|
gboolean is_authenticated;
|
|
-
|
|
- gulong message_finished_signal_id;
|
|
- gulong message_got_headers_signal_id;
|
|
-
|
|
- SoupNegotiateConnectionState *conn_state;
|
|
} SoupAuthNegotiatePrivate;
|
|
|
|
/**
|
|
@@ -108,7 +103,6 @@ static GSList *blacklisted_uris;
|
|
static void parse_uris_from_env_variable (const gchar *env_variable, GSList **list);
|
|
|
|
static void check_server_response (SoupMessage *msg, gpointer auth);
|
|
-static void remove_server_response_handler (SoupMessage *msg, gpointer auth);
|
|
|
|
static const char spnego_OID[] = "\x2b\x06\x01\x05\x05\x02";
|
|
static const gss_OID_desc gss_mech_spnego = { sizeof (spnego_OID) - 1, (void *) &spnego_OID };
|
|
@@ -116,12 +110,10 @@ static const gss_OID_desc gss_mech_spnego = { sizeof (spnego_OID) - 1, (void *)
|
|
static gpointer
|
|
soup_auth_negotiate_create_connection_state (SoupConnectionAuth *auth)
|
|
{
|
|
- SoupAuthNegotiatePrivate *priv = soup_auth_negotiate_get_instance_private (SOUP_AUTH_NEGOTIATE (auth));
|
|
SoupNegotiateConnectionState *conn;
|
|
|
|
conn = g_slice_new0 (SoupNegotiateConnectionState);
|
|
conn->state = SOUP_NEGOTIATE_NEW;
|
|
- priv->conn_state = conn;
|
|
|
|
return conn;
|
|
}
|
|
@@ -137,14 +129,11 @@ static void
|
|
soup_auth_negotiate_free_connection_state (SoupConnectionAuth *auth,
|
|
gpointer state)
|
|
{
|
|
- SoupAuthNegotiate *negotiate = SOUP_AUTH_NEGOTIATE (auth);
|
|
- SoupAuthNegotiatePrivate *priv = soup_auth_negotiate_get_instance_private (negotiate);
|
|
SoupNegotiateConnectionState *conn = state;
|
|
|
|
free_connection_state_data (conn);
|
|
|
|
g_slice_free (SoupNegotiateConnectionState, conn);
|
|
- priv->conn_state = NULL;
|
|
}
|
|
|
|
static GSList *
|
|
@@ -226,7 +215,6 @@ soup_auth_negotiate_update_connection (SoupConnectionAuth *auth, SoupMessage *ms
|
|
#ifdef LIBSOUP_HAVE_GSSAPI
|
|
gboolean success = TRUE;
|
|
SoupNegotiateConnectionState *conn = state;
|
|
- SoupAuthNegotiatePrivate *priv = soup_auth_negotiate_get_instance_private (SOUP_AUTH_NEGOTIATE (auth));
|
|
GError *err = NULL;
|
|
|
|
if (!check_auth_trusted_uri (auth, msg)) {
|
|
@@ -245,24 +233,19 @@ soup_auth_negotiate_update_connection (SoupConnectionAuth *auth, SoupMessage *ms
|
|
|
|
conn->state = SOUP_NEGOTIATE_RECEIVED_CHALLENGE;
|
|
if (soup_gss_build_response (conn, SOUP_AUTH (auth), &err)) {
|
|
- /* Register the callbacks just once */
|
|
- if (priv->message_finished_signal_id == 0) {
|
|
- gulong id = 0;
|
|
- id = g_signal_connect (msg,
|
|
- "finished",
|
|
- G_CALLBACK (remove_server_response_handler),
|
|
- auth);
|
|
- priv->message_finished_signal_id = id;
|
|
- }
|
|
-
|
|
- if (priv->message_got_headers_signal_id == 0) {
|
|
- gulong id = 0;
|
|
+ /* Connect the signal only once per message */
|
|
+ if (!g_object_get_data (G_OBJECT (msg), "negotiate-got-headers-connected")) {
|
|
/* Wait for the 2xx response to verify server response */
|
|
- id = g_signal_connect (msg,
|
|
+ g_signal_connect_data (msg,
|
|
"got_headers",
|
|
G_CALLBACK (check_server_response),
|
|
- auth);
|
|
- priv->message_got_headers_signal_id = id;
|
|
+ g_object_ref (auth),
|
|
+ (GClosureNotify) g_object_unref,
|
|
+ 0);
|
|
+ /* Mark that the signal was connected */
|
|
+ g_object_set_data (G_OBJECT (msg),
|
|
+ "negotiate-got-headers-connected",
|
|
+ GINT_TO_POINTER (1));
|
|
}
|
|
goto out;
|
|
} else {
|
|
@@ -331,7 +314,11 @@ check_server_response (SoupMessage *msg, gpointer auth)
|
|
GError *err = NULL;
|
|
SoupAuthNegotiate *negotiate = auth;
|
|
SoupAuthNegotiatePrivate *priv = soup_auth_negotiate_get_instance_private (negotiate);
|
|
- SoupNegotiateConnectionState *conn = priv->conn_state;
|
|
+ SoupNegotiateConnectionState *conn;
|
|
+
|
|
+ conn = soup_connection_auth_get_connection_state_for_message (SOUP_CONNECTION_AUTH (auth), msg);
|
|
+ if (!conn)
|
|
+ return;
|
|
|
|
if (auth != soup_message_get_auth (msg))
|
|
return;
|
|
@@ -363,19 +350,6 @@ check_server_response (SoupMessage *msg, gpointer auth)
|
|
g_clear_error (&err);
|
|
}
|
|
|
|
-static void
|
|
-remove_server_response_handler (SoupMessage *msg, gpointer auth)
|
|
-{
|
|
- SoupAuthNegotiate *negotiate = auth;
|
|
- SoupAuthNegotiatePrivate *priv = soup_auth_negotiate_get_instance_private (negotiate);
|
|
-
|
|
- g_signal_handler_disconnect (msg, priv->message_got_headers_signal_id);
|
|
- priv->message_got_headers_signal_id = 0;
|
|
-
|
|
- g_signal_handler_disconnect (msg, priv->message_finished_signal_id);
|
|
- priv->message_finished_signal_id = 0;
|
|
-}
|
|
-
|
|
/* Check if scheme://host:port from message matches the given URI. */
|
|
static gint
|
|
match_base_uri (SoupURI *list_uri, SoupURI *msg_uri)
|
|
diff --git a/libsoup/soup-connection-auth.c b/libsoup/soup-connection-auth.c
|
|
index 8362095e..f55cfe6b 100644
|
|
--- a/libsoup/soup-connection-auth.c
|
|
+++ b/libsoup/soup-connection-auth.c
|
|
@@ -71,12 +71,31 @@ soup_connection_auth_finalize (GObject *object)
|
|
G_OBJECT_CLASS (soup_connection_auth_parent_class)->finalize (object);
|
|
}
|
|
|
|
-static gpointer
|
|
-get_connection_state_for_message (SoupConnectionAuth *auth, SoupMessage *msg)
|
|
+
|
|
+/**
|
|
+ * soup_connection_auth_get_connection_state_for_message:
|
|
+ * @auth: a #SoupConnectionAuth
|
|
+ * @msg: a #SoupMessage
|
|
+ *
|
|
+ * Returns an associated connection state object for the given @auth and @msg.
|
|
+ *
|
|
+ * This function is only useful from within implementations of SoupConnectionAuth
|
|
+ * subclasses.
|
|
+ *
|
|
+ * Return value: (transfer none): the connection state
|
|
+ *
|
|
+ * Since: 2.58
|
|
+ **/
|
|
+gpointer
|
|
+soup_connection_auth_get_connection_state_for_message (SoupConnectionAuth *auth,
|
|
+ SoupMessage *msg)
|
|
{
|
|
SoupConnection *conn;
|
|
gpointer state;
|
|
|
|
+ g_return_val_if_fail (SOUP_IS_CONNECTION_AUTH (auth), NULL);
|
|
+ g_return_val_if_fail (SOUP_IS_MESSAGE (msg), NULL);
|
|
+
|
|
conn = soup_message_get_connection (msg);
|
|
state = g_hash_table_lookup (auth->priv->conns, conn);
|
|
if (state)
|
|
@@ -98,7 +117,7 @@ soup_connection_auth_update (SoupAuth *auth,
|
|
GHashTable *auth_params)
|
|
{
|
|
SoupConnectionAuth *cauth = SOUP_CONNECTION_AUTH (auth);
|
|
- gpointer conn = get_connection_state_for_message (cauth, msg);
|
|
+ gpointer conn = soup_connection_auth_get_connection_state_for_message (cauth, msg);
|
|
GHashTableIter iter;
|
|
GString *auth_header;
|
|
gpointer key, value;
|
|
@@ -140,7 +159,7 @@ soup_connection_auth_get_authorization (SoupAuth *auth,
|
|
SoupMessage *msg)
|
|
{
|
|
SoupConnectionAuth *cauth = SOUP_CONNECTION_AUTH (auth);
|
|
- gpointer conn = get_connection_state_for_message (cauth, msg);
|
|
+ gpointer conn = soup_connection_auth_get_connection_state_for_message (cauth, msg);
|
|
|
|
return SOUP_CONNECTION_AUTH_GET_CLASS (auth)->
|
|
get_connection_authorization (cauth, msg, conn);
|
|
@@ -151,7 +170,7 @@ soup_connection_auth_is_ready (SoupAuth *auth,
|
|
SoupMessage *msg)
|
|
{
|
|
SoupConnectionAuth *cauth = SOUP_CONNECTION_AUTH (auth);
|
|
- gpointer conn = get_connection_state_for_message (cauth, msg);
|
|
+ gpointer conn = soup_connection_auth_get_connection_state_for_message (cauth, msg);
|
|
|
|
return SOUP_CONNECTION_AUTH_GET_CLASS (auth)->
|
|
is_connection_ready (SOUP_CONNECTION_AUTH (auth), msg, conn);
|
|
diff --git a/libsoup/soup-connection-auth.h b/libsoup/soup-connection-auth.h
|
|
index f225f0f6..c5fbe7bd 100644
|
|
--- a/libsoup/soup-connection-auth.h
|
|
+++ b/libsoup/soup-connection-auth.h
|
|
@@ -46,6 +46,10 @@ typedef struct {
|
|
|
|
GType soup_connection_auth_get_type (void);
|
|
|
|
+SOUP_AVAILABLE_IN_2_58
|
|
+gpointer soup_connection_auth_get_connection_state_for_message
|
|
+ (SoupConnectionAuth *auth,
|
|
+ SoupMessage *message);
|
|
G_END_DECLS
|
|
|
|
#endif /* SOUP_CONNECTION_AUTH_H */
|
|
--
|
|
2.13.0
|
|
|