From db611ae74404cac1871aa7198617a53cb02a86c0 Mon Sep 17 00:00:00 2001 From: Tomas Popela Date: Jun 30 2017 06:14:28 +0000 Subject: Backport negotiate fixes --- diff --git a/0001-Rework-some-of-the-SoupAuthNegotiate-internals.patch b/0001-Rework-some-of-the-SoupAuthNegotiate-internals.patch new file mode 100644 index 0000000..42f2f17 --- /dev/null +++ b/0001-Rework-some-of-the-SoupAuthNegotiate-internals.patch @@ -0,0 +1,239 @@ +From 05a88c3a8e961e1394b9ccd269f03c7230004498 Mon Sep 17 00:00:00 2001 +From: Tomas Popela +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 + diff --git a/0002-Can-t-access-sites-that-request-closing-the-connecti.patch b/0002-Can-t-access-sites-that-request-closing-the-connecti.patch new file mode 100644 index 0000000..0c5a37a --- /dev/null +++ b/0002-Can-t-access-sites-that-request-closing-the-connecti.patch @@ -0,0 +1,72 @@ +From 1d532c8ea8b5c4a15f16894afcd604155c016ceb Mon Sep 17 00:00:00 2001 +From: Tomas Popela +Date: Wed, 14 Jun 2017 11:46:42 +0200 +Subject: [PATCH 2/3] Can't access sites that request closing the connection + during 401 + +When a 401 message is received, a new token is generated and saved in +the SoupNegotiateConnectionState's respose header. Later when the connection is +closed (as requested by the server), the state is destroyed together with +the response header. When a new request is being created and we are asked for +the connection authorization, the newly created connection state doesn't have it +set. At this point if the connection state is newly created, generate a new token +together with the response header that will be returned as the connection +authorization. + +Also modify how the warning from the soup_gss_build_response is printed +to differentiate if there was a failure during soup_gss_client_init or +soup_gss_client_step. +--- + libsoup/soup-auth-negotiate.c | 29 +++++++++++++++++++++++++++-- + 1 file changed, 27 insertions(+), 2 deletions(-) + +diff --git a/libsoup/soup-auth-negotiate.c b/libsoup/soup-auth-negotiate.c +index 78c56b83..811ee1c2 100644 +--- a/libsoup/soup-auth-negotiate.c ++++ b/libsoup/soup-auth-negotiate.c +@@ -188,7 +188,29 @@ soup_auth_negotiate_get_connection_authorization (SoupConnectionAuth *auth, + SoupNegotiateConnectionState *conn = state; + char *header = NULL; + +- if (conn->state == SOUP_NEGOTIATE_RECEIVED_CHALLENGE) { ++ if (conn->state == SOUP_NEGOTIATE_NEW) { ++ GError *err = NULL; ++ ++ if (!check_auth_trusted_uri (auth, msg)) { ++ conn->state = SOUP_NEGOTIATE_FAILED; ++ return NULL; ++ } ++ ++ if (!soup_gss_build_response (conn, SOUP_AUTH (auth), &err)) { ++ /* FIXME: report further upward via ++ * soup_message_get_error_message */ ++ if (conn->initialized) ++ g_warning ("gssapi step failed: %s", err->message); ++ else ++ g_warning ("gssapi init failed: %s", err->message); ++ conn->state = SOUP_NEGOTIATE_FAILED; ++ g_clear_error (&err); ++ ++ return NULL; ++ } ++ } ++ ++ if (conn->response_header) { + header = conn->response_header; + conn->response_header = NULL; + conn->state = SOUP_NEGOTIATE_SENT_RESPONSE; +@@ -251,7 +273,10 @@ soup_auth_negotiate_update_connection (SoupConnectionAuth *auth, SoupMessage *ms + } else { + /* FIXME: report further upward via + * soup_message_get_error_message */ +- g_warning ("gssapi step failed: %s", err->message); ++ if (conn->initialized) ++ g_warning ("gssapi step failed: %s", err->message); ++ else ++ g_warning ("gssapi init failed: %s", err->message); + success = FALSE; + } + } else if (!strncmp (header, "Negotiate ", 10)) { +-- +2.13.0 + diff --git a/0003-Authentication-should-success-in-some-cases-when-gss.patch b/0003-Authentication-should-success-in-some-cases-when-gss.patch new file mode 100644 index 0000000..d6b5a98 --- /dev/null +++ b/0003-Authentication-should-success-in-some-cases-when-gss.patch @@ -0,0 +1,59 @@ +From aefe8eac4f5b6a3df823224a38f3d20fb2308579 Mon Sep 17 00:00:00 2001 +From: Tomas Popela +Date: Mon, 19 Jun 2017 18:08:16 +0200 +Subject: [PATCH 3/3] Authentication should success in some cases when + gss_init_sec_context() returns error + +Unfortunately, so many programs (curl, Firefox) ignore the return token that is +included in the response, so it is possible that there are servers that send +back broken stuff. Try to behave in the right way (pass the token to +gss_init_sec_context()), show a warning, but don't fail if the server returned +200. + +There is an internal Red Hat site that triggers the described situation +and the "Invalid token was supplied: Unknown error" is being printed to +the console. +--- + libsoup/soup-auth-negotiate.c | 23 +++++++++++++++++++---- + 1 file changed, 19 insertions(+), 4 deletions(-) + +diff --git a/libsoup/soup-auth-negotiate.c b/libsoup/soup-auth-negotiate.c +index 811ee1c2..5a49119b 100644 +--- a/libsoup/soup-auth-negotiate.c ++++ b/libsoup/soup-auth-negotiate.c +@@ -362,13 +362,28 @@ check_server_response (SoupMessage *msg, gpointer auth) + + ret = soup_gss_client_step (conn, auth_headers + 10, &err); + +- priv->is_authenticated = ret == AUTH_GSS_COMPLETE; +- +- if (ret == AUTH_GSS_CONTINUE) { ++ switch (ret) { ++ case AUTH_GSS_COMPLETE: ++ priv->is_authenticated = TRUE; ++ break; ++ case AUTH_GSS_CONTINUE: + conn->state = SOUP_NEGOTIATE_RECEIVED_CHALLENGE; +- } else if (ret == AUTH_GSS_ERROR) { ++ break; ++ case AUTH_GSS_ERROR: + if (err) + g_warning ("%s", err->message); ++ /* Unfortunately, so many programs (curl, Firefox, ..) ignore ++ * the return token that is included in the response, so it is ++ * possible that there are servers that send back broken stuff. ++ * Try to behave in the right way (pass the token to ++ * gss_init_sec_context()), show a warning, but don't fail ++ * if the server returned 200. */ ++ if (msg->status_code == SOUP_STATUS_OK) ++ priv->is_authenticated = TRUE; ++ else ++ conn->state = SOUP_NEGOTIATE_FAILED; ++ break; ++ default: + conn->state = SOUP_NEGOTIATE_FAILED; + } + out: +-- +2.13.0 + diff --git a/libsoup.spec b/libsoup.spec index 4148ede..e25a3e9 100644 --- a/libsoup.spec +++ b/libsoup.spec @@ -2,13 +2,20 @@ Name: libsoup Version: 2.58.1 -Release: 1%{?dist} +Release: 2%{?dist} Summary: Soup, an HTTP library implementation License: LGPLv2 URL: https://wiki.gnome.org/Projects/libsoup Source0: https://download.gnome.org/sources/%{name}/2.58/%{name}-%{version}.tar.xz +# https://bugzilla.gnome.org/show_bug.cgi?id=782940 +Patch01: 0001-Rework-some-of-the-SoupAuthNegotiate-internals.patch +# https://bugzilla.gnome.org/show_bug.cgi?id=783780 +Patch02: 0002-Can-t-access-sites-that-request-closing-the-connecti.patch +# https://bugzilla.gnome.org/show_bug.cgi?id=783781 +Patch03: 0003-Authentication-should-success-in-some-cases-when-gss.patch + BuildRequires: glib2-devel >= %{glib2_version} BuildRequires: glib-networking BuildRequires: intltool @@ -42,6 +49,9 @@ you to develop applications that use the libsoup library. %prep %setup -q +%patch01 -p1 -b .negotiate-internals +%patch02 -p1 -b .negotiate-connection-close +%patch03 -p1 -b .tcms-site-warning %build %configure --disable-static @@ -81,6 +91,9 @@ rm -f $RPM_BUILD_ROOT/%{_libdir}/*.la %{_datadir}/vala/vapi/libsoup-2.4.vapi %changelog +* Fri Jun 30 2017 Tomas Popela - 2.58.1-2 +- Backport negotiate fixes + * Tue May 09 2017 Kalev Lember - 2.58.1-1 - Update to 2.58.1