Merge branch 'tls' of https://github.com/CendioOssman/tigervnc

11 files changed, 68 insertions, 34 deletions

diff --git a/common/rdr/TLSInStream.cxx b/common/rdr/TLSInStream.cxx
index 2339956d..451855c7 100644
--- a/common/rdr/TLSInStream.cxx
+++ b/common/rdr/TLSInStream.cxx
@@ -1,6 +1,7 @@
 /* Copyright (C) 2002-2005 RealVNC Ltd.  All Rights Reserved.
  * Copyright (C) 2005 Martin Koegler
  * Copyright (C) 2010 TigerVNC Team
+ * Copyright (C) 2012-2021 Pierre Ossman for Cendio AB
  * 
  * This is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -38,6 +39,9 @@ ssize_t TLSInStream::pull(gnutls_transport_ptr_t str, void* data, size_t size)
   TLSInStream* self= (TLSInStream*) str;
   InStream *in = self->in;
 
+  delete self->saved_exception;
+  self->saved_exception = NULL;
+
   try {
     if (!in->hasData(1)) {
       gnutls_transport_set_errno(self->session, EAGAIN);
@@ -50,9 +54,15 @@ ssize_t TLSInStream::pull(gnutls_transport_ptr_t str, void* data, size_t size)
     in->readBytes(data, size);
   } catch (EndOfStream&) {
     return 0;
+  } catch (SystemException &e) {
+    vlog.error("Failure reading TLS data: %s", e.str());
+    gnutls_transport_set_errno(self->session, e.err);
+    self->saved_exception = new SystemException(e);
+    return -1;
   } catch (Exception& e) {
     vlog.error("Failure reading TLS data: %s", e.str());
     gnutls_transport_set_errno(self->session, EINVAL);
+    self->saved_exception = new Exception(e);
     return -1;
   }
 
@@ -60,7 +70,7 @@ ssize_t TLSInStream::pull(gnutls_transport_ptr_t str, void* data, size_t size)
 }
 
 TLSInStream::TLSInStream(InStream* _in, gnutls_session_t _session)
-  : session(_session), in(_in)
+  : session(_session), in(_in), saved_exception(NULL)
 {
   gnutls_transport_ptr_t recv, send;
 
@@ -72,6 +82,8 @@ TLSInStream::TLSInStream(InStream* _in, gnutls_session_t _session)
 TLSInStream::~TLSInStream()
 {
   gnutls_transport_set_pull_function(session, NULL);
+
+  delete saved_exception;
 }
 
 bool TLSInStream::fillBuffer(size_t maxSize)
@@ -88,16 +100,18 @@ size_t TLSInStream::readTLS(U8* buf, size_t len)
 {
   int n;
 
-  if (gnutls_record_check_pending(session) == 0) {
-    if (!in->hasData(1))
-      return 0;
-  }
-
   n = gnutls_record_recv(session, (void *) buf, len);
   if (n == GNUTLS_E_INTERRUPTED || n == GNUTLS_E_AGAIN)
     return 0;
 
-  if (n < 0) throw TLSException("readTLS", n);
+  if (n == GNUTLS_E_PULL_ERROR)
+    throw *saved_exception;
+
+  if (n < 0)
+    throw TLSException("readTLS", n);
+
+  if (n == 0)
+    throw EndOfStream();
 
   return n;
 }
diff --git a/common/rdr/TLSInStream.h b/common/rdr/TLSInStream.h
index df5ebb48..8fe35d26 100644
--- a/common/rdr/TLSInStream.h
+++ b/common/rdr/TLSInStream.h
@@ -43,6 +43,8 @@ namespace rdr {
 
     gnutls_session_t session;
     InStream* in;
+
+    Exception* saved_exception;
   };
 };
 
diff --git a/common/rdr/TLSOutStream.cxx b/common/rdr/TLSOutStream.cxx
index a883fe89..dc6b55aa 100644
--- a/common/rdr/TLSOutStream.cxx
+++ b/common/rdr/TLSOutStream.cxx
@@ -1,6 +1,7 @@
 /* Copyright (C) 2002-2005 RealVNC Ltd.  All Rights Reserved.
  * Copyright (C) 2005 Martin Koegler
  * Copyright (C) 2010 TigerVNC Team
+ * Copyright (C) 2012-2021 Pierre Ossman for Cendio AB
  * 
  * This is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -41,12 +42,21 @@ ssize_t TLSOutStream::push(gnutls_transport_ptr_t str, const void* data,
   TLSOutStream* self= (TLSOutStream*) str;
   OutStream *out = self->out;
 
+  delete self->saved_exception;
+  self->saved_exception = NULL;
+
   try {
     out->writeBytes(data, size);
     out->flush();
+  } catch (SystemException &e) {
+    vlog.error("Failure sending TLS data: %s", e.str());
+    gnutls_transport_set_errno(self->session, e.err);
+    self->saved_exception = new SystemException(e);
+    return -1;
   } catch (Exception& e) {
     vlog.error("Failure sending TLS data: %s", e.str());
     gnutls_transport_set_errno(self->session, EINVAL);
+    self->saved_exception = new Exception(e);
     return -1;
   }
 
@@ -54,7 +64,8 @@ ssize_t TLSOutStream::push(gnutls_transport_ptr_t str, const void* data,
 }
 
 TLSOutStream::TLSOutStream(OutStream* _out, gnutls_session_t _session)
-  : session(_session), out(_out), bufSize(DEFAULT_BUF_SIZE), offset(0)
+  : session(_session), out(_out), bufSize(DEFAULT_BUF_SIZE), offset(0),
+    saved_exception(NULL)
 {
   gnutls_transport_ptr_t recv, send;
 
@@ -77,6 +88,7 @@ TLSOutStream::~TLSOutStream()
   gnutls_transport_set_push_function(session, NULL);
 
   delete [] start;
+  delete saved_exception;
 }
 
 size_t TLSOutStream::length()
@@ -129,6 +141,9 @@ size_t TLSOutStream::writeTLS(const U8* data, size_t length)
   if (n == GNUTLS_E_INTERRUPTED || n == GNUTLS_E_AGAIN)
     return 0;
 
+  if (n == GNUTLS_E_PUSH_ERROR)
+    throw *saved_exception;
+
   if (n < 0)
     throw TLSException("writeTLS", n);
 
diff --git a/common/rdr/TLSOutStream.h b/common/rdr/TLSOutStream.h
index c9750463..c5579127 100644
--- a/common/rdr/TLSOutStream.h
+++ b/common/rdr/TLSOutStream.h
@@ -51,6 +51,8 @@ namespace rdr {
     size_t bufSize;
     U8* start;
     size_t offset;
+
+    Exception* saved_exception;
   };
 };
 
diff --git a/common/rfb/CSecurityStack.cxx b/common/rfb/CSecurityStack.cxx
index 55f3133e..2541d81e 100644
--- a/common/rfb/CSecurityStack.cxx
+++ b/common/rfb/CSecurityStack.cxx
@@ -32,10 +32,8 @@ CSecurityStack::CSecurityStack(CConnection* cc, int Type, const char* Name,
 
 CSecurityStack::~CSecurityStack()
 {
-  if (state0)
-    delete state0;
-  if (state1)
-    delete state1;
+  delete state0;
+  delete state1;
 }
 
 bool CSecurityStack::processMsg()
diff --git a/common/rfb/CSecurityTLS.cxx b/common/rfb/CSecurityTLS.cxx
index b32725f0..5337d8d6 100644
--- a/common/rfb/CSecurityTLS.cxx
+++ b/common/rfb/CSecurityTLS.cxx
@@ -3,6 +3,7 @@
  * Copyright (C) 2005 Martin Koegler
  * Copyright (C) 2010 TigerVNC Team
  * Copyright (C) 2010 m-privacy GmbH
+ * Copyright (C) 2012-2021 Pierre Ossman for Cendio AB
  *    
  * This is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -99,11 +100,16 @@ void CSecurityTLS::setDefaults()
    X509CRL.setDefaultStr(crlDefault.buf);
 }
 
-void CSecurityTLS::shutdown(bool needbye)
+void CSecurityTLS::shutdown()
 {
-  if (session && needbye)
-    if (gnutls_bye(session, GNUTLS_SHUT_RDWR) != GNUTLS_E_SUCCESS)
-      vlog.error("gnutls_bye failed");
+  if (session) {
+    int ret;
+    // FIXME: We can't currently wait for the response, so we only send
+    //        our close and hope for the best
+    ret = gnutls_bye(session, GNUTLS_SHUT_WR);
+    if ((ret != GNUTLS_E_SUCCESS) && (ret != GNUTLS_E_INVALID_SESSION))
+      vlog.error("TLS shutdown failed: %s", gnutls_strerror(ret));
+  }
 
   if (anon_cred) {
     gnutls_anon_free_client_credentials(anon_cred);
@@ -139,7 +145,7 @@ void CSecurityTLS::shutdown(bool needbye)
 
 CSecurityTLS::~CSecurityTLS()
 {
-  shutdown(true);
+  shutdown();
 
   delete[] cafile;
   delete[] crlfile;
@@ -186,7 +192,7 @@ bool CSecurityTLS::processMsg()
     }
 
     vlog.error("TLS Handshake failed: %s\n", gnutls_strerror (err));
-    shutdown(false);
+    shutdown();
     throw AuthFailureException("TLS Handshake failed");
   }
 
diff --git a/common/rfb/CSecurityTLS.h b/common/rfb/CSecurityTLS.h
index 476d0ef8..0dcf2ad3 100644
--- a/common/rfb/CSecurityTLS.h
+++ b/common/rfb/CSecurityTLS.h
@@ -55,7 +55,7 @@ namespace rfb {
     static UserMsgBox *msg;
 
   protected:
-    void shutdown(bool needbye);
+    void shutdown();
     void freeResources();
     void setParam();
     void checkSession();
diff --git a/common/rfb/CSecurityVeNCrypt.cxx b/common/rfb/CSecurityVeNCrypt.cxx
index 98dad494..c0fe3273 100644
--- a/common/rfb/CSecurityVeNCrypt.cxx
+++ b/common/rfb/CSecurityVeNCrypt.cxx
@@ -55,8 +55,8 @@ CSecurityVeNCrypt::CSecurityVeNCrypt(CConnection* cc, SecurityClient* sec)
 
 CSecurityVeNCrypt::~CSecurityVeNCrypt()
 {
-  if (availableTypes)
-	delete[] availableTypes;
+  delete[] availableTypes;
+  delete csecurity;
 }
 
 bool CSecurityVeNCrypt::processMsg()
diff --git a/common/rfb/SSecurityStack.cxx b/common/rfb/SSecurityStack.cxx
index 74509e71..81395113 100644
--- a/common/rfb/SSecurityStack.cxx
+++ b/common/rfb/SSecurityStack.cxx
@@ -28,10 +28,8 @@ SSecurityStack::SSecurityStack(SConnection* sc, int Type,
 
 SSecurityStack::~SSecurityStack()
 {
-  if (state0)
-    delete state0;
-  if (state1)
-    delete state1;
+  delete state0;
+  delete state1;
 }
 
 bool SSecurityStack::processMsg()
diff --git a/common/rfb/SSecurityTLS.cxx b/common/rfb/SSecurityTLS.cxx
index d5ef47e6..db0221f8 100644
--- a/common/rfb/SSecurityTLS.cxx
+++ b/common/rfb/SSecurityTLS.cxx
@@ -2,6 +2,7 @@
  * Copyright (C) 2004 Red Hat Inc.
  * Copyright (C) 2005 Martin Koegler
  * Copyright (C) 2010 TigerVNC Team
+ * Copyright (C) 2012-2021 Pierre Ossman for Cendio AB
  *    
  * This is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -64,10 +65,12 @@ SSecurityTLS::SSecurityTLS(SConnection* sc, bool _anon)
 void SSecurityTLS::shutdown()
 {
   if (session) {
-    if (gnutls_bye(session, GNUTLS_SHUT_RDWR) != GNUTLS_E_SUCCESS) {
-      /* FIXME: Treat as non-fatal error */
-      vlog.error("TLS session wasn't terminated gracefully");
-    }
+    int ret;
+    // FIXME: We can't currently wait for the response, so we only send
+    //        our close and hope for the best
+    ret = gnutls_bye(session, GNUTLS_SHUT_WR);
+    if ((ret != GNUTLS_E_SUCCESS) && (ret != GNUTLS_E_INVALID_SESSION))
+      vlog.error("TLS shutdown failed: %s", gnutls_strerror(ret));
   }
 
   if (dh_params) {
diff --git a/common/rfb/SSecurityVeNCrypt.cxx b/common/rfb/SSecurityVeNCrypt.cxx
index 135742c0..70d50d20 100644
--- a/common/rfb/SSecurityVeNCrypt.cxx
+++ b/common/rfb/SSecurityVeNCrypt.cxx
@@ -57,11 +57,7 @@ SSecurityVeNCrypt::SSecurityVeNCrypt(SConnection* sc, SecurityServer *sec)
 SSecurityVeNCrypt::~SSecurityVeNCrypt()
 {
   delete ssecurity;
-
-  if (subTypes) {
-    delete [] subTypes;
-    subTypes = NULL;
-  }
+  delete [] subTypes;
 }
 
 bool SSecurityVeNCrypt::processMsg()