File U_0001-Properly-store-certificate-exceptions.patch of Package tigervnc

Overview Repositories Revisions Requests Users Attributes Meta

File U_0001-Properly-store-certificate-exceptions.patch of Package tigervnc (Revision bbe7bac6691fe180d605b9e67a352ce0)

Currently displaying revision bbe7bac6691fe180d605b9e67a352ce0 , Show latest

From b30f10c681ec87720cff85d490f67098568a9cba Mon Sep 17 00:00:00 2001
From: Pierre Ossman <ossman@cendio.se>
Date: Thu, 21 May 2020 21:10:38 +0200
Subject: [PATCH] Properly store certificate exceptions

The previous method stored the certificates as authorities, meaning that
the owner of that certificate could impersonate any server it wanted
after a client had added an exception.

Handle this more properly by only storing exceptions for specific
hostname/certificate combinations, the same way browsers or SSH does
things.
---
 common/rfb/CSecurityTLS.cxx | 163 ++++++++++++++++++++------------------------
 1 file changed, 73 insertions(+), 90 deletions(-)

diff --git a/common/rfb/CSecurityTLS.cxx b/common/rfb/CSecurityTLS.cxx
index 5c303a37..99008378 100644
--- a/common/rfb/CSecurityTLS.cxx
+++ b/common/rfb/CSecurityTLS.cxx
@@ -250,22 +250,6 @@ void CSecurityTLS::setParam()
     if (*cafile && gnutls_certificate_set_x509_trust_file(cert_cred,cafile,GNUTLS_X509_FMT_PEM) < 0)
       throw AuthFailureException("load of CA cert failed");
 
-    /* Load previously saved certs */
-    char *homeDir = NULL;
-    int err;
-    if (getvnchomedir(&homeDir) == -1)
-      vlog.error("Could not obtain VNC home directory path");
-    else {
-      CharArray caSave(strlen(homeDir) + 19 + 1);
-      sprintf(caSave.buf, "%sx509_savedcerts.pem", homeDir);
-      delete [] homeDir;
-
-      err = gnutls_certificate_set_x509_trust_file(cert_cred, caSave.buf,
-                                                   GNUTLS_X509_FMT_PEM);
-      if (err < 0)
-        vlog.debug("Failed to load saved server certificates from %s", caSave.buf);
-    }
-
     if (*crlfile && gnutls_certificate_set_x509_crl_file(cert_cred,crlfile,GNUTLS_X509_FMT_PEM) < 0)
       throw AuthFailureException("load of CRL failed");
 
@@ -290,7 +274,10 @@ void CSecurityTLS::checkSession()
   const gnutls_datum_t *cert_list;
   unsigned int cert_list_size = 0;
   int err;
+
+  char *homeDir;
   gnutls_datum_t info;
+  size_t len;
 
   if (anon)
     return;
@@ -333,13 +320,13 @@ void CSecurityTLS::checkSession()
     throw AuthFailureException("decoding of certificate failed");
 
   if (gnutls_x509_crt_check_hostname(crt, client->getServerName()) == 0) {
-    char buf[255];
+    CharArray text;
     vlog.debug("hostname mismatch");
-    snprintf(buf, sizeof(buf), "Hostname (%s) does not match any certificate, "
-			       "do you want to continue?", client->getServerName());
-    buf[sizeof(buf) - 1] = '\0';
-    if (!msg->showMsgBox(UserMsgBox::M_YESNO, "hostname mismatch", buf))
-      throw AuthFailureException("hostname mismatch");
+    text.format("Hostname (%s) does not match the server certificate, "
+                "do you want to continue?", client->getServerName());
+    if (!msg->showMsgBox(UserMsgBox::M_YESNO,
+                         "Certificate hostname mismatch", text.buf))
+      throw AuthFailureException("Certificate hostname mismatch");
   }
 
   if (status == 0) {
@@ -364,86 +351,82 @@ void CSecurityTLS::checkSession()
     throw AuthFailureException("Invalid status of server certificate verification");
   }
 
-  vlog.debug("Saved server certificates don't match");
+  /* Certificate is fine, except we don't know the issuer, so TOFU time */
 
-  if (gnutls_x509_crt_print(crt, GNUTLS_CRT_PRINT_ONELINE, &info)) {
-    /*
-     * GNUTLS doesn't correctly export gnutls_free symbol which is
-     * a function pointer. Linking with Visual Studio 2008 Express will
-     * fail when you call gnutls_free().
-     */
-#if WIN32
-    free(info.data);
-#else
-    gnutls_free(info.data);
-#endif
-    throw AuthFailureException("Could not find certificate to display");
+  homeDir = NULL;
+  if (getvnchomedir(&homeDir) == -1) {
+    throw AuthFailureException("Could not obtain VNC home directory "
+                               "path for known hosts storage");
   }
 
-  size_t out_size = 0;
-  char *out_buf = NULL;
-  char *certinfo = NULL;
-  int len = 0;
-
-  vlog.debug("certificate issuer unknown");
-
-  len = snprintf(NULL, 0, "This certificate has been signed by an unknown "
-                          "authority:\n\n%s\n\nDo you want to save it and "
-                          "continue?\n ", info.data);
-  if (len < 0)
-    throw AuthFailureException("certificate decoding error");
-
-  vlog.debug("%s", info.data);
-
-  certinfo = new char[len];
-
-  snprintf(certinfo, len, "This certificate has been signed by an unknown "
-                          "authority:\n\n%s\n\nDo you want to save it and "
-                          "continue? ", info.data);
+  CharArray dbPath(strlen(homeDir) + 16 + 1);
+  sprintf(dbPath.buf, "%sx509_known_hosts", homeDir);
+  delete [] homeDir;
 
-  for (int i = 0; i < len - 1; i++)
-    if (certinfo[i] == ',' && certinfo[i + 1] == ' ')
-      certinfo[i] = '\n';
+  err = gnutls_verify_stored_pubkey(dbPath.buf, NULL,
+                                    client->getServerName(), NULL,
+                                    GNUTLS_CRT_X509, &cert_list[0], 0);
 
-  if (!msg->showMsgBox(UserMsgBox::M_YESNO, "certificate issuer unknown",
-		       certinfo)) {
-    delete [] certinfo;
-    throw AuthFailureException("certificate issuer unknown");
+  /* Previously known? */
+  if (err == GNUTLS_E_SUCCESS) {
+    vlog.debug("Server certificate found in known hosts file");
+    gnutls_x509_crt_deinit(crt);
+    return;
   }
 
-  delete [] certinfo;
-
-  if (gnutls_x509_crt_export(crt, GNUTLS_X509_FMT_PEM, NULL, &out_size)
-      != GNUTLS_E_SHORT_MEMORY_BUFFER)
-    throw AuthFailureException("certificate issuer unknown, and certificate "
-                               "export failed");
+  if ((err != GNUTLS_E_NO_CERTIFICATE_FOUND) &&
+      (err != GNUTLS_E_CERTIFICATE_KEY_MISMATCH)) {
+    throw AuthFailureException("Could not load known hosts database");
+  }
 
-  // Save cert
-  out_buf =  new char[out_size];
+  if (gnutls_x509_crt_print(crt, GNUTLS_CRT_PRINT_ONELINE, &info))
+    throw AuthFailureException("Could not find certificate to display");
 
-  if (gnutls_x509_crt_export(crt, GNUTLS_X509_FMT_PEM, out_buf, &out_size) < 0)
-    throw AuthFailureException("certificate issuer unknown, and certificate "
-                               "export failed");
+  len = strlen((char*)info.data);
+  for (size_t i = 0; i < len - 1; i++) {
+    if (info.data[i] == ',' && info.data[i + 1] == ' ')
+      info.data[i] = '\n';
+  }
 
-  char *homeDir = NULL;
-  if (getvnchomedir(&homeDir) == -1)
-    vlog.error("Could not obtain VNC home directory path");
-  else {
-    FILE *f;
-    CharArray caSave(strlen(homeDir) + 1 + 19);
-    sprintf(caSave.buf, "%sx509_savedcerts.pem", homeDir);
-    delete [] homeDir;
-    f = fopen(caSave.buf, "a+");
-    if (!f)
-      msg->showMsgBox(UserMsgBox::M_OK, "certificate save failed",
-                      "Could not save the certificate");
-    else {
-      fprintf(f, "%s\n", out_buf);
-      fclose(f);
-    }
+  /* New host */
+  if (err == GNUTLS_E_NO_CERTIFICATE_FOUND) {
+    CharArray text;
+
+    vlog.debug("Server host not previously known");
+    vlog.debug("%s", info.data);
+
+    text.format("This certificate has been signed by an unknown "
+                "authority:\n\n%s\n\nSomeone could be trying to "
+                "impersonate the site and you should not "
+                "continue.\n\nDo you want to make an exception "
+                "for this server?", info.data);
+
+    if (!msg->showMsgBox(UserMsgBox::M_YESNO,
+                         "Unknown certificate issuer",
+                         text.buf))
+      throw AuthFailureException("Unknown certificate issuer");
+  } else if (err == GNUTLS_E_CERTIFICATE_KEY_MISMATCH) {
+    CharArray text;
+
+    vlog.debug("Server host key mismatch");
+    vlog.debug("%s", info.data);
+
+    text.format("This host is previously known with a different "
+                "certificate, and the new certificate has been "
+                "signed by an unknown authority:\n\n%s\n\nSomeone "
+                "could be trying to impersonate the site and you "
+                "should not continue.\n\nDo you want to make an "
+                "exception for this server?", info.data);
+
+    if (!msg->showMsgBox(UserMsgBox::M_YESNO,
+                         "Unexpected server certificate",
+                         text.buf))
+      throw AuthFailureException("Unexpected server certificate");
   }
 
-  delete [] out_buf;
+  if (gnutls_store_pubkey(dbPath.buf, NULL, client->getServerName(),
+                          NULL, GNUTLS_CRT_X509, &cert_list[0], 0, 0))
+    vlog.error("Failed to store server certificate to known hosts database");
 
   gnutls_x509_crt_deinit(crt);
   /*
-- 
2.16.4

Places

File U_0001-Properly-store-certificate-exceptions.patch of Package tigervnc (Revision bbe7bac6691fe180d605b9e67a352ce0)

Places