Accept unordered certificate chains

Some TLS servers improperly send an unordered chain of certificates,
where the next certificate in the chain is not the issuer of the current
certificate. Recent versions of GnuTLS will verify the chain anyway to
help reduce unnecessary validation failures (since there is no security
risk in doing so).

When the certificates are unordered, get_peer_certificate_from_session()
will construct GTlsCertificates with incorrect issuer fields, causing
trouble with unordered chains even though GnuTLS would otherwise handle
these fine.

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

Change-Id: I38fb9e15c6500c39ec860c1428e836595c9fba82
Signed-off-by: Seonah Moon <seonah1.moon@samsung.com>

diff --git a/tls/gnutls/gtlscertificate-gnutls.c b/tls/gnutls/gtlscertificate-gnutls.c
index ced5346..354f4c1 100755 (executable)
--- a/tls/gnutls/gtlscertificate-gnutls.c
+++ b/tls/gnutls/gtlscertificate-gnutls.c
@@ -616,3 +616,101 @@ g_tls_certificate_gnutls_get_bytes (GTlsCertificateGnutls *gnutls)
   g_object_get (gnutls, "certificate", &array, NULL);
   return g_byte_array_free_to_bytes (array);
 }
+
+static gnutls_x509_crt_t *
+convert_data_to_gnutls_certs (const gnutls_datum_t  *certs,
+                              guint                  num_certs,
+                              gnutls_x509_crt_fmt_t  format)
+{
+  gnutls_x509_crt_t *gnutls_certs;
+  guint i;
+
+  gnutls_certs = g_new (gnutls_x509_crt_t, num_certs);
+
+  for (i = 0; i < num_certs; i++)
+    {
+      if (gnutls_x509_crt_init (&gnutls_certs[i]) < 0)
+        {
+          i--;
+          goto error;
+        }
+    }
+
+  for (i = 0; i < num_certs; i++)
+    {
+      if (gnutls_x509_crt_import (gnutls_certs[i], &certs[i], format) < 0)
+        {
+          i = num_certs - 1;
+          goto error;
+        }
+    }
+
+  return gnutls_certs;
+
+error:
+  for (; i != G_MAXUINT; i--)
+    gnutls_x509_crt_deinit (gnutls_certs[i]);
+  g_free (gnutls_certs);
+  return NULL;
+}
+
+GTlsCertificateGnutls *
+g_tls_certificate_gnutls_build_chain (const gnutls_datum_t  *certs,
+                                      guint                  num_certs,
+                                      gnutls_x509_crt_fmt_t  format)
+{
+  GPtrArray *glib_certs;
+  gnutls_x509_crt_t *gnutls_certs;
+  GTlsCertificateGnutls *issuer;
+  GTlsCertificateGnutls *result;
+  guint i, j;
+
+  g_return_val_if_fail (certs, NULL);
+
+  gnutls_certs = convert_data_to_gnutls_certs (certs, num_certs, format);
+  if (!gnutls_certs)
+    return NULL;
+
+  glib_certs = g_ptr_array_new_full (num_certs, g_object_unref);
+  for (i = 0; i < num_certs; i++)
+    g_ptr_array_add (glib_certs, g_tls_certificate_gnutls_new (&certs[i], NULL));
+
+  /* Some servers send certs out of order, or will send duplicate
+   * certs, so we need to be careful when assigning the issuer of
+   * our new GTlsCertificateGnutls.
+   */
+  for (i = 0; i < num_certs; i++)
+    {
+      issuer = NULL;
+
+      if (i < num_certs - 1 &&
+          gnutls_x509_crt_check_issuer (gnutls_certs[i], gnutls_certs[i + 1]))
+        {
+          issuer = glib_certs->pdata[i + 1];
+        }
+      else
+        {
+          for (j = 0; j < num_certs; j++)
+            {
+              if (j != i &&
+                  gnutls_x509_crt_check_issuer (gnutls_certs[i], gnutls_certs[j]))
+                {
+                  issuer = glib_certs->pdata[j];
+                  break;
+                }
+            }
+        }
+
+      if (issuer)
+        g_tls_certificate_gnutls_set_issuer (glib_certs->pdata[i], issuer);
+    }
+
+  result = g_object_ref (glib_certs->pdata[0]);
+  g_ptr_array_unref (glib_certs);
+
+  for (i = 0; i < num_certs; i++)
+    gnutls_x509_crt_deinit (gnutls_certs[i]);
+  g_free (gnutls_certs);
+
+  return result;
+}

diff --git a/tls/gnutls/gtlscertificate-gnutls.h b/tls/gnutls/gtlscertificate-gnutls.h
index 94fddeb..bf79883 100644 (file)
--- a/tls/gnutls/gtlscertificate-gnutls.h
+++ b/tls/gnutls/gtlscertificate-gnutls.h
@@ -71,6 +71,10 @@ void                         g_tls_certificate_gnutls_set_issuer      (GTlsCerti
 
 GTlsCertificateGnutls*       g_tls_certificate_gnutls_steal_issuer    (GTlsCertificateGnutls *gnutls);
 
+GTlsCertificateGnutls*       g_tls_certificate_gnutls_build_chain     (const gnutls_datum_t  *certs,
+                                                                       guint                  num_certs,
+                                                                       gnutls_x509_crt_fmt_t  format);
+
 G_END_DECLS
 
 #endif /* __G_TLS_CERTIFICATE_GNUTLS_H___ */

diff --git a/tls/gnutls/gtlsconnection-gnutls.c b/tls/gnutls/gtlsconnection-gnutls.c
index 2431a04..2745773 100755 (executable)
--- a/tls/gnutls/gtlsconnection-gnutls.c
+++ b/tls/gnutls/gtlsconnection-gnutls.c
@@ -1074,29 +1074,22 @@ g_tls_connection_gnutls_push_func (gnutls_transport_ptr_t  transport_data,
   return ret;
 }
 
-
 static GTlsCertificate *
 get_peer_certificate_from_session (GTlsConnectionGnutls *gnutls)
 {
-  GTlsCertificate *chain, *cert;
   const gnutls_datum_t *certs;
+  GTlsCertificateGnutls *chain;
   unsigned int num_certs;
-  int i;
 
   certs = gnutls_certificate_get_peers (gnutls->priv->session, &num_certs);
   if (!certs || !num_certs)
     return NULL;
 
-  chain = NULL;
-  for (i = num_certs - 1; i >= 0; i--)
-    {
-      cert = g_tls_certificate_gnutls_new (&certs[i], chain);
-      if (chain)
-       g_object_unref (chain);
-      chain = cert;
-    }
+  chain = g_tls_certificate_gnutls_build_chain (certs, num_certs, GNUTLS_X509_FMT_DER);
+  if (!chain)
+    return NULL;
 
-  return chain;
+  return G_TLS_CERTIFICATE (chain);
 }
 
 static GTlsCertificateFlags