From a428b61c7dfbd120f00f97c6a656ea3e858736ea Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Mon, 1 Nov 2021 02:52:52 +0200 Subject: [PATCH] gui: Add support for SHA256 fingerprints For the time being both SHA1 and SHA256 fingerprints will be shown in the UI. This allows users to verify new connections between old and new versions of Barrier. After the initial verification we use SHA256 fingerprints. The issue has been reported by Matthias Gerstner . --- .../fingerprint-randomart.feature | 3 + doc/newsfragments/sha256-fingerprints.bugfix | 4 + src/gui/src/MainWindow.cpp | 70 +++++++++++++--- src/gui/src/MainWindow.h | 2 + src/gui/src/MainWindowBase.ui | 83 ++++++++++++++++++- src/gui/src/SslCertificate.cpp | 8 +- src/lib/net/SecureSocket.cpp | 17 ++-- src/lib/net/SecureUtils.cpp | 23 +++++ src/lib/net/SecureUtils.h | 1 + 9 files changed, 185 insertions(+), 26 deletions(-) create mode 100644 doc/newsfragments/fingerprint-randomart.feature create mode 100644 doc/newsfragments/sha256-fingerprints.bugfix diff --git a/doc/newsfragments/fingerprint-randomart.feature b/doc/newsfragments/fingerprint-randomart.feature new file mode 100644 index 00000000..9ffced93 --- /dev/null +++ b/doc/newsfragments/fingerprint-randomart.feature @@ -0,0 +1,3 @@ +Added support for randomart images for easier comparison of SSL +certificate fingerprints. The algorithm is identical to what +OpenSSH uses. diff --git a/doc/newsfragments/sha256-fingerprints.bugfix b/doc/newsfragments/sha256-fingerprints.bugfix new file mode 100644 index 00000000..a724c3b5 --- /dev/null +++ b/doc/newsfragments/sha256-fingerprints.bugfix @@ -0,0 +1,4 @@ +Barrier now uses SHA256 fingerprints for establishing security of encrypted SSL connections. +After upgrading client to new version the existing server fingerprint will need to be approved again. +Client and server will show both SHA1 and SHA256 server fingerprints to allow interoperability +with older versions of Barrier. diff --git a/src/gui/src/MainWindow.cpp b/src/gui/src/MainWindow.cpp index b1d6fc83..ccb5c196 100644 --- a/src/gui/src/MainWindow.cpp +++ b/src/gui/src/MainWindow.cpp @@ -158,9 +158,22 @@ MainWindow::MainWindow(QSettings& settings, AppConfig& appConfig) : m_pComboServerList->hide(); m_pLabelPadlock->hide(); + frame_fingerprint_details->hide(); updateSSLFingerprint(); + connect(toolbutton_show_fingerprint, &QToolButton::clicked, [this](bool checked) + { + m_fingerprint_expanded = !m_fingerprint_expanded; + if (m_fingerprint_expanded) { + frame_fingerprint_details->show(); + toolbutton_show_fingerprint->setArrowType(Qt::ArrowType::UpArrow); + } else { + frame_fingerprint_details->hide(); + toolbutton_show_fingerprint->setArrowType(Qt::ArrowType::DownArrow); + } + }); + // resize window to smallest reasonable size resize(0, 0); } @@ -414,26 +427,32 @@ void MainWindow::checkConnected(const QString& line) void MainWindow::checkFingerprint(const QString& line) { - QRegExp fingerprintRegex(".*server fingerprint: ([A-F0-9:]+)"); + QRegExp fingerprintRegex(".*server fingerprint \\(SHA1\\): ([A-F0-9:]+) \\(SHA256\\): ([A-F0-9:]+)"); if (!fingerprintRegex.exactMatch(line)) { return; } - barrier::FingerprintData fingerprint = { + barrier::FingerprintData fingerprint_sha1 = { barrier::fingerprint_type_to_string(barrier::FingerprintType::SHA1), barrier::string::from_hex(fingerprintRegex.cap(1).toStdString()) }; + barrier::FingerprintData fingerprint_sha256 = { + barrier::fingerprint_type_to_string(barrier::FingerprintType::SHA256), + barrier::string::from_hex(fingerprintRegex.cap(2).toStdString()) + }; + auto db_path = DataDirectories::trusted_servers_ssl_fingerprints_path(); + // We compare only SHA256 fingerprints, but show both SHA1 and SHA256 so that the users can + // still verify fingerprints on old Barrier servers. This way the only time when we are exposed + // to SHA1 vulnerabilities is when the user is reconnecting again. barrier::FingerprintDatabase db; db.read(db_path); - if (db.is_trusted(fingerprint)) { + if (db.is_trusted(fingerprint_sha256)) { return; } - auto formatted_fingerprint = barrier::format_ssl_fingerprint(fingerprint.data); - static bool messageBoxAlreadyShown = false; if (!messageBoxAlreadyShown) { @@ -444,7 +463,11 @@ void MainWindow::checkFingerprint(const QString& line) QMessageBox::information( this, tr("Security question"), tr("Do you trust this fingerprint?\n\n" - "%1\n\n" + "SHA256:\n" + "%1\n" + "%2\n\n" + "SHA1 (obsolete, when using old Barrier server):\n" + "%3\n\n" "This is a server fingerprint. You should compare this " "fingerprint to the one on your server's screen. If the " "two don't match exactly, then it's probably not the server " @@ -452,12 +475,15 @@ void MainWindow::checkFingerprint(const QString& line) "To automatically trust this fingerprint for future " "connections, click Yes. To reject this fingerprint and " "disconnect from the server, click No.") - .arg(QString::fromStdString(formatted_fingerprint)), + .arg(QString::fromStdString(barrier::format_ssl_fingerprint(fingerprint_sha256.data))) + .arg(QString::fromStdString( + barrier::create_fingerprint_randomart(fingerprint_sha256.data))) + .arg(QString::fromStdString(barrier::format_ssl_fingerprint(fingerprint_sha1.data))), QMessageBox::Yes | QMessageBox::No); if (fingerprintReply == QMessageBox::Yes) { // restart core process after trusting fingerprint. - db.add_trusted(fingerprint); + db.add_trusted(fingerprint_sha256); db.write(db_path); startBarrier(); } @@ -987,6 +1013,7 @@ void MainWindow::updateSSLFingerprint() m_pSslCertificate->generateCertificate(); } + toolbutton_show_fingerprint->setEnabled(false); m_pLabelLocalFingerprint->setText("Disabled"); if (!m_AppConfig->getCryptoEnabled()) { @@ -1000,15 +1027,32 @@ void MainWindow::updateSSLFingerprint() barrier::FingerprintDatabase db; db.read(local_path); - if (db.fingerprints().empty()) { + if (db.fingerprints().size() != 2) { return; } - const auto& fingerprint = db.fingerprints().front(); - auto formatted_fingerprint = barrier::format_ssl_fingerprint(fingerprint.data); + for (const auto& fingerprint : db.fingerprints()) { + if (fingerprint.algorithm == "sha1") { + auto fingerprint_str = barrier::format_ssl_fingerprint(fingerprint.data); + label_sha1_fingerprint_full->setText(QString::fromStdString(fingerprint_str)); + continue; + } - m_pLabelLocalFingerprint->setText(QString::fromStdString(formatted_fingerprint)); - m_pLabelLocalFingerprint->setTextInteractionFlags(Qt::TextSelectableByMouse); + if (fingerprint.algorithm == "sha256") { + auto fingerprint_str = barrier::format_ssl_fingerprint(fingerprint.data); + fingerprint_str.resize(40); + fingerprint_str += " ..."; + + auto fingerprint_str_cols = barrier::format_ssl_fingerprint_columns(fingerprint.data); + auto fingerprint_randomart = barrier::create_fingerprint_randomart(fingerprint.data); + + m_pLabelLocalFingerprint->setText(QString::fromStdString(fingerprint_str)); + label_sha256_fingerprint_full->setText(QString::fromStdString(fingerprint_str_cols)); + label_sha256_randomart->setText(QString::fromStdString(fingerprint_randomart)); + } + } + + toolbutton_show_fingerprint->setEnabled(true); } void MainWindow::on_m_pGroupClient_toggled(bool on) diff --git a/src/gui/src/MainWindow.h b/src/gui/src/MainWindow.h index 9a6c615d..59a0e0db 100644 --- a/src/gui/src/MainWindow.h +++ b/src/gui/src/MainWindow.h @@ -203,6 +203,8 @@ public slots: QStringList m_PendingClientNames; LogWindow *m_pLogWindow; + bool m_fingerprint_expanded = false; + private slots: void on_m_pCheckBoxAutoConfig_toggled(bool checked); void on_m_pComboServerList_currentIndexChanged(QString ); diff --git a/src/gui/src/MainWindowBase.ui b/src/gui/src/MainWindowBase.ui index 3e31ac9f..9f8d4896 100644 --- a/src/gui/src/MainWindowBase.ui +++ b/src/gui/src/MainWindowBase.ui @@ -75,10 +75,87 @@ + + Qt::PlainText + + + + + + + ... + + + Qt::DownArrow + + + + + QFrame::StyledPanel + + + QFrame::Raised + + + + QLayout::SetMinimumSize + + + + + + Courier + + + + + + + Qt::LinksAccessibleByMouse|Qt::TextSelectableByMouse + + + + + + + SHA1 (deprecated, compare to old clients only): + + + + + + + + + + Qt::LinksAccessibleByMouse|Qt::TextSelectableByMouse + + + + + + + SHA256: + + + + + + + + + + Qt::LinksAccessibleByMouse|Qt::TextSelectableByMouse + + + + + + @@ -242,7 +319,7 @@ - :/res/icons/16x16/padlock.png + :/res/icons/16x16/padlock.png @@ -377,7 +454,7 @@ - Show &Log + Show &Log Show Log @@ -388,7 +465,7 @@ - + diff --git a/src/gui/src/SslCertificate.cpp b/src/gui/src/SslCertificate.cpp index 39c63674..ea770503 100644 --- a/src/gui/src/SslCertificate.cpp +++ b/src/gui/src/SslCertificate.cpp @@ -73,12 +73,12 @@ void SslCertificate::generateCertificate() void SslCertificate::generateFingerprint(const std::string& cert_path) { try { - auto fingerprint = barrier::get_pem_file_cert_fingerprint(cert_path, - barrier::FingerprintType::SHA1); - auto local_path = DataDirectories::local_ssl_fingerprints_path(); barrier::FingerprintDatabase db; - db.add_trusted(fingerprint); + db.add_trusted(barrier::get_pem_file_cert_fingerprint(cert_path, + barrier::FingerprintType::SHA1)); + db.add_trusted(barrier::get_pem_file_cert_fingerprint(cert_path, + barrier::FingerprintType::SHA256)); db.write(local_path); emit info(tr("SSL fingerprint generated.")); diff --git a/src/lib/net/SecureSocket.cpp b/src/lib/net/SecureSocket.cpp index 245f5287..3c65d9ac 100644 --- a/src/lib/net/SecureSocket.cpp +++ b/src/lib/net/SecureSocket.cpp @@ -657,17 +657,22 @@ bool SecureSocket::verifyCertFingerprint() { // calculate received certificate fingerprint - barrier::FingerprintData fingerprint; + barrier::FingerprintData fingerprint_sha1, fingerprint_sha256; try { - fingerprint = barrier::get_ssl_cert_fingerprint(SSL_get_peer_certificate(m_ssl->m_ssl), - barrier::FingerprintType::SHA1); + auto* cert = SSL_get_peer_certificate(m_ssl->m_ssl); + fingerprint_sha1 = barrier::get_ssl_cert_fingerprint(cert, + barrier::FingerprintType::SHA1); + fingerprint_sha256 = barrier::get_ssl_cert_fingerprint(cert, + barrier::FingerprintType::SHA256); } catch (const std::exception& e) { LOG((CLOG_ERR "%s", e.what())); return false; } - LOG((CLOG_NOTE "server fingerprint: %s", - barrier::format_ssl_fingerprint(fingerprint.data).c_str())); + // note: the GUI parses the following two lines of logs, don't change unnecessarily + LOG((CLOG_NOTE "server fingerprint (SHA1): %s (SHA256): %s", + barrier::format_ssl_fingerprint(fingerprint_sha1.data).c_str(), + barrier::format_ssl_fingerprint(fingerprint_sha256.data).c_str())); auto fingerprint_db_path = DataDirectories::trusted_servers_ssl_fingerprints_path(); @@ -685,7 +690,7 @@ SecureSocket::verifyCertFingerprint() fingerprint_db_path.c_str())); } - if (db.is_trusted(fingerprint)) { + if (db.is_trusted(fingerprint_sha256)) { LOG((CLOG_NOTE "Fingerprint matches trusted fingerprint")); return true; } else { diff --git a/src/lib/net/SecureUtils.cpp b/src/lib/net/SecureUtils.cpp index c7e0a82d..b99dd38c 100644 --- a/src/lib/net/SecureUtils.cpp +++ b/src/lib/net/SecureUtils.cpp @@ -89,6 +89,29 @@ std::string format_ssl_fingerprint(const std::vector& fingerprint, bool return result; } +std::string format_ssl_fingerprint_columns(const std::vector& fingerprint) +{ + auto max_columns = 8; + + std::string hex = barrier::string::to_hex(fingerprint, 2); + barrier::string::uppercase(hex); + if (hex.empty() || hex.size() % 2 != 0) { + return hex; + } + + std::string separated; + for (std::size_t i = 0; i < hex.size(); i += max_columns * 2) { + for (std::size_t j = i; j < i + 16 && j < hex.size() - 1; j += 2) { + separated.push_back(hex[j]); + separated.push_back(hex[j + 1]); + separated.push_back(':'); + } + separated.push_back('\n'); + } + separated.pop_back(); // we don't need last newline character + return separated; +} + FingerprintData get_ssl_cert_fingerprint(X509* cert, FingerprintType type) { if (!cert) { diff --git a/src/lib/net/SecureUtils.h b/src/lib/net/SecureUtils.h index 7525381f..c4d51f33 100644 --- a/src/lib/net/SecureUtils.h +++ b/src/lib/net/SecureUtils.h @@ -28,6 +28,7 @@ namespace barrier { std::string format_ssl_fingerprint(const std::vector& fingerprint, bool separator = true); +std::string format_ssl_fingerprint_columns(const std::vector& fingerprint); FingerprintData get_ssl_cert_fingerprint(X509* cert, FingerprintType type);