Backport negotiate fixes

This commit is contained in:
Tomas Popela 2017-06-30 08:14:28 +02:00
parent 1eb47d2e6b
commit ea0bdb4ce9
4 changed files with 384 additions and 1 deletions

View File

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

View File

@ -0,0 +1,72 @@
From 1d532c8ea8b5c4a15f16894afcd604155c016ceb Mon Sep 17 00:00:00 2001
From: Tomas Popela <tpopela@redhat.com>
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

View File

@ -0,0 +1,59 @@
From aefe8eac4f5b6a3df823224a38f3d20fb2308579 Mon Sep 17 00:00:00 2001
From: Tomas Popela <tpopela@redhat.com>
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

View File

@ -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 <tpopela@redhat.com> - 2.58.1-2
- Backport negotiate fixes
* Tue May 09 2017 Kalev Lember <klember@redhat.com> - 2.58.1-1
- Update to 2.58.1