From daf3d88aa1b554c5d6d41116c51517d30cb7c4b7 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 1 Sep 2017 11:14:35 +0200 Subject: [PATCH] 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 +- media/insecure.svg | 107 +++++++++++++++++++++++++++++++ media/insecure.xpm | 71 ++++++++++++++++++++ media/secure.svg | 83 ++++++++++++++++++++++++ media/secure.xpm | 56 ++++++++++++++++ vncviewer/UserDialog.cxx | 31 ++++++++- vncviewer/UserDialog.h | 2 +- 16 files changed, 370 insertions(+), 7 deletions(-) create mode 100644 media/insecure.svg create mode 100644 media/insecure.xpm create mode 100644 media/secure.svg create mode 100644 media/secure.xpm 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 diff --git a/media/insecure.svg b/media/insecure.svg new file mode 100644 index 00000000..1c5f6fa7 --- /dev/null +++ b/media/insecure.svg @@ -0,0 +1,107 @@ + + + + + + + + + + + + image/svg+xml + + + + + + + + + + + + + + diff --git a/media/insecure.xpm b/media/insecure.xpm new file mode 100644 index 00000000..f5053fe6 --- /dev/null +++ b/media/insecure.xpm @@ -0,0 +1,71 @@ +/* XPM */ +static char *insecure[] = { +/* columns rows colors chars-per-pixel */ +"15 15 50 1 ", +" c black", +". c #020000", +"X c #050000", +"o c #080000", +"O c #0A0000", +"+ c #0C0000", +"@ c #0D0000", +"# c #0F0000", +"$ c #100000", +"% c #110000", +"& c #120000", +"* c #140000", +"= c #290000", +"- c #330000", +"; c #370000", +": c #430000", +"> c #560000", +", c #620000", +"< c #660000", +"1 c #6C0000", +"2 c #7D0000", +"3 c #800000", +"4 c #810000", +"5 c #840000", +"6 c #870000", +"7 c #950000", +"8 c #A20000", +"9 c #AB0000", +"0 c #B30000", +"q c #B40000", +"w c #C00000", +"e c #C40000", +"r c #CD0000", +"t c #DC0000", +"y c #DD0000", +"u c #DF0000", +"i c #E40000", +"p c #E50000", +"a c #E60000", +"s c #EA0000", +"d c #EB0000", +"f c #ED0000", +"g c #F00000", +"h c #F40000", +"j c #F90000", +"k c #FA0000", +"l c #FB0000", +"z c #FC0000", +"x c #FD0000", +"c c red", +/* pixels */ +"ccccccccccccccc", +"ccccccjpkcr:fcc", +"ccccz6+ @1$ rcc", +"cccc2 O-o wccc", +"cccdX%tr# 4cccc", +"ccce >r& 7czccc", +"ccq< =O 8cg60cc", +"cs. 3cg; ac", +"cu 3cg; uc", +"cu 3cg; uc", +"c0 3cg; uc", +"r$ 3cg; uc", +", 5cg; *hc", +"l9lxiyyyyyyyhcc", +"ccccccccccccccc" +}; diff --git a/media/secure.svg b/media/secure.svg new file mode 100644 index 00000000..4da10750 --- /dev/null +++ b/media/secure.svg @@ -0,0 +1,83 @@ + + + + + + + + + + + + image/svg+xml + + + + + + + + + + + diff --git a/media/secure.xpm b/media/secure.xpm new file mode 100644 index 00000000..49a3791f --- /dev/null +++ b/media/secure.xpm @@ -0,0 +1,56 @@ +/* XPM */ +static char *secure[] = { +/* columns rows colors chars-per-pixel */ +"15 15 35 1 ", +" c black", +". c #000200", +"X c #000500", +"o c #000A00", +"O c #000C00", +"+ c #000D00", +"@ c #001000", +"# c #001100", +"$ c #001400", +"% c #003300", +"& c #005500", +"* c #005600", +"= c #006600", +"- c #007D00", +"; c #007E00", +": c #008700", +"> c #008800", +", c #00B300", +"< c #00B400", +"1 c #00C400", +"2 c #00DB00", +"3 c #00DC00", +"4 c #00DD00", +"5 c #00DF00", +"6 c #00E500", +"7 c #00E600", +"8 c #00EA00", +"9 c #00EB00", +"0 c #00ED00", +"q c #00F000", +"w c #00F400", +"e c #00F900", +"r c #00FA00", +"t c #00FC00", +"y c green", +/* pixels */ +"yyyyyyyyyyyyyyy", +"yyyyyye6ryyyyyy", +"yyyyt:O +>tyyyy", +"yyyy- o%o ;yyyy", +"yyy9X#3y2@X0yyy", +"yyy1 *yyy& 1yyy", +"yy<= %>>>% =,yy", +"y8. 7y", +"y5 5y", +"y5 5y", +"y5 5y", +"y5 5y", +"yw# $wy", +"yyq444444444wyy", +"yyyyyyyyyyyyyyy" +}; diff --git a/vncviewer/UserDialog.cxx b/vncviewer/UserDialog.cxx index cf6893c8..640f2a98 100644 --- a/vncviewer/UserDialog.cxx +++ b/vncviewer/UserDialog.cxx @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -42,8 +43,18 @@ #include "parameters.h" #include "UserDialog.h" +/* xpm:s predate const, so they have invalid definitions */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wwrite-strings" +#include "../media/secure.xpm" +#include "../media/insecure.xpm" +#pragma GCC diagnostic pop + using namespace rfb; +static Fl_Pixmap secure_icon(secure); +static Fl_Pixmap insecure_icon(insecure); + static int ret_val = 0; static void button_cb(Fl_Widget *widget, void *val) { @@ -59,7 +70,7 @@ UserDialog::~UserDialog() { } -void UserDialog::getUserPasswd(char** user, char** password) +void UserDialog::getUserPasswd(bool secure, char** user, char** password) { CharArray passwordFileStr(passwordFile.getData()); @@ -83,6 +94,7 @@ void UserDialog::getUserPasswd(char** user, char** password) } Fl_Window *win; + Fl_Box *banner; Fl_Input *username; Fl_Secret_Input *passwd; Fl_Box *icon; @@ -93,9 +105,22 @@ void UserDialog::getUserPasswd(char** user, char** password) win = new Fl_Window(410, 145, _("VNC authentication")); win->callback(button_cb,(void *)0); - y = 10; + banner = new Fl_Box(0, 0, win->w(), 20); + banner->align(FL_ALIGN_CENTER|FL_ALIGN_INSIDE|FL_ALIGN_IMAGE_NEXT_TO_TEXT); + banner->box(FL_FLAT_BOX); + if (secure) { + banner->label(_("This connection is secure")); + banner->color(FL_GREEN); + banner->image(secure_icon); + } else { + banner->label(_("This connection is not secure")); + banner->color(FL_RED); + banner->image(insecure_icon); + } + + y = 20 + 10; - icon = new Fl_Box(10, 10, 50, 50, "?"); + icon = new Fl_Box(10, y, 50, 50, "?"); icon->box(FL_UP_BOX); icon->labelfont(FL_TIMES_BOLD); icon->labelsize(34); diff --git a/vncviewer/UserDialog.h b/vncviewer/UserDialog.h index c6756a8e..b62ba7f3 100644 --- a/vncviewer/UserDialog.h +++ b/vncviewer/UserDialog.h @@ -31,7 +31,7 @@ public: // UserPasswdGetter callbacks - void getUserPasswd(char** user, char** password); + void getUserPasswd(bool secure, char** user, char** password); // UserMsgBox callbacks -- 2.39.5