[geary/wip/775730-long-delay-sent-mail] Fix long delay sending mail when using pinned SMTP server cert. Bug 775730
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/775730-long-delay-sent-mail] Fix long delay sending mail when using pinned SMTP server cert. Bug 775730
- Date: Thu, 8 Dec 2016 06:32:16 +0000 (UTC)
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]