[gcr] Build certificate chains even when intermediates are wrong order

[Stefan Walter <stefw src gnome org>]

commit 63c1742a6eb5aaf6ea6ea25ed4e310fbe62af7bd
Author: Stef Walter <stefw gnome org>
Date:   Sat Apr 27 13:26:59 2013 +0200

    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

 gcr/gcr-certificate-chain.c               |   50 +++++++++++++++++++------
 gcr/tests/files/jabber-server.cer         |  Bin 0 -> 2095 bytes
 gcr/tests/files/startcom-ca.cer           |  Bin 0 -> 1997 bytes
 gcr/tests/files/startcom-intermediate.cer |  Bin 0 -> 1592 bytes
 gcr/tests/test-certificate-chain.c        |   57 +++++++++++++++++++++++++++++
 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
Binary files /dev/null and b/gcr/tests/files/jabber-server.cer differ
diff --git a/gcr/tests/files/startcom-ca.cer b/gcr/tests/files/startcom-ca.cer
new file mode 100644
index 0000000..b60dea2
Binary files /dev/null and b/gcr/tests/files/startcom-ca.cer differ
diff --git a/gcr/tests/files/startcom-intermediate.cer b/gcr/tests/files/startcom-intermediate.cer
new file mode 100644
index 0000000..a27a7df
Binary files /dev/null and b/gcr/tests/files/startcom-intermediate.cer differ
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);