From 7607ce29629734f7e0b33a92669909dac7c6f6ec Mon Sep 17 00:00:00 2001 From: Kang Lin Date: Mon, 12 Aug 2024 14:27:08 +0800 Subject: [PATCH] CSecurityTLS: change the variable that x509 authentication CA and CRL file from global to local --- common/rfb/CConnection.h | 7 ++++++- common/rfb/CSecurityTLS.cxx | 41 +++++++++++-------------------------- common/rfb/CSecurityTLS.h | 3 --- tests/perf/decperf.cxx | 6 ++++++ tests/perf/encperf.cxx | 6 ++++++ vncviewer/CConn.cxx | 9 +++++++- vncviewer/CConn.h | 10 ++++----- vncviewer/OptionsDialog.cxx | 8 ++++---- vncviewer/parameters.cxx | 25 ++++++++++++++++++++-- vncviewer/parameters.h | 5 +++++ 10 files changed, 74 insertions(+), 46 deletions(-) diff --git a/common/rfb/CConnection.h b/common/rfb/CConnection.h index 9c805301ce..4794c46655 100644 --- a/common/rfb/CConnection.h +++ b/common/rfb/CConnection.h @@ -145,7 +145,12 @@ namespace rfb { // case no user name will be retrieved. virtual void getUserPasswd(bool secure, std::string* user, std::string* password) = 0; - + /*! + * Get x509 authentication CA and CRL file, the file format is pem. + * \param ca: certificate authority, the file format is pem. + * \param crl: certificate revocation list, the file format is pem. + */ + virtual int getX509File(std::string* ca, std::string* crl) = 0; virtual bool showMsgBox(MsgBoxFlags flags, const char *title, const char *text) = 0; // authSuccess() is called when authentication has succeeded. diff --git a/common/rfb/CSecurityTLS.cxx b/common/rfb/CSecurityTLS.cxx index bb02aaa692..39fcef81fa 100644 --- a/common/rfb/CSecurityTLS.cxx +++ b/common/rfb/CSecurityTLS.cxx @@ -58,30 +58,8 @@ using namespace rfb; -static const char* configdirfn(const char* fn); - -StringParameter CSecurityTLS::X509CA("X509CA", "X509 CA certificate", - configdirfn("x509_ca.pem"), - ConfViewer); -StringParameter CSecurityTLS::X509CRL("X509CRL", "X509 CRL file", - configdirfn("x509_crl.pem"), - ConfViewer); - static LogWriter vlog("TLS"); -static const char* configdirfn(const char* fn) -{ - static char full_path[PATH_MAX]; - const char* configdir; - - configdir = os::getvncconfigdir(); - if (configdir == nullptr) - return ""; - - snprintf(full_path, sizeof(full_path), "%s/%s", configdir, fn); - return full_path; -} - CSecurityTLS::CSecurityTLS(CConnection* cc_, bool _anon) : CSecurity(cc_), session(nullptr), anon_cred(nullptr), cert_cred(nullptr), @@ -284,12 +262,17 @@ void CSecurityTLS::setParam() if (gnutls_certificate_set_x509_system_trust(cert_cred) < 1) vlog.error("Could not load system certificate trust store"); - - if (gnutls_certificate_set_x509_trust_file(cert_cred, X509CA, GNUTLS_X509_FMT_PEM) < 0) - vlog.error("Could not load user specified certificate authority"); - - if (gnutls_certificate_set_x509_crl_file(cert_cred, X509CRL, GNUTLS_X509_FMT_PEM) < 0) - vlog.error("Could not load user specified certificate revocation list"); + + std::string ca, crl; + int nRet = client->getX509File(&ca, &crl); + if(nRet) + throw AuthFailureException("Get X509 certificate file fail"); + if(!ca.empty()) + if (gnutls_certificate_set_x509_trust_file(cert_cred, ca.c_str(), GNUTLS_X509_FMT_PEM) < 0) + vlog.error("Could not load user specified certificate authority"); + if(!crl.empty()) + if (gnutls_certificate_set_x509_crl_file(cert_cred, crl.c_str(), GNUTLS_X509_FMT_PEM) < 0) + vlog.error("Could not load user specified certificate revocation list"); ret = gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, cert_cred); if (ret != GNUTLS_E_SUCCESS) @@ -471,7 +454,7 @@ void CSecurityTLS::checkSession() "\n" "Do you want to make an exception for this " "server?", info.data); - + if (!cc->showMsgBox(MsgBoxFlags::M_YESNO, "Certificate is not yet valid", text.c_str())) diff --git a/common/rfb/CSecurityTLS.h b/common/rfb/CSecurityTLS.h index 848ef9bb8c..857ea6950d 100644 --- a/common/rfb/CSecurityTLS.h +++ b/common/rfb/CSecurityTLS.h @@ -41,9 +41,6 @@ namespace rfb { int getType() const override { return anon ? secTypeTLSNone : secTypeX509None; } bool isSecure() const override { return !anon; } - static StringParameter X509CA; - static StringParameter X509CRL; - protected: void shutdown(); void freeResources(); diff --git a/tests/perf/decperf.cxx b/tests/perf/decperf.cxx index b3759eb2fc..057c892e04 100644 --- a/tests/perf/decperf.cxx +++ b/tests/perf/decperf.cxx @@ -78,6 +78,7 @@ class CConn : public rfb::CConnection { void serverCutText(const char*) override; virtual void getUserPasswd(bool secure, std::string *user, std::string *password) override; virtual bool showMsgBox(rfb::MsgBoxFlags flags, const char *title, const char *text) override; + virtual int getX509File(std::string *ca, std::string *crl) override; public: double cpuTime; @@ -188,6 +189,11 @@ void CConn::getUserPasswd(bool, std::string *, std::string *) { } +int CConn::getX509File(std::string *, std::string *) +{ + return 0; +} + bool CConn::showMsgBox(rfb::MsgBoxFlags, const char *, const char *) { return true; diff --git a/tests/perf/encperf.cxx b/tests/perf/encperf.cxx index 87efc16575..07855a9b8b 100644 --- a/tests/perf/encperf.cxx +++ b/tests/perf/encperf.cxx @@ -110,6 +110,7 @@ class CConn : public rfb::CConnection { void serverCutText(const char*) override; virtual void getUserPasswd(bool secure, std::string *user, std::string *password) override; virtual bool showMsgBox(rfb::MsgBoxFlags flags, const char *title, const char *text) override; + virtual int getX509File(std::string *ca, std::string *crl) override; public: double decodeTime; @@ -285,6 +286,11 @@ void CConn::getUserPasswd(bool, std::string *, std::string *) { } +int CConn::getX509File(std::string *, std::string *) +{ + return 0; +} + bool CConn::showMsgBox(rfb::MsgBoxFlags, const char *, const char *) { return true; diff --git a/vncviewer/CConn.cxx b/vncviewer/CConn.cxx index 171123937b..2748dd1863 100644 --- a/vncviewer/CConn.cxx +++ b/vncviewer/CConn.cxx @@ -615,4 +615,11 @@ bool CConn::showMsgBox(MsgBoxFlags flags, const char *title, const char *text) void CConn::getUserPasswd(bool secure, std::string *user, std::string *password) { dlg.getUserPasswd(secure, user, password); -} \ No newline at end of file +} + +int CConn::getX509File(std::string *ca, std::string *crl) +{ + *ca = ::X509CA; + *crl = ::X509CRL; + return 0; +} diff --git a/vncviewer/CConn.h b/vncviewer/CConn.h index 498f5d740b..6933355bc1 100644 --- a/vncviewer/CConn.h +++ b/vncviewer/CConn.h @@ -44,14 +44,12 @@ class CConn : public rfb::CConnection // Callback when socket is ready (or broken) static void socketEvent(FL_SOCKET fd, void *data); - - // UserMsgBox interface + + // CConnection callback methods virtual bool showMsgBox(rfb::MsgBoxFlags flags, const char *title, const char *text) override; - - // UserPasswdGetter interface + virtual int getX509File(std::string *ca, std::string *crl) override; virtual void getUserPasswd(bool secure, std::string *user, std::string *password) override; - - // CConnection callback methods + void initDone() override; void setDesktopSize(int w, int h) override; diff --git a/vncviewer/OptionsDialog.cxx b/vncviewer/OptionsDialog.cxx index e04065eced..7cd0f68336 100644 --- a/vncviewer/OptionsDialog.cxx +++ b/vncviewer/OptionsDialog.cxx @@ -303,8 +303,8 @@ void OptionsDialog::loadOptions(void) } #ifdef HAVE_GNUTLS - caInput->value(CSecurityTLS::X509CA); - crlInput->value(CSecurityTLS::X509CRL); + caInput->value(::X509CA); + crlInput->value(::X509CRL); handleX509(encX509Checkbox, this); #endif @@ -434,8 +434,8 @@ void OptionsDialog::storeOptions(void) security.EnableSecType(secTypeX509Plain); } - CSecurityTLS::X509CA.setParam(caInput->value()); - CSecurityTLS::X509CRL.setParam(crlInput->value()); + ::X509CA.setParam(caInput->value()); + ::X509CRL.setParam(crlInput->value()); #endif #ifdef HAVE_NETTLE diff --git a/vncviewer/parameters.cxx b/vncviewer/parameters.cxx index a03623db74..86281f971d 100644 --- a/vncviewer/parameters.cxx +++ b/vncviewer/parameters.cxx @@ -52,6 +52,7 @@ using namespace std; static LogWriter vlog("Parameters"); +static const char* configdirfn(const char* fn); IntParameter pointerEventInterval("PointerEventInterval", "Time in milliseconds to rate-limit" @@ -76,6 +77,13 @@ StringParameter passwordFile("PasswordFile", "Password file for VNC authentication", ""); AliasParameter passwd("passwd", "Alias for PasswordFile", &passwordFile); +StringParameter X509CA("X509CA", "X509 CA certificate", + configdirfn("x509_ca.pem"), + ConfViewer); +StringParameter X509CRL("X509CRL", "X509 CRL file", + configdirfn("x509_crl.pem"), + ConfViewer); + BoolParameter autoSelect("AutoSelect", "Auto select pixel format and encoding. " "Default if PreferredEncoding and FullColor are not specified.", @@ -175,8 +183,8 @@ static const char* IDENTIFIER_STRING = "TigerVNC Configuration file Version 1.0" static VoidParameter* parameterArray[] = { /* Security */ #ifdef HAVE_GNUTLS - &CSecurityTLS::X509CA, - &CSecurityTLS::X509CRL, + &X509CA, + &X509CRL, #endif // HAVE_GNUTLS &SecurityClient::secTypes, /* Misc. */ @@ -221,6 +229,19 @@ static const struct EscapeMap { { '\r', 'r' }, { '\\', '\\' } }; +static const char* configdirfn(const char* fn) +{ + static char full_path[PATH_MAX]; + const char* configdir; + + configdir = os::getvncconfigdir(); + if (configdir == nullptr) + return ""; + + snprintf(full_path, sizeof(full_path), "%s/%s", configdir, fn); + return full_path; +} + static bool encodeValue(const char* val, char* dest, size_t destSize) { size_t pos = 0; diff --git a/vncviewer/parameters.h b/vncviewer/parameters.h index df7bc42059..5c27d1f0d9 100644 --- a/vncviewer/parameters.h +++ b/vncviewer/parameters.h @@ -37,6 +37,11 @@ extern rfb::BoolParameter dotWhenNoCursor; extern rfb::StringParameter passwordFile; +#ifdef HAVE_GNUTLS +extern rfb::StringParameter X509CA; +extern rfb::StringParameter X509CRL; +#endif + extern rfb::BoolParameter autoSelect; extern rfb::BoolParameter fullColour; extern rfb::AliasParameter fullColourAlias;