From daf3d88aa1b554c5d6d41116c51517d30cb7c4b7 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 1 Sep 2017 11:14:35 +0200 Subject: Display security state when asking for password Indicate to the user how secure the transport channel is so they can avoid entering their password for untrusted sites. --- common/rfb/CConnection.h | 2 ++ common/rfb/CSecurity.h | 1 + common/rfb/CSecurityPlain.cxx | 2 +- common/rfb/CSecurityStack.cxx | 9 +++++++++ common/rfb/CSecurityStack.h | 1 + common/rfb/CSecurityTLS.h | 1 + common/rfb/CSecurityVeNCrypt.cxx | 6 ++++++ common/rfb/CSecurityVeNCrypt.h | 1 + common/rfb/CSecurityVncAuth.cxx | 2 +- common/rfb/UserPasswdGetter.h | 2 +- 10 files changed, 24 insertions(+), 3 deletions(-) (limited to 'common') diff --git a/common/rfb/CConnection.h b/common/rfb/CConnection.h index e0a000ff..e29c0331 100644 --- a/common/rfb/CConnection.h +++ b/common/rfb/CConnection.h @@ -134,6 +134,8 @@ namespace rfb { // Identities, to determine the unique(ish) name of the server. const char* getServerName() const { return serverName.buf; } + bool isSecure() const { return csecurity ? csecurity->isSecure() : false; } + enum stateEnum { RFBSTATE_UNINITIALISED, RFBSTATE_PROTOCOL_VERSION, diff --git a/common/rfb/CSecurity.h b/common/rfb/CSecurity.h index 36da5c7a..3fedc508 100644 --- a/common/rfb/CSecurity.h +++ b/common/rfb/CSecurity.h @@ -49,6 +49,7 @@ namespace rfb { virtual void destroy() { delete this; } virtual int getType() const = 0; virtual const char* description() const = 0; + virtual bool isSecure() const { return false; } /* * Use variable directly instead of dumb get/set methods. diff --git a/common/rfb/CSecurityPlain.cxx b/common/rfb/CSecurityPlain.cxx index 0320ce2d..8e383c31 100644 --- a/common/rfb/CSecurityPlain.cxx +++ b/common/rfb/CSecurityPlain.cxx @@ -33,7 +33,7 @@ bool CSecurityPlain::processMsg(CConnection* cc) CharArray username; CharArray password; - (CSecurity::upg)->getUserPasswd(&username.buf, &password.buf); + (CSecurity::upg)->getUserPasswd(cc->isSecure(), &username.buf, &password.buf); // Return the response to the server os->writeU32(strlen(username.buf)); diff --git a/common/rfb/CSecurityStack.cxx b/common/rfb/CSecurityStack.cxx index cfc60fd5..47c3f6db 100644 --- a/common/rfb/CSecurityStack.cxx +++ b/common/rfb/CSecurityStack.cxx @@ -63,3 +63,12 @@ bool CSecurityStack::processMsg(CConnection* cc) return res; } + +bool CSecurityStack::isSecure() const +{ + if (state0 && state0->isSecure()) + return true; + if (state == 1 && state1 && state1->isSecure()) + return true; + return false; +} diff --git a/common/rfb/CSecurityStack.h b/common/rfb/CSecurityStack.h index a76b3fe3..a16003f0 100644 --- a/common/rfb/CSecurityStack.h +++ b/common/rfb/CSecurityStack.h @@ -32,6 +32,7 @@ namespace rfb { virtual bool processMsg(CConnection* cc); virtual int getType() const {return type;}; virtual const char* description() const {return name;} + virtual bool isSecure() const; protected: int state; CSecurity* state0; diff --git a/common/rfb/CSecurityTLS.h b/common/rfb/CSecurityTLS.h index 57d964d7..e726d1e9 100644 --- a/common/rfb/CSecurityTLS.h +++ b/common/rfb/CSecurityTLS.h @@ -48,6 +48,7 @@ namespace rfb { virtual int getType() const { return anon ? secTypeTLSNone : secTypeX509None; } virtual const char* description() const { return anon ? "TLS Encryption without VncAuth" : "X509 Encryption without VncAuth"; } + virtual bool isSecure() const { return !anon; } static void setDefaults(); static StringParameter X509CA; diff --git a/common/rfb/CSecurityVeNCrypt.cxx b/common/rfb/CSecurityVeNCrypt.cxx index a15da4a6..4a25245a 100644 --- a/common/rfb/CSecurityVeNCrypt.cxx +++ b/common/rfb/CSecurityVeNCrypt.cxx @@ -191,3 +191,9 @@ bool CSecurityVeNCrypt::processMsg(CConnection* cc) return csecurity->processMsg(cc); } +bool CSecurityVeNCrypt::isSecure() const +{ + if (csecurity && csecurity->isSecure()) + return true; + return false; +} diff --git a/common/rfb/CSecurityVeNCrypt.h b/common/rfb/CSecurityVeNCrypt.h index 55d0744a..1ff0c020 100644 --- a/common/rfb/CSecurityVeNCrypt.h +++ b/common/rfb/CSecurityVeNCrypt.h @@ -39,6 +39,7 @@ namespace rfb { virtual bool processMsg(CConnection* cc);// { return true; } int getType() const {return chosenType;} virtual const char* description() const { return secTypeName(chosenType); } + virtual bool isSecure() const; protected: CSecurity *csecurity; diff --git a/common/rfb/CSecurityVncAuth.cxx b/common/rfb/CSecurityVncAuth.cxx index f44e56ea..46463e0a 100644 --- a/common/rfb/CSecurityVncAuth.cxx +++ b/common/rfb/CSecurityVncAuth.cxx @@ -49,7 +49,7 @@ bool CSecurityVncAuth::processMsg(CConnection* cc) rdr::U8 challenge[vncAuthChallengeSize]; is->readBytes(challenge, vncAuthChallengeSize); PlainPasswd passwd; - (CSecurity::upg)->getUserPasswd(0, &passwd.buf); + (CSecurity::upg)->getUserPasswd(cc->isSecure(), 0, &passwd.buf); // Calculate the correct response rdr::U8 key[8]; diff --git a/common/rfb/UserPasswdGetter.h b/common/rfb/UserPasswdGetter.h index 18b0bae3..13493e4d 100644 --- a/common/rfb/UserPasswdGetter.h +++ b/common/rfb/UserPasswdGetter.h @@ -24,7 +24,7 @@ namespace rfb { // dialog, getpass(), etc. The user buffer pointer can be null, in which // case no user name will be retrieved. The caller MUST delete [] the // result(s). - virtual void getUserPasswd(char** user, char** password)=0; + virtual void getUserPasswd(bool secure, char** user, char** password)=0; }; } #endif -- cgit v1.2.3 From e43e5e30517498ec070b568a7d91edb942779d63 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 1 Sep 2017 11:15:31 +0200 Subject: Add better error message for insecure certificate algorithms --- common/rfb/CSecurityTLS.cxx | 3 +++ 1 file changed, 3 insertions(+) (limited to 'common') diff --git a/common/rfb/CSecurityTLS.cxx b/common/rfb/CSecurityTLS.cxx index 8a053e3d..58423fbf 100644 --- a/common/rfb/CSecurityTLS.cxx +++ b/common/rfb/CSecurityTLS.cxx @@ -332,6 +332,9 @@ void CSecurityTLS::checkSession() if (status & GNUTLS_CERT_SIGNER_NOT_CA) vlog.debug("server cert signer not CA"); + if (status & GNUTLS_CERT_INSECURE_ALGORITHM) + throw AuthFailureException("The server certificate uses an insecure algorithm"); + if ((status & (~allowed_errors)) != 0) { /* No other errors are allowed */ vlog.debug("GNUTLS status of certificate verification: %u", status); -- cgit v1.2.3 From b993ea78d72323709a9e1c6a2fac03e214ef5209 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 1 Sep 2017 11:15:57 +0200 Subject: Use better security method description when using VeNCrypt The sub-modules generally provide a better description than just the short security method name. --- common/rfb/CSecurityVeNCrypt.cxx | 7 +++++++ common/rfb/CSecurityVeNCrypt.h | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) (limited to 'common') diff --git a/common/rfb/CSecurityVeNCrypt.cxx b/common/rfb/CSecurityVeNCrypt.cxx index 4a25245a..f9597cc7 100644 --- a/common/rfb/CSecurityVeNCrypt.cxx +++ b/common/rfb/CSecurityVeNCrypt.cxx @@ -191,6 +191,13 @@ bool CSecurityVeNCrypt::processMsg(CConnection* cc) return csecurity->processMsg(cc); } +const char* CSecurityVeNCrypt::description() const +{ + if (csecurity) + return csecurity->description(); + return "VeNCrypt"; +} + bool CSecurityVeNCrypt::isSecure() const { if (csecurity && csecurity->isSecure()) diff --git a/common/rfb/CSecurityVeNCrypt.h b/common/rfb/CSecurityVeNCrypt.h index 1ff0c020..6d978c75 100644 --- a/common/rfb/CSecurityVeNCrypt.h +++ b/common/rfb/CSecurityVeNCrypt.h @@ -38,7 +38,7 @@ namespace rfb { ~CSecurityVeNCrypt(); virtual bool processMsg(CConnection* cc);// { return true; } int getType() const {return chosenType;} - virtual const char* description() const { return secTypeName(chosenType); } + virtual const char* description() const; virtual bool isSecure() const; protected: -- cgit v1.2.3