Build certificate chains even when intermediates are wrong order

In GcrCertificateChain we respect the RFC 5246 which requires
that certificates appear in the correct order from the server:
First the endpoint, then intermediates, and (optionally the
root last).

However some servers (like hermes.jabber.org) send certificates
in an incorrect order. It seems like many SSL implementations
accept intermediate certificates out of order.

https://bugzilla.gnome.org/show_bug.cgi?id=699026

5 files changed, 95 insertions, 12 deletions

diff --git a/gcr/gcr-certificate-chain.c b/gcr/gcr-certificate-chain.c
index 5637634..4c7de1c 100644
--- a/gcr/gcr-certificate-chain.c
+++ b/gcr/gcr-certificate-chain.c
@@ -214,12 +214,33 @@ cleanup_chain_private (GcrCertificateChainPrivate *pv)
 	return pv;
 }
 
+static GcrCertificate *
+pop_certificate (GPtrArray *certificates,
+                 GcrCertificate *issued)
+{
+	GcrCertificate *certificate;
+	gint i;
+
+	for (i = 0; i < certificates->len; i++) {
+		certificate = certificates->pdata[i];
+		if (!issued || gcr_certificate_is_issuer (issued, certificate)) {
+			g_object_ref (certificate);
+			g_ptr_array_remove_index_fast (certificates, i);
+			return certificate;
+		}
+	}
+
+	return NULL;
+}
+
 static gboolean
 perform_build_chain (GcrCertificateChainPrivate *pv, GCancellable *cancellable,
                      GError **rerror)
 {
 	GError *error = NULL;
 	GcrCertificate *certificate;
+	GcrCertificate *issued;
+	GPtrArray *input;
 	gboolean lookups;
 	gboolean ret;
 	guint length;
@@ -237,8 +258,13 @@ perform_build_chain (GcrCertificateChainPrivate *pv, GCancellable *cancellable,
 		return TRUE;
 	}
 
+	input = pv->certificates;
+	pv->certificates = g_ptr_array_new_with_free_func (g_object_unref);
+
 	/* First check for pinned certificates */
-	certificate = g_ptr_array_index (pv->certificates, 0);
+	certificate = pop_certificate (input, NULL);
+	g_ptr_array_add (pv->certificates, certificate);
+
 	if (_gcr_debugging) {
 		subject = gcr_certificate_get_subject_dn (certificate);
 		_gcr_debug ("first certificate: %s", subject);
@@ -252,6 +278,7 @@ perform_build_chain (GcrCertificateChainPrivate *pv, GCancellable *cancellable,
 			_gcr_debug ("failed to lookup pinned certificate: %s",
 			            egg_error_message (error));
 			g_propagate_error (rerror, error);
+			g_ptr_array_unref (input);
 			return FALSE;
 		}
 
@@ -263,16 +290,15 @@ perform_build_chain (GcrCertificateChainPrivate *pv, GCancellable *cancellable,
 			_gcr_debug ("found pinned certificate for peer '%s', truncating chain",
 			            pv->peer);
 
-			g_ptr_array_set_size (pv->certificates, 1);
+			g_ptr_array_unref (input);
 			pv->status = GCR_CERTIFICATE_CHAIN_PINNED;
 			return TRUE;
 		}
 	}
 
-	length = 1;
-
 	/* The first certificate is always unconditionally in the chain */
 	while (pv->status == GCR_CERTIFICATE_CHAIN_UNKNOWN) {
+		issued = certificate;
 
 		/* Stop the chain if previous was self-signed */
 		if (gcr_certificate_is_issuer (certificate, certificate)) {
@@ -281,9 +307,9 @@ perform_build_chain (GcrCertificateChainPrivate *pv, GCancellable *cancellable,
 			break;
 		}
 
-		/* Try the next certificate in the chain */
-		if (length < pv->certificates->len) {
-			certificate = g_ptr_array_index (pv->certificates, length);
+		/* Get the next certificate */
+		certificate = pop_certificate (input, issued);
+		if (certificate) {
 			if (_gcr_debugging) {
 				subject = gcr_certificate_get_subject_dn (certificate);
 				_gcr_debug ("next certificate: %s", subject);
@@ -292,15 +318,15 @@ perform_build_chain (GcrCertificateChainPrivate *pv, GCancellable *cancellable,
 
 		/* No more in chain, try to lookup */
 		} else if (lookups) {
-			certificate = gcr_pkcs11_certificate_lookup_issuer (certificate,
+			certificate = gcr_pkcs11_certificate_lookup_issuer (issued,
 			                                                    cancellable, &error);
 			if (error != NULL) {
 				_gcr_debug ("failed to lookup issuer: %s", error->message);
 				g_propagate_error (rerror, error);
+				g_ptr_array_unref (input);
 				return FALSE;
 
 			} else if (certificate) {
-				g_ptr_array_add (pv->certificates, certificate);
 				if (_gcr_debugging) {
 					subject = gcr_certificate_get_subject_dn (certificate);
 					_gcr_debug ("found issuer certificate: %s", subject);
@@ -324,6 +350,7 @@ perform_build_chain (GcrCertificateChainPrivate *pv, GCancellable *cancellable,
 			break;
 		}
 
+		g_ptr_array_add (pv->certificates, certificate);
 		++length;
 
 		/* See if this certificate is an anchor */
@@ -335,6 +362,7 @@ perform_build_chain (GcrCertificateChainPrivate *pv, GCancellable *cancellable,
 				_gcr_debug ("failed to lookup anchored certificate: %s",
 				            egg_error_message (error));
 				g_propagate_error (rerror, error);
+				g_ptr_array_unref (input);
 				return FALSE;
 
 			/* Stop the chain at the first anchor */
@@ -348,9 +376,7 @@ perform_build_chain (GcrCertificateChainPrivate *pv, GCancellable *cancellable,
 
 	/* TODO: Need to check each certificate in the chain for distrusted */
 
-	/* Truncate to the appropriate length */
-	g_assert (length <= pv->certificates->len);
-	g_ptr_array_set_size (pv->certificates, length);
+	g_ptr_array_unref (input);
 	return TRUE;
 }
 
diff --git a/gcr/tests/files/jabber-server.cer b/gcr/tests/files/jabber-server.cer
new file mode 100644
index 0000000..086b3ae
--- /dev/null
+++ b/gcr/tests/files/jabber-server.cer
diff --git a/gcr/tests/files/startcom-ca.cer b/gcr/tests/files/startcom-ca.cer
new file mode 100644
index 0000000..b60dea2
--- /dev/null
+++ b/gcr/tests/files/startcom-ca.cer
diff --git a/gcr/tests/files/startcom-intermediate.cer b/gcr/tests/files/startcom-intermediate.cer
new file mode 100644
index 0000000..a27a7df
--- /dev/null
+++ b/gcr/tests/files/startcom-intermediate.cer
diff --git a/gcr/tests/test-certificate-chain.c b/gcr/tests/test-certificate-chain.c
index cbeaeb9..2fa42d9 100644
--- a/gcr/tests/test-certificate-chain.c
+++ b/gcr/tests/test-certificate-chain.c
@@ -121,9 +121,18 @@ mock_certificate_new (gconstpointer data, gsize n_data)
  */
 
 typedef struct {
+	/* A self signed certificate */
 	GcrCertificate *cert_self;
+
+	/* Simple CA + issued */
 	GcrCertificate *cert_ca;
 	GcrCertificate *cert_signed;
+
+	/* Root + intermediate + issued */
+	GcrCertificate *cert_root;
+	GcrCertificate *cert_inter;
+	GcrCertificate *cert_host;
+
 	CK_FUNCTION_LIST funcs;
 } Test;
 
@@ -173,6 +182,24 @@ setup (Test *test, gconstpointer unused)
 		g_assert_not_reached ();
 	test->cert_ca = mock_certificate_new (contents, n_contents);
 	g_free (contents);
+
+	/* A root CA */
+	if (!g_file_get_contents (SRCDIR "/files/startcom-ca.cer", &contents, &n_contents, NULL))
+		g_assert_not_reached ();
+	test->cert_root = mock_certificate_new (contents, n_contents);
+	g_free (contents);
+
+	/* An intermediate */
+	if (!g_file_get_contents (SRCDIR "/files/startcom-intermediate.cer", &contents, &n_contents, NULL))
+		g_assert_not_reached ();
+	test->cert_inter = mock_certificate_new (contents, n_contents);
+	g_free (contents);
+
+	/* Signed by above intermediate */
+	if (!g_file_get_contents (SRCDIR "/files/jabber-server.cer", &contents, &n_contents, NULL))
+		g_assert_not_reached ();
+	test->cert_host = mock_certificate_new (contents, n_contents);
+	g_free (contents);
 }
 
 static void
@@ -573,6 +600,35 @@ test_with_lookup_error (Test *test, gconstpointer unused)
 }
 
 static void
+test_wrong_order_anchor (Test *test, gconstpointer unused)
+{
+	GcrCertificateChain *chain;
+	GError *error = NULL;
+
+	chain = gcr_certificate_chain_new ();
+
+	/* Two certificates in chain with ca trust anchor */
+	gcr_certificate_chain_add (chain, test->cert_host);
+	gcr_certificate_chain_add (chain, test->cert_root);
+	gcr_certificate_chain_add (chain, test->cert_inter);
+	add_anchor_to_module (test->cert_root, GCR_PURPOSE_CLIENT_AUTH);
+
+	g_assert_cmpuint (gcr_certificate_chain_get_length (chain), ==, 3);
+
+	if (!gcr_certificate_chain_build (chain, GCR_PURPOSE_CLIENT_AUTH,
+	                                  NULL, 0, NULL, &error))
+		g_assert_not_reached ();
+	g_assert_no_error (error);
+
+	g_assert_cmpuint (gcr_certificate_chain_get_status (chain), ==,
+	                  GCR_CERTIFICATE_CHAIN_ANCHORED);
+	g_assert_cmpuint (gcr_certificate_chain_get_length (chain), ==, 3);
+	g_assert (gcr_certificate_chain_get_anchor (chain) == test->cert_root);
+
+	g_object_unref (chain);
+}
+
+static void
 test_with_anchor_error (Test *test, gconstpointer unused)
 {
 	GcrCertificateChain *chain;
@@ -650,6 +706,7 @@ main (int argc, char **argv)
 	g_test_add ("/gcr/certificate-chain/with_anchor_and_lookup_ca", Test, NULL, setup, test_with_anchor_and_lookup_ca, teardown);
 	g_test_add ("/gcr/certificate-chain/with_pinned", Test, NULL, setup, test_with_pinned, teardown);
 	g_test_add ("/gcr/certificate-chain/without_lookups", Test, NULL, setup, test_without_lookups, teardown);
+	g_test_add ("/gcr/certificate-chain/wrong_order_anchor", Test, NULL, setup, test_wrong_order_anchor, teardown);
 	g_test_add ("/gcr/certificate-chain/with_lookup_error", Test, NULL, setup, test_with_lookup_error, teardown);
 	g_test_add ("/gcr/certificate-chain/with_anchor_error", Test, NULL, setup, test_with_anchor_error, teardown);
 	g_test_add ("/gcr/certificate-chain/with_anchor_error_async", Test, NULL, setup, test_with_anchor_error_async, teardown);