feat: Client ID verification
based on barrier: 229abab Fixes: CVE-2021-42072, CVE-2021-42073
This commit is contained in:
@ -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<deskflow::FingerprintData> 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);
|
||||
|
||||
@ -20,22 +20,27 @@ FingerprintDialog::FingerprintDialog(
|
||||
{
|
||||
ui->setupUi(this);
|
||||
|
||||
if (mode == FingerprintDialogMode::Remote) {
|
||||
setWindowTitle(tr("Security Question"));
|
||||
ui->lblBody->setText(tr("<p>You are connecting to a server.</p>"
|
||||
"<p>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</p>"));
|
||||
|
||||
ui->lblFooter->setText(tr("<p>Do you want to trust this fingerprint for future "
|
||||
"connections? If you don't, a connection cannot be made.</p>"));
|
||||
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("<p>Do you want to trust this fingerprint for future "
|
||||
"connections? If you don't, a connection cannot be made.</p>"));
|
||||
|
||||
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) {
|
||||
|
||||
@ -18,7 +18,8 @@ class FingerprintDialog;
|
||||
enum FingerprintDialogMode
|
||||
{
|
||||
Local,
|
||||
Remote
|
||||
Client,
|
||||
Server
|
||||
};
|
||||
|
||||
class FingerprintDialog : public QDialog
|
||||
|
||||
@ -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) {
|
||||
|
||||
@ -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";
|
||||
|
||||
@ -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;
|
||||
|
||||
@ -136,6 +136,9 @@ void ServerApp::help()
|
||||
<< " -a, --address <address> listen for clients on the given address.\n"
|
||||
<< " -c, --config <pathname> 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 <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);
|
||||
|
||||
|
||||
@ -29,6 +29,7 @@ public:
|
||||
public:
|
||||
std::string m_configFile = "";
|
||||
std::shared_ptr<Config> m_config;
|
||||
bool m_chkPeerCert = true;
|
||||
};
|
||||
|
||||
} // namespace deskflow
|
||||
|
||||
@ -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");
|
||||
|
||||
@ -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<std::mutex> 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<std::mutex> 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;
|
||||
}
|
||||
|
||||
|
||||
@ -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);
|
||||
|
||||
|
||||
@ -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 */
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user