From e51956e2c73022f139828c78d920c9a9b897bfb0 Mon Sep 17 00:00:00 2001 From: sithlord48 Date: Fri, 29 Nov 2024 08:19:41 -0500 Subject: [PATCH] feat: Client ID verification based on barrier: 229abab Fixes: CVE-2021-42072, CVE-2021-42073 --- src/apps/deskflow-gui/MainWindow.cpp | 10 ++-- .../dialogs/FingerprintDialog.cpp | 31 +++++++----- .../deskflow-gui/dialogs/FingerprintDialog.h | 3 +- src/lib/client/Client.cpp | 2 +- src/lib/common/constants.h.in | 1 + src/lib/deskflow/ArgParser.cpp | 2 + src/lib/deskflow/ServerApp.cpp | 6 ++- src/lib/deskflow/ServerArgs.h | 1 + src/lib/gui/core/CoreProcess.cpp | 4 ++ src/lib/net/SecureSocket.cpp | 47 ++++++++++++++----- src/lib/net/SecureSocket.h | 3 +- src/lib/net/SecurityLevel.h | 3 +- 12 files changed, 79 insertions(+), 34 deletions(-) diff --git a/src/apps/deskflow-gui/MainWindow.cpp b/src/apps/deskflow-gui/MainWindow.cpp index e8d232484..57f76ab9d 100644 --- a/src/apps/deskflow-gui/MainWindow.cpp +++ b/src/apps/deskflow-gui/MainWindow.cpp @@ -718,7 +718,7 @@ void MainWindow::checkConnected(const QString &line) void MainWindow::checkFingerprint(const QString &line) { - static const QRegularExpression re(R"(.*server fingerprint: \(SHA1\) ([A-F0-9:]+) \(SHA256\) ([A-F0-9:]+))"); + static const QRegularExpression re(R"(.*peer fingerprint: \(SHA1\) ([A-F0-9:]+) \(SHA256\) ([A-F0-9:]+))"); auto match = re.match(line); if (!match.hasMatch()) { return; @@ -735,7 +735,10 @@ void MainWindow::checkFingerprint(const QString &line) }; // Only Save the sha256 - auto localPath = QStringLiteral("%1/%2").arg(getTlsPath(), kFingerprintTrustedServersFilename).toStdString(); + const bool isClient = m_coreProcess.mode() == CoreMode::Client; + const auto trustFile = isClient ? kFingerprintTrustedServersFilename : kFingerprintTrustedClientsFilename; + const auto localPath = QStringLiteral("%1/%2").arg(getTlsPath(), trustFile).toStdString(); + deskflow::FingerprintDatabase db; db.read(localPath); if (db.isTrusted(sha256)) { @@ -744,7 +747,8 @@ void MainWindow::checkFingerprint(const QString &line) m_coreProcess.stop(); const QList fingerprints{sha1, sha256}; - FingerprintDialog fingerprintDialog(this, fingerprints, FingerprintDialogMode::Remote); + auto dialogMode = isClient ? FingerprintDialogMode::Client : FingerprintDialogMode::Server; + FingerprintDialog fingerprintDialog(this, fingerprints, dialogMode); if (fingerprintDialog.exec() == QDialog::Accepted) { db.addTrusted(sha256); db.write(localPath); diff --git a/src/apps/deskflow-gui/dialogs/FingerprintDialog.cpp b/src/apps/deskflow-gui/dialogs/FingerprintDialog.cpp index 6427f150c..ef5b4f2d3 100644 --- a/src/apps/deskflow-gui/dialogs/FingerprintDialog.cpp +++ b/src/apps/deskflow-gui/dialogs/FingerprintDialog.cpp @@ -20,22 +20,27 @@ FingerprintDialog::FingerprintDialog( { ui->setupUi(this); - if (mode == FingerprintDialogMode::Remote) { - setWindowTitle(tr("Security Question")); - ui->lblBody->setText(tr("

You are connecting to a server.

" - "

Compare the fingerprints below to the ones on your server. " - "If the two don't match exactly, then it's probably not the server " - "you're expecting (it could be a malicious user).\n

")); - - ui->lblFooter->setText(tr("

Do you want to trust this fingerprint for future " - "connections? If you don't, a connection cannot be made.

")); - ui->buttonBox->setStandardButtons(QDialogButtonBox::Yes | QDialogButtonBox::No); - connect(ui->buttonBox->button(QDialogButtonBox::Yes), &QPushButton::clicked, this, &QDialog::accept); - connect(ui->buttonBox->button(QDialogButtonBox::No), &QPushButton::clicked, this, &QDialog::reject); - } else { + if (mode == FingerprintDialogMode::Local) { setWindowTitle(tr("Your Fingerprints")); ui->lblBody->setText(tr("These are the fingerprints for this computer")); ui->lblFooter->setVisible(false); + } else { + auto body = tr("Compare the fingerprints in this dialog to those on the %1.\n" + "Only accept this dialog if they match!"); + + if (mode == FingerprintDialogMode::Server) { + ui->lblBody->setText(tr("A new Client is connecting\n%1").arg(body.arg(tr("client")))); + } else { + ui->lblBody->setText(tr("You are connecting to a new server\n%1").arg(body.arg(tr("server")))); + } + + setWindowTitle(tr("Security Question")); + ui->lblFooter->setText(tr("

Do you want to trust this fingerprint for future " + "connections? If you don't, a connection cannot be made.

")); + + ui->buttonBox->setStandardButtons(QDialogButtonBox::Yes | QDialogButtonBox::No); + connect(ui->buttonBox->button(QDialogButtonBox::Yes), &QPushButton::clicked, this, &QDialog::accept); + connect(ui->buttonBox->button(QDialogButtonBox::No), &QPushButton::clicked, this, &QDialog::reject); } for (const auto &fingerprint : fingerprints) { diff --git a/src/apps/deskflow-gui/dialogs/FingerprintDialog.h b/src/apps/deskflow-gui/dialogs/FingerprintDialog.h index 4727e1f4b..3b5274296 100644 --- a/src/apps/deskflow-gui/dialogs/FingerprintDialog.h +++ b/src/apps/deskflow-gui/dialogs/FingerprintDialog.h @@ -18,7 +18,8 @@ class FingerprintDialog; enum FingerprintDialogMode { Local, - Remote + Client, + Server }; class FingerprintDialog : public QDialog diff --git a/src/lib/client/Client.cpp b/src/lib/client/Client.cpp index ed28b794a..dd6200fa0 100644 --- a/src/lib/client/Client.cpp +++ b/src/lib/client/Client.cpp @@ -131,7 +131,7 @@ void Client::connect(size_t addressIndex) return; } - auto securityLevel = m_useSecureNetwork ? SecurityLevel::Encrypted : SecurityLevel::PlainText; + auto securityLevel = m_useSecureNetwork ? SecurityLevel::PeerAuth : SecurityLevel::PlainText; try { if (m_args.m_hostMode) { diff --git a/src/lib/common/constants.h.in b/src/lib/common/constants.h.in index ddbe2879c..4d75ea0ce 100644 --- a/src/lib/common/constants.h.in +++ b/src/lib/common/constants.h.in @@ -32,3 +32,4 @@ const auto kTlsDbSize = 2; const auto kCertificateFilename = "@CMAKE_PROJECT_NAME@.pem"; const auto kFingerprintLocalFilename = "local-fingerprint"; const auto kFingerprintTrustedServersFilename = "trusted-servers"; +const auto kFingerprintTrustedClientsFilename = "trusted-clients"; diff --git a/src/lib/deskflow/ArgParser.cpp b/src/lib/deskflow/ArgParser.cpp index 364a825ab..fe9776888 100644 --- a/src/lib/deskflow/ArgParser.cpp +++ b/src/lib/deskflow/ArgParser.cpp @@ -43,6 +43,8 @@ bool ArgParser::parseServerArgs(deskflow::ServerArgs &args, int argc, const char } else if (isArg(i, argc, argv, nullptr, "server")) { ++i; continue; + } else if (isArg(i, argc, argv, nullptr, "--disable-client-cert-check")) { + args.m_chkPeerCert = false; } else { LOG((CLOG_CRIT "%s: unrecognized option `%s'" BYE, args.m_pname, argv[i], args.m_pname)); return false; diff --git a/src/lib/deskflow/ServerApp.cpp b/src/lib/deskflow/ServerApp.cpp index 9715c8882..c701ee2dc 100644 --- a/src/lib/deskflow/ServerApp.cpp +++ b/src/lib/deskflow/ServerApp.cpp @@ -136,6 +136,9 @@ void ServerApp::help() << " -a, --address
listen for clients on the given address.\n" << " -c, --config use the named configuration file " << "instead.\n" HELP_COMMON_INFO_1 + << " --disable-client-cert-check disable client SSL certificate \n" + " checking (deprecated)\n" + << HELP_SYS_INFO HELP_COMMON_INFO_2 << "\n" #if WINAPI_XWINDOWS << " --display when in X mode, connect to the X server\n" @@ -612,7 +615,8 @@ void ServerApp::handleResume(const Event &, void *) ClientListener *ServerApp::openClientListener(const NetworkAddress &address) { - auto securityLevel = args().m_enableCrypto ? SecurityLevel::Encrypted : SecurityLevel::PlainText; + auto securityLevel = args().m_enableCrypto ? args().m_chkPeerCert ? SecurityLevel::PeerAuth : SecurityLevel::Encrypted + : SecurityLevel::PlainText; ClientListener *listen = new ClientListener(getAddress(address), getSocketFactory(), m_events, securityLevel); diff --git a/src/lib/deskflow/ServerArgs.h b/src/lib/deskflow/ServerArgs.h index 2935f0e49..0aa6a100b 100644 --- a/src/lib/deskflow/ServerArgs.h +++ b/src/lib/deskflow/ServerArgs.h @@ -29,6 +29,7 @@ public: public: std::string m_configFile = ""; std::shared_ptr m_config; + bool m_chkPeerCert = true; }; } // namespace deskflow diff --git a/src/lib/gui/core/CoreProcess.cpp b/src/lib/gui/core/CoreProcess.cpp index 4b9b1d197..c4bb8ac31 100644 --- a/src/lib/gui/core/CoreProcess.cpp +++ b/src/lib/gui/core/CoreProcess.cpp @@ -554,6 +554,10 @@ bool CoreProcess::addServerArgs(QStringList &args, QString &app) args << "--log" << m_appConfig.logFilename(); } + if (!m_appConfig.requireClientCerts()) { + args << "--disable-client-cert-check"; + } + QString configFilename = persistServerConfig(); if (configFilename.isEmpty()) { qFatal("config file name empty for server args"); diff --git a/src/lib/net/SecureSocket.cpp b/src/lib/net/SecureSocket.cpp index fe4026bef..0c713547b 100644 --- a/src/lib/net/SecureSocket.cpp +++ b/src/lib/net/SecureSocket.cpp @@ -50,6 +50,11 @@ struct Ssl SSL *m_ssl; }; +static int verifyIgnoreCertCallback(X509_STORE_CTX *, void *) +{ + return 1; +} + SecureSocket::SecureSocket( IEventQueue *events, SocketMultiplexer *socketMultiplexer, IArchNetwork::EAddressFamily family, SecurityLevel securityLevel @@ -370,6 +375,13 @@ void SecureSocket::initContext(bool server) if (m_ssl->m_context == NULL) { SslLogger::logError(); } + + if (m_securityLevel == SecurityLevel::PeerAuth) { + // We want to ask for peer certificate, but not verify it. If we don't ask for peer + // certificate, e.g. client won't send it. + SSL_CTX_set_verify(m_ssl->m_context, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, nullptr); + SSL_CTX_set_cert_verify_callback(m_ssl->m_context, verifyIgnoreCertCallback, nullptr); + } } void SecureSocket::createSSL() @@ -435,6 +447,16 @@ int SecureSocket::secureAccept(int socket) // If not fatal and no retry, state is good if (retry == 0) { + if (m_securityLevel == SecurityLevel::PeerAuth) { + std::string dbDir = deskflow::string::sprintf( + "%s/%s/%s", ARCH->getProfileDirectory().c_str(), kSslDir, kFingerprintTrustedClientsFilename + ); + if (!verifyCertFingerprint(dbDir)) { + retry = 0; + disconnect(); + return -1; // Fail + } + } m_secureReady = true; LOG((CLOG_INFO "accepted secure socket")); SslLogger::logSecureCipherInfo(m_ssl->m_ssl); @@ -457,7 +479,6 @@ int SecureSocket::secureAccept(int socket) int SecureSocket::secureConnect(int socket) { - std::lock_guard ssl_lock{ssl_mutex_}; std::string certDir = deskflow::string::sprintf("%s/%s/%s", ARCH->getProfileDirectory().c_str(), kSslDir, kCertificateFilename); @@ -468,6 +489,8 @@ int SecureSocket::secureConnect(int socket) return -1; } + std::lock_guard ssl_lock{ssl_mutex_}; + createSSL(); // attach the socket descriptor @@ -501,7 +524,10 @@ int SecureSocket::secureConnect(int socket) retry = 0; // No error, set ready, process and return ok m_secureReady = true; - if (verifyCertFingerprint()) { + std::string dbDir = deskflow::string::sprintf( + "%s/%s/%s", ARCH->getProfileDirectory().c_str(), kSslDir, kFingerprintTrustedServersFilename + ); + if (verifyCertFingerprint(dbDir)) { LOG((CLOG_INFO "connected to secure socket")); if (!showCertificate()) { disconnect(); @@ -624,7 +650,7 @@ void SecureSocket::disconnect() sendEvent(getEvents()->forIStream().inputShutdown()); } -bool SecureSocket::verifyCertFingerprint() +bool SecureSocket::verifyCertFingerprint(const deskflow::fs::path &fingerprintDbPath) { deskflow::FingerprintData sha1; deskflow::FingerprintData sha256; @@ -639,25 +665,20 @@ bool SecureSocket::verifyCertFingerprint() // Gui Must Parse these two lines, DO NOT CHANGE LOG( - (CLOG_NOTE "server fingerprint: (SHA1) %s (SHA256) %s", deskflow::formatSSLFingerprint(sha1.data).c_str(), + (CLOG_NOTE "peer fingerprint: (SHA1) %s (SHA256) %s", deskflow::formatSSLFingerprint(sha1.data).c_str(), deskflow::formatSSLFingerprint(sha256.data).c_str()) ); - std::string trustedServersFilename = deskflow::string::sprintf( - "%s/%s/%s", ARCH->getProfileDirectory().c_str(), kSslDir, kFingerprintTrustedServersFilename - ); - // check if this fingerprint exist - std::string fileLine; std::ifstream file; - deskflow::openUtf8Path(file, trustedServersFilename); + deskflow::openUtf8Path(file, fingerprintDbPath); deskflow::FingerprintDatabase db; - db.read(trustedServersFilename); + db.read(fingerprintDbPath); if (!db.fingerprints().empty()) { - LOG((CLOG_NOTE "read %d fingerprints from %s", db.fingerprints().size(), trustedServersFilename.c_str())); + LOG((CLOG_NOTE "read %d fingerprints from %s", db.fingerprints().size(), fingerprintDbPath.c_str())); } else { - LOG((CLOG_ERR "failed to open trusted fingerprints file: %s", trustedServersFilename.c_str())); + LOG((CLOG_ERR "failed to open trusted fingerprints file: %s", fingerprintDbPath.c_str())); return false; } diff --git a/src/lib/net/SecureSocket.h b/src/lib/net/SecureSocket.h index 0206e4c4d..6bd9469f0 100644 --- a/src/lib/net/SecureSocket.h +++ b/src/lib/net/SecureSocket.h @@ -7,6 +7,7 @@ #pragma once +#include "io/filesystem.h" #include "net/SecurityLevel.h" #include "net/TCPSocket.h" #include "net/XSocket.h" @@ -75,7 +76,7 @@ private: bool showCertificate() const; void checkResult(int n, int &retry); void disconnect(); - bool verifyCertFingerprint(); + bool verifyCertFingerprint(const deskflow::fs::path &fingerprintDbPath); ISocketMultiplexerJob *serviceConnect(ISocketMultiplexerJob *, bool, bool, bool); diff --git a/src/lib/net/SecurityLevel.h b/src/lib/net/SecurityLevel.h index 7866f31e1..1ad434ff7 100644 --- a/src/lib/net/SecurityLevel.h +++ b/src/lib/net/SecurityLevel.h @@ -13,5 +13,6 @@ enum class SecurityLevel { PlainText, /** Connections will not be encrypted */ - Encrypted /** Connections will be encrypted */ + Encrypted, /** Connections will be encrypted */ + PeerAuth /** Connections will be encrypted and peers must be authenticated */ };