Attachment 65997 Details for Bug 53956 – Run IPA discovery in parallel against multiple KDCs

[patch] Run IPA discovery in parallel against multiple KDCs

0002-Run-IPA-discovery-in-parallel-against-multiple-KDCs.patch (text/plain), 12.08 KB, created by Stef Walter on 2012-08-23 06:34:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stef Walter

Created: 2012-08-23 06:34:49 UTC

Size: 12.08 KB

patch

obsolete

>From e71c81266492042463d3c94210fdf4d1da70b9a3 Mon Sep 17 00:00:00 2001
>From: Stef Walter <stefw@gnome.org>
>Date: Wed, 22 Aug 2012 18:23:16 +0200
>Subject: [PATCH 2/2] Run IPA discovery in parallel against multiple KDCs
>
> * Since we actually try to connect to the https port of the KDC
>   this can timeout due to firewalls.
> * Set maximum timeout to 5 seconds for connection.
> * Try to connect to up to 3 KDCs in parallel, which should give us
>   redundancy, and only take 5 seconds total when firewalled.
>
>://bugs.freedesktop.org/show_bug.cgi?id=53956
>---
> service/realm-ipa-discover.c      | 112 ++++++++++++++------------------------
> service/realm-ipa-discover.h      |   2 +-
> service/realm-kerberos-discover.c |  47 +++++++++-------
> 3 files changed, 69 insertions(+), 92 deletions(-)
>
>diff --git a/service/realm-ipa-discover.c b/service/realm-ipa-discover.c
>index 941dc96..7b3118a 100644
>--- a/service/realm-ipa-discover.c
>+++ b/service/realm-ipa-discover.c
>@@ -36,11 +36,9 @@ typedef struct {
> 	GAsyncReadyCallback callback;
> 	gpointer user_data;
> 
>-	/* Used while iterating through services */
>-	GQueue remaining_services;
>+	GSrvTarget *kdc;
> 	GBytes *http_request;
> 	GIOStream *current_connection;
>-	GSrvTarget *current_target;
> 	GTlsCertificate *peer_certificate;
> 	GSocketConnectable *peer_identity;
> } RealmIpaDiscover;
>@@ -55,10 +53,6 @@ typedef struct {
> 
> GType realm_ipa_discover_get_type (void) G_GNUC_CONST;
> 
>-static void ipa_discover_step (RealmIpaDiscover *self);
>-
>-static void ipa_discover_step_clear (RealmIpaDiscover *self);
>-
> void  realm_ipa_discover_async_result_init (GAsyncResultIface *iface);
> 
> G_DEFINE_TYPE_WITH_CODE (RealmIpaDiscover, realm_ipa_discover, G_TYPE_OBJECT,
>@@ -68,7 +62,7 @@ G_DEFINE_TYPE_WITH_CODE (RealmIpaDiscover, realm_ipa_discover, G_TYPE_OBJECT,
> static void
> realm_ipa_discover_init (RealmIpaDiscover *self)
> {
>-	g_queue_init (&self->remaining_services);
>+
> }
> 
> static void
>@@ -77,11 +71,21 @@ realm_ipa_discover_finalize (GObject *obj)
> 	RealmIpaDiscover *self = REALM_IPA_DISCOVER (obj);
> 
> 	g_object_unref (self->invocation);
>-	g_list_free_full (self->services, (GDestroyNotify)g_srv_target_free);
> 	g_clear_error (&self->error);
>+	g_srv_target_free (self->kdc);
>+
>+	if (self->http_request)
>+		g_bytes_unref (self->http_request);
>+
>+	if (self->current_connection)
>+		g_object_unref (self->current_connection);
>+
>+	if (self->peer_certificate)
>+		g_object_unref (self->peer_certificate);
>+
>+	if (self->peer_identity)
>+		g_object_unref (self->peer_identity);
> 
>-	ipa_discover_step_clear (self);
>-	g_queue_clear (&self->remaining_services);
> 	g_assert (self->callback == NULL);
> 
> 	G_OBJECT_CLASS (realm_ipa_discover_parent_class)->finalize (obj);
>@@ -308,29 +312,6 @@ write_all_bytes_finish (GOutputStream *stream,
> }
> 
> static void
>-ipa_discover_step_clear (RealmIpaDiscover *self)
>-{
>-	if (self->http_request)
>-		g_bytes_unref (self->http_request);
>-	self->http_request = NULL;
>-
>-	if (self->current_connection)
>-		g_object_unref (self->current_connection);
>-	self->current_connection = NULL;
>-
>-	/* Owned by self->services */
>-	self->current_target = NULL;
>-
>-	if (self->peer_certificate)
>-		g_object_unref (self->peer_certificate);
>-	self->peer_certificate = NULL;
>-
>-	if (self->peer_identity)
>-		g_object_unref (self->peer_identity);
>-	self->peer_identity = NULL;
>-}
>-
>-static void
> ipa_discover_complete (RealmIpaDiscover *self)
> {
> 	GAsyncReadyCallback call;
>@@ -449,7 +430,7 @@ on_read_http_response (GObject *source,
> 	if (error != NULL)
> 		ipa_discover_take_error (self, "Couldn't read certificate via HTTP", error);
> 	if (!self->completed)
>-		ipa_discover_step (self);
>+		ipa_discover_complete (self);
> 
> 	g_object_unref (self);
> }
>@@ -473,7 +454,7 @@ on_write_http_request (GObject *source,
> 		                      on_read_http_response, g_object_ref (self));
> 	} else {
> 		ipa_discover_take_error (self, "Couldn't send HTTP request for certificate", error);
>-		ipa_discover_step (self);
>+		ipa_discover_complete (self);
> 	}
> 
> 	g_object_unref (self);
>@@ -495,7 +476,7 @@ on_connect_to_host (GObject *source,
> 
> 	request = g_strdup_printf ("GET /ipa/config/ca.crt HTTP/1.0\r\n"
> 	                           "Host: %s\r\n"
>-	                           "\r\n", g_srv_target_get_hostname (self->current_target));
>+	                           "\r\n", g_srv_target_get_hostname (self->kdc));
> 	self->http_request = g_bytes_new_take (request, strlen (request));
> 
> 	connection = g_socket_client_connect_to_host_finish (G_SOCKET_CLIENT (source), result, &error);
>@@ -511,11 +492,11 @@ on_connect_to_host (GObject *source,
> 	           g_error_matches (error, G_IO_ERROR, G_IO_ERROR_NETWORK_UNREACHABLE) ||
> 	           g_error_matches (error, G_IO_ERROR, G_IO_ERROR_TIMED_OUT)) {
> 		realm_debug ("Couldn't connect to check for IPA domain: %s", error->message);
>-		ipa_discover_step (self);
>+		ipa_discover_complete (self);
> 
> 	} else {
> 		ipa_discover_take_error (self, "Couldn't connect to check for IPA domain", error);
>-		ipa_discover_step (self);
>+		ipa_discover_complete (self);
> 	}
> 
> 	g_object_unref (self);
>@@ -539,36 +520,47 @@ on_connection_event (GSocketClient      *client,
> 	} else if (event == G_SOCKET_CLIENT_RESOLVED) {
> 		g_return_if_fail (self->peer_identity == NULL);
> 		self->peer_identity = g_object_ref (connectable);
>+
>+	/* Once connected, raise the timeout, so TLS can succeed */
>+	} else if (event == G_SOCKET_CLIENT_CONNECTED) {
>+		g_socket_client_set_timeout (client, 30);
> 	}
> }
> 
> #define VALID_DNS_CHARS \
> 	"abcdefghijklnmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-."
> 
>-static void
>-ipa_discover_step (RealmIpaDiscover *self)
>+void
>+realm_ipa_discover_async (GSrvTarget *kdc,
>+                          GDBusMethodInvocation *invocation,
>+                          GAsyncReadyCallback callback,
>+                          gpointer user_data)
> {
>+	RealmIpaDiscover *self;
> 	GSocketClient *client;
> 	const gchar *hostname;
> 
>-	ipa_discover_step_clear (self);
>+	g_return_if_fail (G_IS_DBUS_METHOD_INVOCATION (invocation));
> 
>-	if (g_queue_is_empty (&self->remaining_services)) {
>-		ipa_discover_complete (self);
>-		return;
>-	}
>+	self = g_object_new (REALM_TYPE_IPA_DISCOVER, NULL);
>+	self->invocation = g_object_ref (invocation);
>+	self->callback = callback;
>+	self->user_data = user_data;
>+	self->kdc = g_srv_target_copy (kdc);
> 
>-	self->current_target = g_queue_pop_head (&self->remaining_services);
>-	hostname = g_srv_target_get_hostname (self->current_target);
>+	hostname = g_srv_target_get_hostname (self->kdc);
> 
> 	/* If an invalid hostname, skip and go to next step */
> 	if (strspn (hostname, VALID_DNS_CHARS) != strlen (hostname)) {
>-		ipa_discover_step (self);
>+		ipa_discover_complete (self);
> 		return;
> 	}
> 
> 	client = g_socket_client_new ();
> 
>+	/* Initial socket connections are limited to a low timeout*/
>+	g_socket_client_set_timeout (client, 5);
>+
> 	/*
> 	 * Note that we accept invalid certificates, we're just comparing them
> 	 * with what's on the server at this point. Later during the join the
>@@ -588,28 +580,6 @@ ipa_discover_step (RealmIpaDiscover *self)
> 	                                       NULL, on_connect_to_host, g_object_ref (self));
> 
> 	g_object_unref (client);
>-}
>-
>-void
>-realm_ipa_discover_async (GList *kdcs,
>-                          GDBusMethodInvocation *invocation,
>-                          GAsyncReadyCallback callback,
>-                          gpointer user_data)
>-{
>-	RealmIpaDiscover *self;
>-	GList *l;
>-
>-	g_return_if_fail (G_IS_DBUS_METHOD_INVOCATION (invocation));
>-
>-	self = g_object_new (REALM_TYPE_IPA_DISCOVER, NULL);
>-	self->invocation = g_object_ref (invocation);
>-	self->callback = callback;
>-	self->user_data = user_data;
>-
>-	for (l = kdcs; l != NULL; l = g_list_next (l))
>-		g_queue_push_tail (&self->remaining_services, l->data);
>-
>-	ipa_discover_step (self);
> 
> 	/* self is released in ipa_discover_complete */
> }
>diff --git a/service/realm-ipa-discover.h b/service/realm-ipa-discover.h
>index 8930696..6da1b99 100644
>--- a/service/realm-ipa-discover.h
>+++ b/service/realm-ipa-discover.h
>@@ -21,7 +21,7 @@
> 
> G_BEGIN_DECLS
> 
>-void           realm_ipa_discover_async        (GList *kdcs,
>+void           realm_ipa_discover_async        (GSrvTarget *kdc,
>                                                 GDBusMethodInvocation *invocation,
>                                                 GAsyncReadyCallback callback,
>                                                 gpointer user_data);
>diff --git a/service/realm-kerberos-discover.c b/service/realm-kerberos-discover.c
>index 1e6fdb5..58a82e9 100644
>--- a/service/realm-kerberos-discover.c
>+++ b/service/realm-kerberos-discover.c
>@@ -42,11 +42,11 @@ typedef struct {
> 	gchar *domain;
> 	GList *servers;
> 	gboolean found_kerberos;
>-	gboolean finished_kerberos;
>+	gint outstanding_kerberos;
> 	gboolean found_msdcs;
>-	gboolean finished_msdcs;
>+	gint outstanding_msdcs;
> 	gboolean found_ipa;
>-	gboolean finished_ipa;
>+	gint outstanding_ipa;
> 	gboolean completed;
> 	GError *error;
> 	Callback *callback;
>@@ -157,9 +157,9 @@ maybe_complete_discover (RealmKerberosDiscover *self)
> 	gboolean complete;
> 
> 	/* If discovered either AD or IPA successfully, then complete */
>-	complete = (self->finished_kerberos &&
>+	complete = (!self->outstanding_kerberos &&
> 	            (self->found_msdcs || self->found_ipa ||
>-	             (self->finished_msdcs && self->finished_ipa)));
>+	             (!self->outstanding_msdcs && !self->outstanding_ipa)));
> 
> 	if (!complete)
> 		return;
>@@ -185,19 +185,18 @@ on_discover_ipa (GObject *source,
> 	RealmKerberosDiscover *self = REALM_KERBEROS_DISCOVER (user_data);
> 	GError *error = NULL;
> 
>-	if (self->completed) {
>-		g_object_unref (self);
>-		return;
>-	}
>+	g_assert (self->outstanding_ipa > 0);
>+	self->outstanding_ipa--;
> 
>-	self->found_ipa = realm_ipa_discover_finish (result, &error);
>-	if (error != NULL) {
>-		g_clear_error (&self->error);
>-		self->error = error;
>+	if (!self->completed && !self->found_ipa) {
>+		self->found_ipa = realm_ipa_discover_finish (result, &error);
>+		if (error != NULL) {
>+			g_clear_error (&self->error);
>+			self->error = error;
>+		}
>+		maybe_complete_discover (self);
> 	}
> 
>-	self->finished_ipa = TRUE;
>-	maybe_complete_discover (self);
> 	g_object_unref (self);
> }
> 
>@@ -211,6 +210,9 @@ on_resolve_kerberos (GObject *source,
> 	GError *error = NULL;
> 	GString *info;
> 	GList *l;
>+	gint i;
>+
>+	self->outstanding_kerberos = 0;
> 
> 	if (self->completed) {
> 		g_object_unref (self);
>@@ -237,10 +239,13 @@ on_resolve_kerberos (GObject *source,
> 
> 		g_string_free (info, TRUE);
> 
>-		/* Unless sure domain is AD, start IPA discovery */
>+		/* Unless sure domain is AD, start IPA discovery, for first N KDCs */
> 		if (!self->found_msdcs) {
>-			realm_ipa_discover_async (self->servers, invocation,
>-			                          on_discover_ipa, g_object_ref (self));
>+			for (l = self->servers, i = 0; l != NULL && i < 3; l = g_list_next (l), i++) {
>+				realm_ipa_discover_async (l->data, invocation,
>+				                          on_discover_ipa, g_object_ref (self));
>+				self->outstanding_ipa++;
>+			}
> 		}
> 
> 	} else {
>@@ -249,7 +254,6 @@ on_resolve_kerberos (GObject *source,
> 		self->error = error;
> 	}
> 
>-	self->finished_kerberos = TRUE;
> 	maybe_complete_discover (self);
> 	g_object_unref (self);
> }
>@@ -265,6 +269,8 @@ on_resolve_msdcs (GObject *source,
> 	GError *error = NULL;
> 	GList *records;
> 
>+	self->outstanding_msdcs = 0;
>+
> 	if (self->completed) {
> 		g_object_unref (self);
> 		return;
>@@ -281,7 +287,6 @@ on_resolve_msdcs (GObject *source,
> 		self->error = error;
> 	}
> 
>-	self->finished_msdcs = TRUE;
> 	maybe_complete_discover (self);
> 	g_object_unref (self);
> }
>@@ -302,6 +307,7 @@ kerberos_discover_domain_begin (RealmKerberosDiscover *self)
> 	resolver = g_resolver_get_default ();
> 	g_resolver_lookup_service_async (resolver, "kerberos", "udp", self->domain, NULL,
> 	                                 on_resolve_kerberos, g_object_ref (self));
>+	self->outstanding_kerberos = 1;
> 
> 	/* Active Directory DNS zones have this subzone */
> 	msdcs = g_strdup_printf ("dc._msdcs.%s", self->domain);
>@@ -312,6 +318,7 @@ kerberos_discover_domain_begin (RealmKerberosDiscover *self)
> 
> 	g_resolver_lookup_service_async (resolver, "kerberos", "tcp", msdcs, NULL,
> 	                                 on_resolve_msdcs, g_object_ref (self));
>+	self->outstanding_msdcs = 1;
> 
> 	g_free (msdcs);
> 
>-- 
>1.7.12
>

Actions: View | Splinter Review

Attachments on bug 53956: 65996 | 65997

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.