[geary/wip/775730-long-delay-sent-mail] Fix long delay sending mail when using pinned SMTP server cert. Bug 775730



commit 872e393276456d56ba43964d3e98a76668659fdf
Author: Michael James Gratton <mike vee net>
Date:   Thu Dec 8 17:30:21 2016 +1100

    Fix long delay sending mail when using pinned SMTP server cert. Bug 775730
    
    Since glib-networking actually integrates with the p11f-kit
    infrastructure for the default GNU TLS backend, if we just set the GCR
    peer name correctly when pinning a certificate, it'll automatically check
    for pinned certs for us.
    
    This lets us remove the code currently checking for a pinned cert in the
    main loop that was causing a re-connect when encountering a pinned cert,
    but at the cost of no longer being able to unpin certs from the app any
    more. This is a tradeoff I'm happy with.
    
    * src/client/application/geary-controller.vala (GearyController): Use new
      GcrService class for managing cert pinning. Remove check for pinned
      cert on TLS error and (possibly sketchy anyway) code unpin existing
      certs.
    
    * src/client/application/gcr-service.vala: New class for managing GCR
      code in the one place.
    
    * src/client/application/geary-args.vala (Args): Remobe the revoke-certs
      arg and global var.
    
    * src/CMakeLists.txt: Add new source file.

 src/CMakeLists.txt                           |    1 +
 src/client/application/gcr-service.vala      |   77 +++++++++++++++++++++
 src/client/application/geary-args.vala       |    2 -
 src/client/application/geary-controller.vala |   96 +++++++-------------------
 4 files changed, 102 insertions(+), 74 deletions(-)
---
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index bc8d728..cf0b268 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -312,6 +312,7 @@ ${CMAKE_BINARY_DIR}/geary-version.vala
 
 set(CLIENT_SRC
 client/application/autostart-manager.vala
+client/application/gcr-service.vala
 client/application/geary-application.vala
 client/application/geary-args.vala
 client/application/geary-config.vala
diff --git a/src/client/application/gcr-service.vala b/src/client/application/gcr-service.vala
new file mode 100644
index 0000000..40794fa
--- /dev/null
+++ b/src/client/application/gcr-service.vala
@@ -0,0 +1,77 @@
+/*
+ * Copyright 2016 Michael Gratton <mike vee net>
+ *
+ * This software is licensed under the GNU Lesser General Public License
+ * (version 2.1 or later). See the COPYING file in this distribution.
+ */
+
+// Required because Gcr's VAPI is behind-the-times
+// TODO: When bindings available, use async variants of these calls
+extern const string GCR_PURPOSE_SERVER_AUTH;
+extern async bool gcr_trust_add_pinned_certificate_async(Gcr.Certificate cert, string purpose, string peer,
+    Cancellable? cancellable) throws Error;
+extern async bool gcr_trust_is_certificate_pinned_async(Gcr.Certificate cert, string purpose, string peer,
+    Cancellable? cancellable) throws Error;
+extern async bool gcr_trust_remove_pinned_certificate_async(Gcr.Certificate cert, string purpose, string 
peer,
+    Cancellable? cancellable) throws Error;
+
+/**
+ * Accesses desktop-wide TLS certificate store using GCR.
+ */
+public class GcrService: Geary.BaseObject {
+
+    /**
+     * Determines if a TLS certificate as trusted for a specific host.
+     */
+    public async bool is_trusted(Geary.Endpoint target,
+                                 TlsCertificate test_cert,
+                                 Cancellable? cancellable = null)
+    throws Error {
+        Gcr.Certificate cert = new Gcr.SimpleCertificate(
+            test_cert.certificate.data
+        );
+        string peer = to_gcr_peer(target);
+        return yield gcr_trust_is_certificate_pinned_async(
+            cert, GCR_PURPOSE_SERVER_AUTH, peer, cancellable
+        );
+    }
+
+    /**
+     * Marks a TLS certificate as trusted by pinning it.
+     */
+    public async void add_pinned(Geary.Endpoint target,
+                                 TlsCertificate trusted_cert,
+                                 Cancellable? cancellable = null)
+    throws Error {
+        Gcr.Certificate cert = new Gcr.SimpleCertificate(
+            trusted_cert.certificate.data
+        );
+        string peer = to_gcr_peer(target);
+        yield gcr_trust_add_pinned_certificate_async(
+            cert, GCR_PURPOSE_SERVER_AUTH, peer, cancellable
+        );
+    }
+
+    /**
+     *  Unmarks a TLS certificate as trusted by unpinning it.
+     */
+    public async void remove_pinned(Geary.Endpoint target,
+                                    TlsCertificate untrusted_cert,
+                                    Cancellable? cancellable = null)
+    throws Error {
+        Gcr.Certificate cert = new Gcr.SimpleCertificate(
+            untrusted_cert.certificate.data
+        );
+        string peer = to_gcr_peer(target);
+        yield gcr_trust_remove_pinned_certificate_async(
+            cert, GCR_PURPOSE_SERVER_AUTH, peer, cancellable
+        );
+    }
+
+    private inline string to_gcr_peer(Geary.Endpoint host) {
+        // The default GIO GNUTLS backend uses the remote address
+        // hostname as the peer, so we need to here as well
+        return host.remote_address.get_hostname();
+    }
+
+}
diff --git a/src/client/application/geary-args.vala b/src/client/application/geary-args.vala
index 99abf9e..770a8bc 100644
--- a/src/client/application/geary-args.vala
+++ b/src/client/application/geary-args.vala
@@ -23,7 +23,6 @@ private const OptionEntry[] options = {
     /// "Normalization" can also be called "synchronization"
     { "log-folder-normalization", 0, 0, OptionArg.NONE, ref log_folder_normalization, N_("Log folder 
normalization"), null },
     { "inspector", 'i', 0, OptionArg.NONE, ref inspector, N_("Allow inspection of WebView"), null },
-    { "revoke-certs", 0, 0, OptionArg.NONE, ref revoke_certs, N_("Revoke all server certificates with TLS 
warnings"), null },
     { "quit", 'q', 0, OptionArg.NONE, ref quit, N_("Perform a graceful quit"), null },
     { "version", 'V', 0, OptionArg.NONE, ref version, N_("Display program version"), null },
     { null }
@@ -41,7 +40,6 @@ public bool log_sql = false;
 public bool log_folder_normalization = false;
 public bool inspector = false;
 public bool quit = false;
-public bool revoke_certs = false;
 public bool version = false;
 
 public bool parse(string[] args) {
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 03e2383..1e3db8e 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -6,16 +6,6 @@
  * (version 2.1 or later). See the COPYING file in this distribution.
  */
 
-// Required because Gcr's VAPI is behind-the-times
-// TODO: When bindings available, use async variants of these calls
-extern const string GCR_PURPOSE_SERVER_AUTH;
-extern bool gcr_trust_add_pinned_certificate(Gcr.Certificate cert, string purpose, string peer,
-    Cancellable? cancellable) throws Error;
-extern bool gcr_trust_is_certificate_pinned(Gcr.Certificate cert, string purpose, string peer,
-    Cancellable? cancellable) throws Error;
-extern bool gcr_trust_remove_pinned_certificate(Gcr.Certificate cert, string purpose, string peer,
-    Cancellable? cancellable) throws Error;
-
 /**
  * Primary controller for a Geary application instance.
  */
@@ -97,6 +87,8 @@ public class GearyController : Geary.BaseObject {
     public Soup.Session? avatar_session { get; private set; default = null; }
     private Soup.Cache? avatar_cache = null;
 
+    private GcrService certificate_store = new GcrService();
+
     private Geary.Account? current_account = null;
     private Gee.HashMap<Geary.Account, Geary.App.EmailStore> email_stores
         = new Gee.HashMap<Geary.Account, Geary.App.EmailStore>();
@@ -659,61 +651,20 @@ public class GearyController : Geary.BaseObject {
                 err.message);
         }
     }
-    
-    private static void get_gcr_params(Geary.Endpoint endpoint, out Gcr.Certificate cert,
-        out string peer) {
-        cert = new Gcr.SimpleCertificate(endpoint.untrusted_certificate.certificate.data);
-        peer = "%s:%u".printf(endpoint.remote_address.hostname, endpoint.remote_address.port);
-    }
-    
+
     private async void locked_prompt_untrusted_host_async(Geary.AccountInformation account_information,
         Geary.Endpoint endpoint, Geary.Endpoint.SecurityType security, TlsConnection cx,
         Geary.Service service) {
         // possible while waiting on mutex that this endpoint became trusted or untrusted
         if (endpoint.trust_untrusted_host != Geary.Trillian.UNKNOWN)
             return;
-        
-        // get GCR parameters
-        Gcr.Certificate cert;
-        string peer;
-        get_gcr_params(endpoint, out cert, out peer);
-        
-        // Geary allows for user to auto-revoke all questionable server certificates without
-        // digging around in a keyring/pk manager
-        if (Args.revoke_certs) {
-            debug("Auto-revoking certificate for %s...", peer);
-            
-            try {
-                gcr_trust_remove_pinned_certificate(cert, GCR_PURPOSE_SERVER_AUTH, peer, null);
-            } catch (Error err) {
-                message("Unable to auto-revoke server certificate for %s: %s", peer, err.message);
-                
-                // drop through, not absolutely valid to do this (might also mean certificate
-                // was never pinned)
-            }
-        }
-        
-        // if pinned, the user has already made an exception for this server and its certificate,
-        // so go ahead w/o asking
-        try {
-            if (gcr_trust_is_certificate_pinned(cert, GCR_PURPOSE_SERVER_AUTH, peer, null)) {
-                debug("Certificate for %s is pinned, accepting connection...", peer);
-                
-                endpoint.trust_untrusted_host = Geary.Trillian.TRUE;
-                
-                return;
-            }
-        } catch (Error err) {
-            message("Unable to check if server certificate for %s is pinned, assuming not: %s",
-                peer, err.message);
-        }
-        
+
         // if these are in validation, there are complex GTK and workflow issues from simply
         // presenting the prompt now, so caller who connected will need to do it on their own dime
         if (!validating_endpoints.contains(endpoint))
             prompt_for_untrusted_host(main_window, account_information, endpoint, service, false);
     }
-    
+
     private void prompt_for_untrusted_host(Gtk.Window? parent, Geary.AccountInformation account_information,
         Geary.Endpoint endpoint, Geary.Service service, bool is_validation) {
         CertificateWarningDialog dialog = new CertificateWarningDialog(parent, account_information,
@@ -722,27 +673,28 @@ public class GearyController : Geary.BaseObject {
             case CertificateWarningDialog.Result.TRUST:
                 endpoint.trust_untrusted_host = Geary.Trillian.TRUE;
             break;
-            
+
             case CertificateWarningDialog.Result.ALWAYS_TRUST:
                 endpoint.trust_untrusted_host = Geary.Trillian.TRUE;
-                
-                // get GCR parameters for pinning
-                Gcr.Certificate cert;
-                string peer;
-                get_gcr_params(endpoint, out cert, out peer);
-                
-                // pinning the certificate creates an exception for the next time a connection
-                // is attempted
-                debug("Pinning certificate for %s...", peer);
-                try {
-                    gcr_trust_add_pinned_certificate(cert, GCR_PURPOSE_SERVER_AUTH, peer, null);
-                } catch (Error err) {
-                    ErrorDialog error_dialog = new ErrorDialog(main_window,
-                        _("Unable to store server trust exception"), err.message);
-                    error_dialog.run();
-                }
+
+                debug("Pinning certificate for %s...", endpoint.to_string());
+                this.certificate_store.add_pinned.begin(
+                    endpoint,
+                    endpoint.untrusted_certificate,
+                    null,
+                    (obj, res) => {
+                        try {
+                            this.certificate_store.add_pinned.end(res);
+                        } catch (Error err) {
+                            ErrorDialog error_dialog = new ErrorDialog(
+                                this.main_window,
+                                _("Unable to store server trust exception"),
+                                err.message);
+                            error_dialog.run();
+                        }
+                    });
             break;
-            
+
             default:
                 endpoint.trust_untrusted_host = Geary.Trillian.FALSE;
                 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]