CVE-2020-26117: Improper Certificate Validation (CWE-295)
value: In rfb/CSecurityTLS.cxx and rfb/CSecurityTLS.java in TigerVNC before 1.11.0, viewers mishandle TLS certificate exceptions. They store the certificates as authorities, meaning that the owner of a certificate could impersonate any server after a client had added an exception.
CWE description: The product does not validate, or incorrectly validates, a certificate.
CVSS v3 base score: 8.1
Published: 2020-09-27T04:15Z
Repository: https://github.com/TigerVNC/tigervnc
Language: Java
Files changed: java/com/tigervnc/rfb/CSecurityTLS.java
Commit date: 2020-09-08 10:13:32 +0200
Commit message: Properly store certificate exceptions in Java viewer

Like the native viewer, the Java viewer didn't store certificate
exceptions properly. Whilst not as bad as the native viewer, it still
failed to check that a stored certificate wouldn't be maliciously used
for another server. In practice this can in most cases be used to
impersonate another server.

Handle this like the native viewer by storing exceptions for a specific
hostname/certificate combination.

Diff: diff --git a/java/com/tigervnc/rfb/CSecurityTLS.java b/java/com/tigervnc/rfb/CSecurityTLS.java
index ad6f6fe1d..e63945dc2 100644
--- a/java/com/tigervnc/rfb/CSecurityTLS.java
+++ b/java/com/tigervnc/rfb/CSecurityTLS.java
@@ -105,16 +105,10 @@ public class CSecurityTLS extends CSecurity {
       X509CA.setDefaultStr(getDefaultCA());
     if (new File(getDefaultCRL()).exists())
       X509CRL.setDefaultStr(getDefaultCRL());
   }
 
-// FIXME:
-// Need to shutdown the connection cleanly
-
-// FIXME?
-// add a finalizer method that calls shutdown
-
   public boolean processMsg(CConnection cc) {
     is = (FdInStream)cc.getInStream();
     os = (FdOutStream)cc.getOutStream();
     client = cc;
 
@@ -267,87 +261,130 @@ public class CSecurityTLS extends CSecurity {
     public void checkServerTrusted(X509Certificate[] chain, String authType)
       throws CertificateException
     {
       Collection<? extends Certificate> certs = null;
       X509Certificate cert = chain[0];
+      String pk =
+        Base64.getEncoder().encodeToString(cert.getPublicKey().getEncoded());
       try {
         cert.checkValidity();
+        verifyHostname(cert);
+      } catch(CertificateParsingException e) {
+        throw new SystemException(e.getMessage());
       } catch(CertificateNotYetValidException e) {
         throw new AuthFailureException("server certificate has not been activated");
       } catch(CertificateExpiredException e) {
         if (!msg.showMsgBox(YES_NO_OPTION, "certificate has expired",
 			      "The certificate of the server has expired, "+
 			      "do you want to continue?"))
           throw new AuthFailureException("server certificate has expired");
       }
-      String thumbprint = getThumbprint(cert);
       File vncDir = new File(FileUtils.getVncHomeDir());
-      File certFile = new File(vncDir, "x509_savedcerts.pem");
-      CertificateFactory cf = CertificateFactory.getInstance("X.509");
-      if (vncDir.exists() && certFile.exists() && certFile.canRead()) {
-        InputStream certStream = new MyFileInputStream(certFile);
-        certs = cf.generateCertificates(certStream);
-        for (Certificate c : certs)
-          if (thumbprint.equals(getThumbprint((X509Certificate)c)))
-            return;
-      }
+      if (!vncDir.exists())
+        throw new AuthFailureException("Could not obtain VNC home directory "+
+                                       "path for known hosts storage");
+      File dbPath = new File(vncDir, "x509_known_hosts");
+      String info =
+        "  Subject: "+cert.getSubjectX500Principal().getName()+"\n"+
+        "  Issuer: "+cert.getIssuerX500Principal().getName()+"\n"+
+        "  Serial Number: "+cert.getSerialNumber()+"\n"+
+        "  Version: "+cert.getVersion()+"\n"+
+        "  Signature Algorithm: "+cert.getPublicKey().getAlgorithm()+"\n"+
+        "  Not Valid Before: "+cert.getNotBefore()+"\n"+
+        "  Not Valid After: "+cert.getNotAfter()+"\n"+
+        "  SHA-1 Fingerprint: "+getThumbprint(cert)+"\n";
       try {
-        verifyHostname(cert);
+        if (dbPath.exists()) {
+          FileReader db = new FileReader(dbPath);
+          BufferedReader dbBuf = new BufferedReader(db);
+          String line;
+          String server = client.getServerName().toLowerCase();
+          while ((line = dbBuf.readLine())!=null) {
+            String fields[] = line.split("\\|");
+            if (fields.length==6) {
+              if (server.equals(fields[2]) && pk.equals(fields[5])) {
+                vlog.debug("Server certificate found in known hosts file");
+                dbBuf.close();
+                return;
+              } else if (server.equals(fields[2]) && !pk.equals(fields[5]) ||
+                         !server.equals(fields[2]) && pk.equals(fields[5])) {
+                throw new CertStoreException();
+              }
+            }
+          }
+          dbBuf.close();
+        }
         tm.checkServerTrusted(chain, authType);
+      } catch (IOException e) {
+        throw new AuthFailureException("Could not load known hosts database");
+      } catch (CertStoreException e) {
+        vlog.debug("Server host key mismatch");
+        vlog.debug(info);
+        String text =
+          "This host is previously known with a different "+
+          "certificate, and the new certificate has been "+
+          "signed by an unknown authority\n"+
+          "\n"+info+"\n"+
+          "Someone could be trying to impersonate the site and you should not continue.\n"+
+          "\n"+
+          "Do you want to make an exception for this server?";
+        if (!msg.showMsgBox(YES_NO_OPTION, "Unexpected certificate issuer", text))
+          throw new AuthFailureException("Unexpected certificate issuer");
+        store_pubkey(dbPath, client.getServerName().toLowerCase(), pk);
       } catch (java.lang.Exception e) {
         if (e.getCause() instanceof CertPathBuilderException) {
-          String certinfo =
+          vlog.debug("Server host not previously known");
+          vlog.debug(info);
+          String text =
             "This certificate has been signed by an unknown authority\n"+
+            "\n"+info+"\n"+
+            "Someone could be trying to impersonate the site and you should not continue.\n"+
             "\n"+
-            "  Subject: "+cert.getSubjectX500Principal().getName()+"\n"+
-            "  Issuer: "+cert.getIssuerX500Principal().getName()+"\n"+
-            "  Serial Number: "+cert.getSerialNumber()+"\n"+
-            "  Version: "+cert.getVersion()+"\n"+
-            "  Signature Algorithm: "+cert.getPublicKey().getAlgorithm()+"\n"+
-            "  Not Valid Before: "+cert.getNotBefore()+"\n"+
-            "  Not Valid After: "+cert.getNotAfter()+"\n"+
-            "  SHA1 Fingerprint: "+getThumbprint(cert)+"\n"+
-            "\n"+
-            "Do you want to save it and continue?";
-          if (!msg.showMsgBox(YES_NO_OPTION, "certificate issuer unknown",
-                certinfo)) {
-            throw new AuthFailureException("certificate issuer unknown");
-          }
-          if (certs == null || !certs.contains(cert)) {
-            byte[] der = cert.getEncoded();
-            String pem = Base64.getEncoder().encodeToString(der);
-            pem = pem.replaceAll("(.{64})", "$1\n");
-            FileWriter fw = null;
-            try {
-              if (!vncDir.exists())
-                vncDir.mkdir();
-              if (!certFile.exists() && !certFile.createNewFile()) {
-                vlog.error("Certificate save failed.");
-              } else {
-                fw = new FileWriter(certFile.getAbsolutePath(), true);
-                fw.write("-----BEGIN CERTIFICATE-----\n");
-                fw.write(pem+"\n");
-                fw.write("-----END CERTIFICATE-----\n");
-              }
-            } catch (IOException ioe) {
-              msg.showMsgBox(OK_OPTION, "certificate save failed",
-                             "Could not save the certificate");
-            } finally {
-              try {
-                if (fw != null)
-                  fw.close();
-              } catch(IOException ioe2) {
-                throw new Exception(ioe2.getMessage());
-              }
-            }
-          }
+            "Do you want to make an exception for this server?";
+          if (!msg.showMsgBox(YES_NO_OPTION, "Unknown certificate issuer", text))
+            throw new AuthFailureException("Unknown certificate issuer");
+          store_pubkey(dbPath, client.getServerName().toLowerCase(), pk);
         } else {
           throw new SystemException(e.getMessage());
         }
       }
     }
 
+    private void store_pubkey(File dbPath, String serverName, String pk)
+    {
+      ArrayList<String> lines = new ArrayList<String>();
+      File vncDir = new File(FileUtils.getVncHomeDir());
+      try {
+        if (dbPath.exists()) {
+          FileReader db = new FileReader(dbPath);
+          BufferedReader dbBuf = new BufferedReader(db);
+          String line;
+          while ((line = dbBuf.readLine())!=null) {
+            String fields[] = line.split("\\|");
+            if (fields.length==6)
+              if (!serverName.equals(fields[2]) && !pk.equals(fields[5]))
+                lines.add(line);
+          }
+          dbBuf.close();
+        }
+      } catch (IOException e) {
+        throw new AuthFailureException("Could not load known hosts database");
+      }
+      try {
+        if (!dbPath.exists())
+          dbPath.createNewFile();
+        FileWriter fw = new FileWriter(dbPath.getAbsolutePath(), false);
+        Iterator i = lines.iterator();
+        while (i.hasNext())
+          fw.write((String)i.next()+"\n");
+        fw.write("|g0|"+serverName+"|*|0|"+pk+"\n");
+        fw.close();
+      } catch (IOException e) {
+        vlog.error("Failed to store server certificate to known hosts database");
+      }
+    }
+
     public X509Certificate[] getAcceptedIssuers ()
     {
       return tm.getAcceptedIssuers();
     }
 
@@ -397,16 +434,17 @@ public class CSecurityTLS extends CSecurity {
             }
           }
         }
         Object[] answer = {"YES", "NO"};
         int ret = JOptionPane.showOptionDialog(null,
-          "Hostname verification failed. Do you want to continue?",
-          "Hostname Verification Failure",
+          "Hostname ("+client.getServerName()+") does not match the"+
+          " server certificate, do you want to continue?",
+          "Certificate hostname mismatch",
           JOptionPane.YES_NO_OPTION, JOptionPane.WARNING_MESSAGE,
           null, answer, answer[0]);
         if (ret != JOptionPane.YES_OPTION)
-          throw new WarningException("Hostname verification failed.");
+          throw new WarningException("Certificate hostname mismatch.");
       } catch (CertificateParsingException e) {
         throw new SystemException(e.getMessage());
       } catch (InvalidNameException e) {
         throw new SystemException(e.getMessage());
       }

Vulnerable code: // FIXME:
// Need to shutdown the connection cleanly

// FIXME?
// add a finalizer method that calls shutdown

      String thumbprint = getThumbprint(cert);
      File certFile = new File(vncDir, "x509_savedcerts.pem");
      CertificateFactory cf = CertificateFactory.getInstance("X.509");
      if (vncDir.exists() && certFile.exists() && certFile.canRead()) {
        InputStream certStream = new MyFileInputStream(certFile);
        certs = cf.generateCertificates(certStream);
        for (Certificate c : certs)
          if (thumbprint.equals(getThumbprint((X509Certificate)c)))
            return;
      }
        verifyHostname(cert);
          String certinfo =
            "  Subject: "+cert.getSubjectX500Principal().getName()+"\n"+
            "  Issuer: "+cert.getIssuerX500Principal().getName()+"\n"+
            "  Serial Number: "+cert.getSerialNumber()+"\n"+
            "  Version: "+cert.getVersion()+"\n"+
            "  Signature Algorithm: "+cert.getPublicKey().getAlgorithm()+"\n"+
            "  Not Valid Before: "+cert.getNotBefore()+"\n"+
            "  Not Valid After: "+cert.getNotAfter()+"\n"+
            "  SHA1 Fingerprint: "+getThumbprint(cert)+"\n"+
            "\n"+
            "Do you want to save it and continue?";
          if (!msg.showMsgBox(YES_NO_OPTION, "certificate issuer unknown",
                certinfo)) {
            throw new AuthFailureException("certificate issuer unknown");
          }
          if (certs == null || !certs.contains(cert)) {
            byte[] der = cert.getEncoded();
            String pem = Base64.getEncoder().encodeToString(der);
            pem = pem.replaceAll("(.{64})", "$1\n");
            FileWriter fw = null;
            try {
              if (!vncDir.exists())
                vncDir.mkdir();
              if (!certFile.exists() && !certFile.createNewFile()) {
                vlog.error("Certificate save failed.");
              } else {
                fw = new FileWriter(certFile.getAbsolutePath(), true);
                fw.write("-----BEGIN CERTIFICATE-----\n");
                fw.write(pem+"\n");
                fw.write("-----END CERTIFICATE-----\n");
              }
            } catch (IOException ioe) {
              msg.showMsgBox(OK_OPTION, "certificate save failed",
                             "Could not save the certificate");
            } finally {
              try {
                if (fw != null)
                  fw.close();
              } catch(IOException ioe2) {
                throw new Exception(ioe2.getMessage());
              }
            }
          }
          "Hostname verification failed. Do you want to continue?",
          "Hostname Verification Failure",
          throw new WarningException("Hostname verification failed.");
Fixed code:       String pk =
        Base64.getEncoder().encodeToString(cert.getPublicKey().getEncoded());
        verifyHostname(cert);
      } catch(CertificateParsingException e) {
        throw new SystemException(e.getMessage());
      if (!vncDir.exists())
        throw new AuthFailureException("Could not obtain VNC home directory "+
                                       "path for known hosts storage");
      File dbPath = new File(vncDir, "x509_known_hosts");
      String info =
        "  Subject: "+cert.getSubjectX500Principal().getName()+"\n"+
        "  Issuer: "+cert.getIssuerX500Principal().getName()+"\n"+
        "  Serial Number: "+cert.getSerialNumber()+"\n"+
        "  Version: "+cert.getVersion()+"\n"+
        "  Signature Algorithm: "+cert.getPublicKey().getAlgorithm()+"\n"+
        "  Not Valid Before: "+cert.getNotBefore()+"\n"+
        "  Not Valid After: "+cert.getNotAfter()+"\n"+
        "  SHA-1 Fingerprint: "+getThumbprint(cert)+"\n";
        if (dbPath.exists()) {
          FileReader db = new FileReader(dbPath);
          BufferedReader dbBuf = new BufferedReader(db);
          String line;
          String server = client.getServerName().toLowerCase();
          while ((line = dbBuf.readLine())!=null) {
            String fields[] = line.split("\\|");
            if (fields.length==6) {
              if (server.equals(fields[2]) && pk.equals(fields[5])) {
                vlog.debug("Server certificate found in known hosts file");
                dbBuf.close();
                return;
              } else if (server.equals(fields[2]) && !pk.equals(fields[5]) ||
                         !server.equals(fields[2]) && pk.equals(fields[5])) {
                throw new CertStoreException();
              }
            }
          }
          dbBuf.close();
        }
      } catch (IOException e) {
        throw new AuthFailureException("Could not load known hosts database");
      } catch (CertStoreException e) {
        vlog.debug("Server host key mismatch");
        vlog.debug(info);
        String text =
          "This host is previously known with a different "+
          "certificate, and the new certificate has been "+
          "signed by an unknown authority\n"+
          "\n"+info+"\n"+
          "Someone could be trying to impersonate the site and you should not continue.\n"+
          "\n"+
          "Do you want to make an exception for this server?";
        if (!msg.showMsgBox(YES_NO_OPTION, "Unexpected certificate issuer", text))
          throw new AuthFailureException("Unexpected certificate issuer");
        store_pubkey(dbPath, client.getServerName().toLowerCase(), pk);
          vlog.debug("Server host not previously known");
          vlog.debug(info);
          String text =
            "\n"+info+"\n"+
            "Someone could be trying to impersonate the site and you should not continue.\n"+
            "Do you want to make an exception for this server?";
          if (!msg.showMsgBox(YES_NO_OPTION, "Unknown certificate issuer", text))
            throw new AuthFailureException("Unknown certificate issuer");
          store_pubkey(dbPath, client.getServerName().toLowerCase(), pk);
    private void store_pubkey(File dbPath, String serverName, String pk)
    {
      ArrayList<String> lines = new ArrayList<String>();
      File vncDir = new File(FileUtils.getVncHomeDir());
      try {
        if (dbPath.exists()) {
          FileReader db = new FileReader(dbPath);
          BufferedReader dbBuf = new BufferedReader(db);
          String line;
          while ((line = dbBuf.readLine())!=null) {
            String fields[] = line.split("\\|");
            if (fields.length==6)
              if (!serverName.equals(fields[2]) && !pk.equals(fields[5]))
                lines.add(line);
          }
          dbBuf.close();
        }
      } catch (IOException e) {
        throw new AuthFailureException("Could not load known hosts database");
      }
      try {
        if (!dbPath.exists())
          dbPath.createNewFile();
        FileWriter fw = new FileWriter(dbPath.getAbsolutePath(), false);
        Iterator i = lines.iterator();
        while (i.hasNext())
          fw.write((String)i.next()+"\n");
        fw.write("|g0|"+serverName+"|*|0|"+pk+"\n");
        fw.close();
      } catch (IOException e) {
        vlog.error("Failed to store server certificate to known hosts database");
      }
    }

          "Hostname ("+client.getServerName()+") does not match the"+
          " server certificate, do you want to continue?",
          "Certificate hostname mismatch",
          throw new WarningException("Certificate hostname mismatch.");