From d4e86dc91e1d8a940dc40872fe94ef9ac0fed1b5 Mon Sep 17 00:00:00 2001
From: Michael Gratton <mike@vee.net>
Date: Tue, 25 Aug 2020 03:54:09 +0000
Subject: [PATCH] Merge branch 'mjog/866-self-signed-certificates' into
 'mainline'

Fix invalid certificate pinning when GCR support is unavailable

Closes #866

See merge request GNOME/geary!529

(cherry picked from commit 423a55b00f1dc6bee9dc17e67c0aea6f42387a77)

5088adfe Application.CertificateManager: Rename some methods for clarity
0d957559 Application.CertificateManager: Check locally pinned certs for equality
---
 .../application-certificate-manager.vala      | 44 +++++++++----------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/src/client/application/application-certificate-manager.vala b/src/client/application/application-certificate-manager.vala
index 4881d73c0..65f6af4fa 100644
--- a/src/client/application/application-certificate-manager.vala
+++ b/src/client/application/application-certificate-manager.vala
@@ -381,8 +381,8 @@ private class Application.TlsDatabase : GLib.TlsDatabase {
         GLib.TlsCertificateFlags ret = this.parent.verify_chain(
             chain, purpose, identity, interaction, flags, cancellable
         );
-        if (should_verify(ret, purpose, identity) &&
-            verify(chain, identity, cancellable)) {
+        if (check_pinned(ret, purpose, identity) &&
+            is_pinned(chain, identity, cancellable)) {
             ret = 0;
         }
         return ret;
@@ -399,16 +399,16 @@ private class Application.TlsDatabase : GLib.TlsDatabase {
         GLib.TlsCertificateFlags ret = yield this.parent.verify_chain_async(
             chain, purpose, identity, interaction, flags, cancellable
         );
-        if (should_verify(ret, purpose, identity) &&
-            yield verify_async(chain, identity, cancellable)) {
+        if (check_pinned(ret, purpose, identity) &&
+            yield is_pinned_async(chain, identity, cancellable)) {
             ret = 0;
         }
         return ret;
     }
 
-    private inline bool should_verify(GLib.TlsCertificateFlags parent_ret,
-                                      string purpose,
-                                      GLib.SocketConnectable? identity) {
+    private inline bool check_pinned(GLib.TlsCertificateFlags parent_ret,
+                                     string purpose,
+                                     GLib.SocketConnectable? identity) {
         // If the parent didn't verify, check for a locally pinned
         // cert if it looks like we should, but always reject revoked
         // certs
@@ -420,22 +420,22 @@ private class Application.TlsDatabase : GLib.TlsDatabase {
         );
     }
 
-    private bool verify(GLib.TlsCertificate chain,
-                        GLib.SocketConnectable identity,
-                        GLib.Cancellable? cancellable)
+    private bool is_pinned(GLib.TlsCertificate chain,
+                           GLib.SocketConnectable identity,
+                           GLib.Cancellable? cancellable)
         throws GLib.Error {
-        bool is_verified = false;
+        bool is_pinned = false;
         string id = to_name(identity);
         TrustContext? context = null;
         lock (this.pinned_certs) {
             context = this.pinned_certs.get(id);
             if (context != null) {
-                is_verified = true;
+                is_pinned = context.certificate.is_same(chain);
             } else {
                 // Cert not found in memory, check with GCR if
                 // enabled.
                 if (this.use_gcr) {
-                    is_verified = gcr_trust_is_certificate_pinned(
+                    is_pinned = gcr_trust_is_certificate_pinned(
                         new Gcr.SimpleCertificate(chain.certificate.data),
                         GLib.TlsDatabase.PURPOSE_AUTHENTICATE_SERVER,
                         id,
@@ -443,7 +443,7 @@ private class Application.TlsDatabase : GLib.TlsDatabase {
                     );
                 }
 
-                if (!is_verified) {
+                if (!is_pinned) {
                     // Cert is not pinned in memory or in GCR, so look
                     // for it on disk. Do this even if GCR support is
                     // enabled, since if the cert was previously saved
@@ -453,7 +453,7 @@ private class Application.TlsDatabase : GLib.TlsDatabase {
                             this.store_dir, id, cancellable
                         );
                         this.pinned_certs.set(id, context);
-                        is_verified = true;
+                        is_pinned = context.certificate.is_same(chain);
                     } catch (GLib.IOError.NOT_FOUND err) {
                         // Cert was not found saved, so it not pinned
                     } catch (GLib.Error err) {
@@ -465,18 +465,18 @@ private class Application.TlsDatabase : GLib.TlsDatabase {
                 }
             }
         }
-        return is_verified;
+        return is_pinned;
     }
 
-    private async bool verify_async(GLib.TlsCertificate chain,
-                                    GLib.SocketConnectable identity,
-                                    GLib.Cancellable? cancellable)
+    private async bool is_pinned_async(GLib.TlsCertificate chain,
+                                       GLib.SocketConnectable identity,
+                                       GLib.Cancellable? cancellable)
         throws GLib.Error {
-        bool is_valid = false;
+        bool pinned = false;
         yield Geary.Nonblocking.Concurrent.global.schedule_async(() => {
-                is_valid = verify(chain, identity, cancellable);
+                pinned = is_pinned(chain, identity, cancellable);
             }, cancellable);
-        return is_valid;
+        return pinned;
     }
 
     private TrustContext? lookup_id(string id) {
-- 
GitLab