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);