- U_0001-Properly-store-certificate-exceptions.patch,

U_0002-Properly-store-certificate-exceptions-in-Java-viewer.patch
  * Properly store certificate exceptions (boo#1176733)
- adjusted u_tigervnc-add-autoaccept-parameter.patch

OBS-URL: https://build.opensuse.org/package/show/X11:XOrg/tigervnc?expand=0&rev=189
This commit is contained in:
Stefan Dirsch 2020-09-25 11:26:30 +00:00 committed by Git OBS Bridge
parent 522fe20d64
commit 849f8f0e68
5 changed files with 495 additions and 14 deletions

View File

@ -0,0 +1,228 @@
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

View File

@ -0,0 +1,234 @@
From f029745f63ac7d22fb91639b2cb5b3ab56134d6e Mon Sep 17 00:00:00 2001
From: "Brian P. Hinz" <bphinz@users.sf.net>
Date: Tue, 8 Sep 2020 10:13:32 +0200
Subject: [PATCH] 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.
---
java/com/tigervnc/rfb/CSecurityTLS.java | 164 ++++++++++++++++++++------------
1 file changed, 101 insertions(+), 63 deletions(-)
diff --git a/java/com/tigervnc/rfb/CSecurityTLS.java b/java/com/tigervnc/rfb/CSecurityTLS.java
index ad6f6fe1..e63945dc 100644
--- a/java/com/tigervnc/rfb/CSecurityTLS.java
+++ b/java/com/tigervnc/rfb/CSecurityTLS.java
@@ -107,12 +107,6 @@ public class CSecurityTLS extends CSecurity {
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();
@@ -269,8 +263,13 @@ public class CSecurityTLS extends CSecurity {
{
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) {
@@ -279,73 +278,111 @@ public class CSecurityTLS extends CSecurity {
"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();
@@ -399,12 +436,13 @@ 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) {
--
2.16.4

View File

@ -1,3 +1,11 @@
-------------------------------------------------------------------
Fri Sep 25 10:38:58 UTC 2020 - Stefan Dirsch <sndirsch@suse.com>
- U_0001-Properly-store-certificate-exceptions.patch,
U_0002-Properly-store-certificate-exceptions-in-Java-viewer.patch
* Properly store certificate exceptions (boo#1176733)
- adjusted u_tigervnc-add-autoaccept-parameter.patch
-------------------------------------------------------------------
Wed Sep 16 10:01:17 UTC 2020 - Stefan Dirsch <sndirsch@suse.com>

View File

@ -147,6 +147,8 @@ Patch10: n_correct_path_in_desktop_file.patch
Patch11: U_viewer-reset-ctrl-alt-to-menu-state-on-focus.patch
Patch12: tigervnc-fix-saving-of-bad-server-certs.patch
Patch13: u_xorg-server-1.20.7-ddxInputThreadInit.patch
Patch21: U_0001-Properly-store-certificate-exceptions.patch
Patch22: U_0002-Properly-store-certificate-exceptions-in-Java-viewer.patch
%description
TigerVNC is an implementation of VNC (Virtual Network Computing), a
@ -259,12 +261,14 @@ It maps common x11vnc arguments to x0vncserver arguments.
%patch5 -p1
%patch6 -p1
%patch7 -p1
%patch8 -p1
%patch9 -p1
%patch10 -p1
%patch11 -p1
%patch12 -p1
%patch13 -p1
%patch21 -p1
%patch22 -p1
%patch8 -p1
cp -r %{_prefix}/src/xserver/* unix/xserver/
pushd unix/xserver

View File

@ -1,7 +1,7 @@
Index: tigervnc-1.9.0/java/com/tigervnc/rfb/CSecurityTLS.java
Index: tigervnc-1.10.1/java/com/tigervnc/rfb/CSecurityTLS.java
===================================================================
--- tigervnc-1.9.0.orig/java/com/tigervnc/rfb/CSecurityTLS.java
+++ tigervnc-1.9.0/java/com/tigervnc/rfb/CSecurityTLS.java
--- tigervnc-1.10.1.orig/java/com/tigervnc/rfb/CSecurityTLS.java
+++ tigervnc-1.10.1/java/com/tigervnc/rfb/CSecurityTLS.java
@@ -66,6 +66,9 @@ public class CSecurityTLS extends CSecur
public static StringParameter X509CRL
= new StringParameter("X509CRL",
@ -20,18 +20,25 @@ Index: tigervnc-1.9.0/java/com/tigervnc/rfb/CSecurityTLS.java
}
public static String getDefaultCA() {
@@ -283,6 +287,10 @@ public class CSecurityTLS extends CSecur
tm.checkServerTrusted(chain, authType);
@@ -278,6 +282,7 @@ public class CSecurityTLS extends CSecur
"do you want to continue?"))
throw new AuthFailureException("server certificate has expired");
}
+ String thumbprint = getThumbprint(cert);
File vncDir = new File(FileUtils.getVncHomeDir());
if (!vncDir.exists())
throw new AuthFailureException("Could not obtain VNC home directory "+
@@ -332,6 +337,9 @@ public class CSecurityTLS extends CSecur
store_pubkey(dbPath, client.getServerName().toLowerCase(), pk);
} catch (java.lang.Exception e) {
if (e.getCause() instanceof CertPathBuilderException) {
+ if (certautoaccept != null && thumbprint.equalsIgnoreCase(certautoaccept)) {
+ return;
+ }
+
String certinfo =
"This certificate has been signed by an unknown authority\n"+
"\n"+
@@ -471,7 +479,7 @@ public class CSecurityTLS extends CSecur
vlog.debug("Server host not previously known");
vlog.debug(info);
String text =
@@ -519,7 +527,7 @@ public class CSecurityTLS extends CSecur
private SSLEngineManager manager;
private boolean anon;
@ -40,10 +47,10 @@ Index: tigervnc-1.9.0/java/com/tigervnc/rfb/CSecurityTLS.java
private FdInStream is;
private FdOutStream os;
Index: tigervnc-1.9.0/java/com/tigervnc/vncviewer/VncViewer.java
Index: tigervnc-1.10.1/java/com/tigervnc/vncviewer/VncViewer.java
===================================================================
--- tigervnc-1.9.0.orig/java/com/tigervnc/vncviewer/VncViewer.java
+++ tigervnc-1.9.0/java/com/tigervnc/vncviewer/VncViewer.java
--- tigervnc-1.10.1.orig/java/com/tigervnc/vncviewer/VncViewer.java
+++ tigervnc-1.10.1/java/com/tigervnc/vncviewer/VncViewer.java
@@ -393,6 +393,8 @@ public class VncViewer extends javax.swi
// Called right after zero-arg constructor in applet mode
setLookAndFeel();