GNOME.org
 

[glib-networking/tls-database] gnutls: Cleanup of GTlsDatabase based on Dan Winship's comments

commit 0010f62472711d5d5343220a0ac11f4050f790a2
Author: Stef Walter <stefw collabora co uk>
Date:   Thu Dec 16 16:33:55 2010 +0000

    gnutls: Cleanup of GTlsDatabase based on Dan Winship's comments
    
     * Add way to modify GTlsCertificateGnutls::issuer while keeping
       public API read-only.
     * Use GTlsCertificate as the chain instead of GList.
     * Include anchor in the chain.
     * Modify the @chain passed to verify_chain to reflect what
       was verified.
     * Match cleanup made to GTlsDatabase in glib project.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=636572#c5

 tls/gnutls/gtlscertificate-gnutls.c |   15 +++
 tls/gnutls/gtlscertificate-gnutls.h |    4 +
 tls/gnutls/gtlsdatabase-gnutls.c    |  207 ++++++++++++++++++-----------------
 3 files changed, 126 insertions(+), 100 deletions(-)
---
diff --git a/tls/gnutls/gtlscertificate-gnutls.c b/tls/gnutls/gtlscertificate-gnutls.c
index 68b6e2d..3fd49a2 100644
--- a/tls/gnutls/gtlscertificate-gnutls.c
+++ b/tls/gnutls/gtlscertificate-gnutls.c
@@ -496,3 +496,18 @@ g_tls_certificate_gnutls_verify_identity (GTlsCertificateGnutls *gnutls,
 
   return G_TLS_CERTIFICATE_BAD_IDENTITY;
 }
+
+void
+g_tls_certificate_gnutls_set_issuer (GTlsCertificateGnutls *gnutls,
+                                     GTlsCertificateGnutls *issuer)
+{
+  g_return_if_fail (G_IS_TLS_CERTIFICATE_GNUTLS (gnutls));
+  g_return_if_fail (!issuer || G_IS_TLS_CERTIFICATE_GNUTLS (issuer));
+
+  if (issuer)
+    g_object_ref (issuer);
+  if (gnutls->priv->issuer)
+    g_object_unref (gnutls->priv->issuer);
+  gnutls->priv->issuer = issuer;
+  g_object_notify (G_OBJECT (gnutls), "issuer");
+}
diff --git a/tls/gnutls/gtlscertificate-gnutls.h b/tls/gnutls/gtlscertificate-gnutls.h
index a94117a..201dbb4 100644
--- a/tls/gnutls/gtlscertificate-gnutls.h
+++ b/tls/gnutls/gtlscertificate-gnutls.h
@@ -56,6 +56,10 @@ GTlsCertificateFlags         g_tls_certificate_gnutls_verify_identity (GTlsCerti
 
 GTlsCertificateFlags         g_tls_certificate_gnutls_convert_flags   (guint                  gnutls_flags);
 
+void                         g_tls_certificate_gnutls_set_issuer      (GTlsCertificateGnutls *gnutls,
+                                                                       GTlsCertificateGnutls *issuer);
+
+GTlsCertificateGnutls*       g_tls_certificate_gnutls_steal_issuer    (GTlsCertificateGnutls *gnutls);
 
 G_END_DECLS
 
diff --git a/tls/gnutls/gtlsdatabase-gnutls.c b/tls/gnutls/gtlsdatabase-gnutls.c
index aeacf27..75181dd 100644
--- a/tls/gnutls/gtlsdatabase-gnutls.c
+++ b/tls/gnutls/gtlsdatabase-gnutls.c
@@ -57,13 +57,13 @@ list_unref_free (GList *list)
   return NULL;
 }
 
-static GTlsCertificateGnutls*
+static GTlsCertificate*
 lookup_best_issuer (GTlsDatabaseGnutls *self,
                     GTlsCertificateGnutls *certificate,
                     GCancellable *cancellable,
                     GError **error)
 {
-  GTlsCertificateGnutls *result;
+  GTlsCertificate *result;
   gnutls_datum_t dn = { NULL, 0 };
   gnutls_x509_crt_t cert;
   GList *list;
@@ -111,43 +111,42 @@ is_self_signed (GTlsCertificateGnutls *certificate)
 }
 
 static gint
-build_certificate_chain (GTlsDatabaseGnutls        *self,
-                         GList                     *input,
-                         GSocketConnectable        *identity,
-                         gint                       flags,
-                         GCancellable              *cancellable,
-                         GList                    **chain,
-                         GList                    **anchors,
-                         GError                   **error)
+build_certificate_chain (GTlsDatabaseGnutls      *self,
+                         GTlsCertificateGnutls   *chain,
+                         GSocketConnectable      *identity,
+                         GTlsDatabaseVerifyFlags  flags,
+                         GCancellable            *cancellable,
+                         GTlsCertificateGnutls  **anchor,
+                         GError                 **error)
 {
 
   GTlsCertificateGnutls *certificate;
+  GTlsCertificate *issuer;
 
-  g_assert (anchors);
+  g_assert (anchor);
   g_assert (chain);
   g_assert (error);
   g_assert (!*error);
 
-  *chain = NULL;
-  *anchors = NULL;
-
-  /* Get the first certificate */
-  certificate = g_object_ref (G_TLS_CERTIFICATE_GNUTLS (input->data));
-  input = g_list_next (input);
+  /*
+   * Remember that the first certificate never changes in the chain.
+   * When we find a self-signed, pinned or anchored certificate, all
+   * issuers are truncated from the chain.
+   */
 
-  /* Add this to the output chain */
-  *chain = g_list_append (*chain, certificate);
+  *anchor = NULL;
+  certificate = chain;
 
   /* First check for pinned certificate */
   if (g_tls_database_gnutls_lookup_assertion (self, certificate,
                                               G_TLS_DATABASE_GNUTLS_PINNED_CERTIFICATE,
                                               identity, cancellable, error))
     {
+      g_tls_certificate_gnutls_set_issuer (certificate, NULL);
       return STATUS_PINNED;
     }
   else if(*error)
     {
-      *chain = list_unref_free (*chain);
       return STATUS_FAILURE;
     }
 
@@ -155,66 +154,61 @@ build_certificate_chain (GTlsDatabaseGnutls        *self,
     {
       /* Was the last certificate self-signed? */
       if (is_self_signed (certificate))
+        {
+          g_tls_certificate_gnutls_set_issuer (certificate, NULL);
           return STATUS_SELFSIGNED;
+        }
 
       /* Bring over the next certificate in the chain */
-      if (input)
+      issuer = g_tls_certificate_get_issuer (G_TLS_CERTIFICATE (certificate));
+      if (issuer)
         {
-          /* XXXX should we be skipping certs with wrong issuer here? */
-          certificate = g_object_ref (G_TLS_CERTIFICATE_GNUTLS (input->data));
-          input = g_list_next (input);
+          g_return_val_if_fail (G_IS_TLS_CERTIFICATE_GNUTLS (issuer), STATUS_FAILURE);
         }
 
-      /* No more in chain, try to lookup */
+      /* Search for the next certificate in chain */
       else
         {
-          certificate = lookup_best_issuer (self, certificate, cancellable, error);
+          issuer = lookup_best_issuer (self, certificate, cancellable, error);
           if (*error)
-            {
-              *chain = list_unref_free (*chain);
               return STATUS_FAILURE;
-            }
-          else if (!certificate)
-            {
+          else if (!issuer)
               return STATUS_INCOMPLETE;
-            }
+          g_return_val_if_fail (G_IS_TLS_CERTIFICATE_GNUTLS (issuer), STATUS_FAILURE);
+          g_tls_certificate_gnutls_set_issuer (certificate, G_TLS_CERTIFICATE_GNUTLS (issuer));
         }
 
+      g_assert (issuer);
+      certificate = G_TLS_CERTIFICATE_GNUTLS (issuer);
+
       /* Now look up whether this certificate is an anchor */
       if (g_tls_database_gnutls_lookup_assertion (self, certificate,
                                                   G_TLS_DATABASE_GNUTLS_ANCHORED_CERTIFICATE,
                                                   identity, cancellable, error))
         {
-          *anchors = g_list_append (NULL, certificate);
+          g_tls_certificate_gnutls_set_issuer (certificate, NULL);
           return STATUS_ANCHORED;
         }
       else if (error)
         {
-          g_object_unref (certificate);
-          *chain = list_unref_free (*chain);
           return STATUS_FAILURE;
         }
-      else
-        {
-          *chain = g_list_append (*chain, certificate);
-        }
     }
 
   g_assert_not_reached ();
 }
 
 static GTlsCertificateFlags
-double_check_before_after_dates (GList *chain)
+double_check_before_after_dates (GTlsCertificateGnutls *chain)
 {
   GTlsCertificateFlags gtls_flags = 0;
   gnutls_x509_crt_t cert;
   time_t t, now;
-  GList *l;
 
   now = time (NULL);
-  for (l = chain; l; l = g_list_next (l))
+  while (chain)
     {
-      cert = g_tls_certificate_gnutls_get_cert (l->data);
+      cert = g_tls_certificate_gnutls_get_cert (chain);
       t = gnutls_x509_crt_get_activation_time (cert);
       if (t == (time_t) -1 || t > now)
         gtls_flags |= G_TLS_CERTIFICATE_NOT_ACTIVATED;
@@ -222,114 +216,127 @@ double_check_before_after_dates (GList *chain)
       t = gnutls_x509_crt_get_expiration_time (cert);
       if (t == (time_t) -1 || t < now)
         gtls_flags |= G_TLS_CERTIFICATE_EXPIRED;
+
+      chain = G_TLS_CERTIFICATE_GNUTLS (g_tls_certificate_get_issuer
+                                        (G_TLS_CERTIFICATE (chain)));
     }
 
   return gtls_flags;
 }
 
 static void
-convert_certificate_list_to_gnutls (GList *chain,
-                                    gnutls_x509_crt_t **gnutls_chain,
-                                    guint *n_chain)
+convert_certificate_chain_to_gnutls (GTlsCertificateGnutls *chain,
+                                     gnutls_x509_crt_t **gnutls_chain,
+                                     guint *gnutls_chain_length)
 {
+  GTlsCertificate *cert;
   guint i;
 
   g_assert (gnutls_chain);
-  g_assert (n_chain);
+  g_assert (gnutls_chain_length);
 
-  *n_chain = g_list_length (chain);
-  if (*n_chain)
-    *gnutls_chain = g_new0 (gnutls_x509_crt_t, *n_chain);
-  else
-    *gnutls_chain = NULL;
+  for (*gnutls_chain_length = 0, cert = G_TLS_CERTIFICATE (chain);
+      cert; cert = g_tls_certificate_get_issuer (cert))
+    ++(*gnutls_chain_length);
+
+  *gnutls_chain = g_new0 (gnutls_x509_crt_t, *gnutls_chain_length);
+
+  for (i = 0, cert = G_TLS_CERTIFICATE (chain);
+      cert; cert = g_tls_certificate_get_issuer (cert), ++i)
+    (*gnutls_chain)[i] = g_tls_certificate_gnutls_get_cert (G_TLS_CERTIFICATE_GNUTLS (cert));
 
-  for (i = 0; i < *n_chain; ++i, chain = g_list_next (chain))
-    (*gnutls_chain)[i] = g_tls_certificate_gnutls_get_cert (chain->data);
+  g_assert (i == *gnutls_chain_length);
 }
 
-static GList*
-g_tls_database_gnutls_build_chain_and_verify (GTlsDatabase          *database,
-                                              GList                 *input,
-                                              GSocketConnectable    *identity,
-                                              gint                   flags,
-                                              GCancellable          *cancellable,
-                                              GTlsCertificateFlags  *verify_result,
-                                              GError               **error)
+static GTlsCertificateFlags
+g_tls_database_gnutls_verify_chain (GTlsDatabase           *database,
+                                    GTlsCertificate        *chain,
+                                    GSocketConnectable     *identity,
+                                    GTlsDatabaseVerifyFlags flags,
+                                    GCancellable           *cancellable,
+                                    GError                **error)
 {
   GTlsDatabaseGnutls *self;
-  gnutls_x509_crt_t *gnutls_chain;
-  gnutls_x509_crt_t *gnutls_anchors;
+  GTlsCertificateFlags result;
   GError *err = NULL;
-  GList *chain, *anchors;
+  GTlsCertificateGnutls *anchor;
   guint gnutls_result;
-  guint n_anchors, n_chain;
+  gnutls_x509_crt_t *certs, *anchors;
+  guint certs_length, anchors_length;
   gint status, gerr;
 
-  g_return_val_if_fail (G_IS_TLS_DATABASE_GNUTLS (database), NULL);
-  g_return_val_if_fail (input, NULL);
-  g_return_val_if_fail (G_IS_SOCKET_CONNECTABLE (identity), NULL);
-  g_return_val_if_fail (verify_result, NULL);
-  g_return_val_if_fail (!error || !*error, NULL);
+  g_return_val_if_fail (G_IS_TLS_DATABASE_GNUTLS (database),
+                        G_TLS_CERTIFICATE_GENERIC_ERROR);
+  g_return_val_if_fail (G_IS_TLS_CERTIFICATE_GNUTLS (chain),
+                        G_TLS_CERTIFICATE_GENERIC_ERROR);
+  g_return_val_if_fail (!identity || G_IS_SOCKET_CONNECTABLE (identity),
+                        G_TLS_CERTIFICATE_GENERIC_ERROR);
+  g_return_val_if_fail (!error || !*error, G_TLS_CERTIFICATE_GENERIC_ERROR);
 
   self = G_TLS_DATABASE_GNUTLS (database);
-  chain = anchors = NULL;
+  anchor = NULL;
 
-  status = build_certificate_chain (self, input, identity, flags, cancellable,
-                                    &chain, &anchors, &err);
-  if (err)
+  status = build_certificate_chain (self, G_TLS_CERTIFICATE_GNUTLS (chain), identity,
+                                    flags, cancellable, &anchor, &err);
+  if (status == STATUS_FAILURE)
     {
       g_propagate_error (error, err);
-      return NULL;
+      return G_TLS_CERTIFICATE_GENERIC_ERROR;
     }
 
-  /* A pinned certificate is verified on its own */
+  /*
+   * A pinned certificate is verified on its own, without any further
+   * verification.
+   */
   if (status == STATUS_PINNED)
+      return 0;
+
+  convert_certificate_chain_to_gnutls (G_TLS_CERTIFICATE_GNUTLS (chain),
+                                       &certs, &certs_length);
+
+  if (anchor)
     {
-      list_unref_free (anchors);
-      *verify_result = 0;
-      return chain;
+      g_assert (g_tls_certificate_get_issuer (G_TLS_CERTIFICATE (anchor)) == NULL);
+      convert_certificate_chain_to_gnutls (G_TLS_CERTIFICATE_GNUTLS (anchor),
+                                           &anchors, &anchors_length);
+    }
+  else
+    {
+      anchors = NULL;
+      anchors_length = 0;
     }
 
-  /* The certificate chain for validation by gnutls */
-  convert_certificate_list_to_gnutls (chain, &gnutls_chain, &n_chain);
-  convert_certificate_list_to_gnutls (anchors, &gnutls_anchors, &n_anchors);
-
-  gerr = gnutls_x509_crt_list_verify (gnutls_chain, n_chain,
-                                      gnutls_anchors, n_anchors,
+  gerr = gnutls_x509_crt_list_verify (certs, certs_length,
+                                      anchors, anchors_length,
                                       NULL, 0, GNUTLS_VERIFY_ALLOW_X509_V1_CA_CRT,
                                       &gnutls_result);
 
-  list_unref_free (anchors);
-  g_free (gnutls_chain);
-  g_free (gnutls_anchors);
+  g_free (certs);
+  g_free (anchors);
 
   if (gerr != 0)
-    {
-      g_free (gnutls_chain);
-      g_free (gnutls_anchors);
-      *verify_result = G_TLS_CERTIFICATE_GENERIC_ERROR;
-      return chain;
-    }
+      return G_TLS_CERTIFICATE_GENERIC_ERROR;
 
-  *verify_result = g_tls_certificate_gnutls_convert_flags (gnutls_result);
+  result = g_tls_certificate_gnutls_convert_flags (gnutls_result);
 
   /*
    * We have to check these ourselves since gnutls_x509_crt_list_verify
    * won't bother if it gets an UNKNOWN_CA.
    */
-  *verify_result |= double_check_before_after_dates (chain);
+  result |= double_check_before_after_dates (G_TLS_CERTIFICATE_GNUTLS (chain));
 
   if (identity)
-    *verify_result |= g_tls_certificate_gnutls_verify_identity (chain->data, identity);
+    result |= g_tls_certificate_gnutls_verify_identity (G_TLS_CERTIFICATE_GNUTLS (chain),
+                                                        identity);
 
-  return chain;
+  return result;
 }
 
 static void
 g_tls_database_gnutls_class_init (GTlsDatabaseGnutlsClass *klass)
 {
   GTlsDatabaseClass *database_class = G_TLS_DATABASE_CLASS (klass);
-  database_class->build_chain_and_verify = g_tls_database_gnutls_build_chain_and_verify;
+  database_class->verify_chain = g_tls_database_gnutls_verify_chain;
 }
 
 gboolean
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]