use system trust store for TLS

[kim0]

I just issued a certificate from letsencrypt. Started a server with it. Connected from tigervnc 1.9.0 on OSX, and got hit by this warning, which is annoying. I did the whole letsencrypt dance, just to avoid such warnings. Hope tigervnc can start loving LE soon.

[kim0]

Sorry for the shoutout @CendioOssman .. but I just need to know whether this looks like a real bug, or some misconfiguration on my side. Thanks!

[kim0]

I've tested this on Windows-10, I'm still getting the warning. I have also imported the letsencrypt X3 cross-signed certificate, into the machine's trusted root CA store. This didn't help, and still getting the error. Not sure if TigerVNC does not use the OS's root-ca store, by design or if that's a bug .. Any thoughts @CendioOssman ? Thanks!

[CendioOssman]

Unfortunately I never got terribly involved in the TLS code, and we don't have an active developer for those bits right now. It might very well be that we don't integrate properly with the system trust store. What you can do at least is to specify the CA certificate to the client in the X509CA option. You can also try specifying -GnuTLSPriority=@SYSTEM and see if that gets the proper behaviour.

[kim0]

Thanks for the reply! Yes, indeed adding the CA cert to the client options, does work. But it would have been infinitely better if things just worked :)

Regarding -GnuTLSPriority=@SYSTEM, trying that on OSX, it failed, and I'm getting an error message (without asking for username/password): Authentication failure: gnutls_set_priority_direct failed

What would have been a simple and good enough workaround, if we could specify the CA pem file contents directly inline in a .tigervnc file. That way, I could just ship .tigervnc files to users, they'd double click and it works. Instead of asking them to type the path where they unzipped the CA files.

Hoping someone will pick this up soonish .. Thanks folks

[CendioOssman]

It looks like we need to call gnutls_certificate_set_x509_system_trust(). It seems to have support for all platforms. However I notice that it requires macOS 10.7. @bphinz, which version are you targeting in your builds of GnuTLS?

[bphinz]

I’ll have to check which SDK version is targeted (either 10.6 or 10.7), but it’s built against GnuTLS 3.3.30.

-brian

On Sat, Jul 28, 2018 at 8:47 AM Pierre Ossman (Work account) < notifications@github.com> wrote:

It looks like we need to call gnutls_certificate_set_x509_system_trust(). It seems to have support for all platforms. However I notice that it requires macOS 10.7. @bphinz https://github.com/bphinz, which version are you targeting in your builds of GnuTLS?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/TigerVNC/tigervnc/issues/685#issuecomment-408605825, or mute the thread https://github.com/notifications/unsubscribe-auth/AHnWbdstF0IMMqoExcJ7fOj6kHjCRpv8ks5uLF1hgaJpZM4VZbrf .

[bphinz]

The OSX builds are targeting the 10.6 SDK

[kim0]

I hope switching to 10.7 SDK will not be a problem? I can test a beta build if needed, let me know any time. Thanks

[CendioOssman]

Completely untested patch:

diff --git a/common/rfb/CSecurityTLS.cxx b/common/rfb/CSecurityTLS.cxx
index 9a698f03..9eeb76cc 100644
--- a/common/rfb/CSecurityTLS.cxx
+++ b/common/rfb/CSecurityTLS.cxx
@@ -229,6 +229,9 @@ void CSecurityTLS::setParam()
     if (gnutls_certificate_allocate_credentials(&cert_cred) != GNUTLS_E_SUCCESS)
       throw AuthFailureException("gnutls_certificate_allocate_credentials failed");

+    if (gnutls_certificate_set_x509_system_trust(cert_cred) != GNUTLS_E_SUCCESS)
+      vlog.error("Could not load system certificate trust store");
+
     if (*cafile && gnutls_certificate_set_x509_trust_file(cert_cred,cafile,GNUTLS_X509_FMT_PEM) < 0)
       throw AuthFailureException("load of CA cert failed");

[kim0]

Thanks a lot @CendioOssman .. Is it possible to run a nightly build with that patch, then I can verify the resulting binaries on OSX/Windows? Creating my own build envs is not too easy. Thanks again!

[bphinz]

Is it possible to run a nightly build with that patch, then I can verify the resulting binaries on OSX/Windows?

I think it would require updating the minimum supported version of OSX to be 10.7 and then altering our build environment to match that. I'm not really opposed to doing so, we certainly deprecate other EoL'd OS versions. @CendioOssman - your thoughts?

[CendioOssman]

I have no concerns about people running such old versions. But it is a bit odd to raise the requirements because of GnuTLS rather than a code change in TigerVNC. We also cannot currently use a newer SDK here at Cendio because of issues with gcc.

But I suppose we could do a "soft" requirements increase, where we just use a newer build system for the binaries but don't add any code that strictly requires something newer.

[CendioOssman]

@bphinz, can you do a test build to see if it actually solves @kim0's problem first?

[bphinz]

I can, but it will be a couple of days. I took down the build system because I'm rebuilding it and I'm in the process of switching the OS on the main server.

On Tue, Jul 31, 2018 at 10:16 AM, Pierre Ossman (Work account) < notifications@github.com> wrote:

@bphinz https://github.com/bphinz, can you do a test build to see if it actually solves @kim0 https://github.com/kim0's problem first?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TigerVNC/tigervnc/issues/685#issuecomment-409236432, or mute the thread https://github.com/notifications/unsubscribe-auth/AHnWbRpoEH2dqyp0V5LC5-tIhU_XBJjUks5uMGa_gaJpZM4VZbrf .

[bphinz]

The nightly builds are back online. Did you want that test build done using the 10.7 SDK or just applying the patch to our current environment?

[kim0]

I think @CendioOssman mentioned However I notice that it requires macOS 10.7 .. So, IMO after applying that patch, we will need to switch to 10.7 as a requirement. Thanks a lot @bphinz 👍

[bphinz]

@kim0, try this: https://drive.google.com/open?id=1RLeP8QsdUNrxb44xi-oybficOJ4vszou

[kim0]

wow, that does seem to resolve the issue! Thanks a lot @bphinz for this build, and thanks a million @CendioOssman for the patch.

To be clear, I tried this 3 times on the old version, and 3 on the new version. Every-time the old version shows the certificate warning, and the new build does NOT 👍

I'm not sure about your schedule for future releases, can we aim to get this fix out fairly rapidly? Thanks a million team

[CendioOssman]

Thanks for testing. I've commited the fix in c04f756bd256a1172334db673598eadfc93cd7ff.

@bphinz, can you make sure GnuTLS is built for 10.7 in the regular builds?

[bphinz]

The builds are now using the 10.7 SDK