Merge pull request #1346 from p12tic/client-identity-verification

Implement client identity verification [SECURITY VULNERABILITIES CVE-2021-42072, CVE-2021-42073]
debauchee · Nov 1, 2021 · b5adc93 · b5adc93
2 parents 6d7eca4 + 7cacbd1
commit b5adc93
Show file tree

Hide file tree

Showing 31 changed files with 680 additions and 255 deletions.
diff --git a/doc/newsfragments/client-certificate-checking.bugfix b/doc/newsfragments/client-certificate-checking.bugfix
@@ -0,0 +1,7 @@
+SECURITY ISSUE
+
+Barrier now supports client identity verification (fixes CVE-2021-42072, CVE-2021-42073).
+
+To support seamless upgrades from older versions of Barrier this is currently disabled by default.
+The feature can be enabled in the settings dialog. If enabled, older clients of Barrier will be
+rejected.
diff --git a/doc/newsfragments/client-send-certificate.feature b/doc/newsfragments/client-send-certificate.feature
@@ -0,0 +1 @@
+Barrier client now sends certificate that the server can verify.
diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt
@@ -29,6 +29,7 @@ set(GUI_SOURCE_FILES
     src/CommandProcess.cpp
     src/DataDownloader.cpp
     src/DisplayIsValid.cpp
+    src/FingerprintAcceptDialog.cpp
     src/HotkeyDialog.cpp
     src/IpcClient.cpp
     src/Ipc.cpp
@@ -104,6 +105,7 @@ set(GUI_UI_FILES
     src/AboutDialogBase.ui
     src/ActionDialogBase.ui
     src/AddClientDialogBase.ui
+    src/FingerprintAcceptDialog.ui
     src/HotkeyDialogBase.ui
     src/LogWindowBase.ui
     src/MainWindowBase.ui

diff --git a/src/gui/src/AppConfig.cpp b/src/gui/src/AppConfig.cpp
@@ -158,6 +158,8 @@ void AppConfig::loadSettings()
     m_ElevateMode = static_cast<ElevateMode>(elevateMode.toInt());
     m_AutoConfigPrompted = settings().value("autoConfigPrompted", false).toBool();
     m_CryptoEnabled = settings().value("cryptoEnabled", true).toBool();
+    // TODO: set default value of requireClientCertificate to true on Barrier 2.5.0
+    m_RequireClientCertificate = settings().value("requireClientCertificate", false).toBool();
     m_AutoHide = settings().value("autoHide", false).toBool();
     m_AutoStart = settings().value("autoStart", false).toBool();
     m_MinimizeToTray = settings().value("minimizeToTray", false).toBool();
@@ -181,6 +183,7 @@ void AppConfig::saveSettings()
     settings().setValue("elevateModeEnum", static_cast<int>(m_ElevateMode));
     settings().setValue("autoConfigPrompted", m_AutoConfigPrompted);
     settings().setValue("cryptoEnabled", m_CryptoEnabled);
+    settings().setValue("requireClientCertificate", m_RequireClientCertificate);
     settings().setValue("autoHide", m_AutoHide);
     settings().setValue("autoStart", m_AutoStart);
     settings().setValue("minimizeToTray", m_MinimizeToTray);
@@ -225,6 +228,10 @@ void AppConfig::setCryptoEnabled(bool e) { m_CryptoEnabled = e; }
 
 bool AppConfig::getCryptoEnabled() const { return m_CryptoEnabled; }
 
+void AppConfig::setRequireClientCertificate(bool e) { m_RequireClientCertificate = e; }
+
+bool AppConfig::getRequireClientCertificate() const { return m_RequireClientCertificate; }
+
 void AppConfig::setAutoHide(bool b) { m_AutoHide = b; }
 
 bool AppConfig::getAutoHide() { return m_AutoHide; }

diff --git a/src/gui/src/AppConfig.h b/src/gui/src/AppConfig.h
@@ -91,6 +91,9 @@ class AppConfig: public QObject
         void setCryptoEnabled(bool e);
         bool getCryptoEnabled() const;
 
+        void setRequireClientCertificate(bool e);
+        bool getRequireClientCertificate() const;
+
         void setAutoHide(bool b);
         bool getAutoHide();
 
@@ -132,6 +135,7 @@ class AppConfig: public QObject
         ElevateMode m_ElevateMode;
         bool m_AutoConfigPrompted;
         bool m_CryptoEnabled;
+        bool m_RequireClientCertificate = false;
         bool m_AutoHide;
         bool m_AutoStart;
         bool m_MinimizeToTray;

diff --git a/src/gui/src/FingerprintAcceptDialog.cpp b/src/gui/src/FingerprintAcceptDialog.cpp
@@ -0,0 +1,65 @@
+/*
+    barrier -- mouse and keyboard sharing utility
+    Copyright (C) Barrier contributors
+
+    This package is free software; you can redistribute it and/or
+    modify it under the terms of the GNU General Public License
+    found in the file LICENSE that should have accompanied this file.
+
+    This package is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "FingerprintAcceptDialog.h"
+#include "ui_FingerprintAcceptDialog.h"
+#include "net/SecureUtils.h"
+
+FingerprintAcceptDialog::FingerprintAcceptDialog(QWidget *parent,
+                                                 BarrierType type,
+                                                 const barrier::FingerprintData& fingerprint_sha1,
+                                                 const barrier::FingerprintData& fingerprint_sha256) :
+    QDialog(parent),
+    ui_{std::make_unique<Ui::FingerprintAcceptDialog>()}
+{
+    ui_->setupUi(this);
+
+    if (type == BarrierType::Server) {
+        ui_->label_sha1->hide();
+        ui_->label_sha1_fingerprint_full->hide();
+    } else {
+        ui_->label_sha1_fingerprint_full->setText(
+                QString::fromStdString(barrier::format_ssl_fingerprint(fingerprint_sha1.data)));
+    }
+
+    ui_->label_sha256_fingerprint_full->setText(
+            QString::fromStdString(barrier::format_ssl_fingerprint_columns(fingerprint_sha256.data)));
+    ui_->label_sha256_fingerprint_randomart->setText(
+            QString::fromStdString(barrier::create_fingerprint_randomart(fingerprint_sha256.data)));
+
+    QString explanation;
+    if (type == BarrierType::Server) {
+        explanation = tr("This is a client fingerprint. You should compare this "
+                         "fingerprint to the one on your client's screen. If the "
+                         "two don't match exactly, then it's probably not the client "
+                         "you're expecting (it could be a malicious user).\n\n"
+                         "To automatically trust this fingerprint for future "
+                         "connections, click Yes. To reject this fingerprint and "
+                         "disconnect the client, click No.");
+    } else {
+        explanation = tr("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 "
+                         "you're expecting (it could be a malicious user).\n\n"
+                         "To automatically trust this fingerprint for future "
+                         "connections, click Yes. To reject this fingerprint and "
+                         "disconnect from the server, click No.");
+    }
+    ui_->label_explanation->setText(explanation);
+}
+
+FingerprintAcceptDialog::~FingerprintAcceptDialog() = default;
diff --git a/src/gui/src/FingerprintAcceptDialog.h b/src/gui/src/FingerprintAcceptDialog.h
@@ -0,0 +1,45 @@
+/*
+    barrier -- mouse and keyboard sharing utility
+    Copyright (C) Barrier contributors
+
+    This package is free software; you can redistribute it and/or
+    modify it under the terms of the GNU General Public License
+    found in the file LICENSE that should have accompanied this file.
+
+    This package is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef BARRIER_GUI_FINGERPRINT_ACCEPT_DIALOG_H
+#define BARRIER_GUI_FINGERPRINT_ACCEPT_DIALOG_H
+
+#include "net/FingerprintData.h"
+#include "barrier/BarrierType.h"
+#include <QDialog>
+#include <memory>
+
+namespace Ui {
+class FingerprintAcceptDialog;
+}
+
+class FingerprintAcceptDialog : public QDialog
+{
+    Q_OBJECT
+
+public:
+    explicit FingerprintAcceptDialog(QWidget* parent,
+                                     BarrierType type,
+                                     const barrier::FingerprintData& fingerprint_sha1,
+                                     const barrier::FingerprintData& fingerprint_sha256);
+    ~FingerprintAcceptDialog() override;
+
+private:
+    std::unique_ptr<Ui::FingerprintAcceptDialog> ui_;
+};
+
+#endif // BARRIER_GUI_FINGERPRINT_ACCEPT_DIALOG_H
diff --git a/src/gui/src/FingerprintAcceptDialog.ui b/src/gui/src/FingerprintAcceptDialog.ui
@@ -0,0 +1,174 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ui version="4.0">
+ <class>FingerprintAcceptDialog</class>
+ <widget class="QDialog" name="FingerprintAcceptDialog">
+  <property name="geometry">
+   <rect>
+    <x>0</x>
+    <y>0</y>
+    <width>600</width>
+    <height>400</height>
+   </rect>
+  </property>
+  <property name="sizePolicy">
+   <sizepolicy hsizetype="Ignored" vsizetype="Preferred">
+    <horstretch>0</horstretch>
+    <verstretch>0</verstretch>
+   </sizepolicy>
+  </property>
+  <property name="windowTitle">
+   <string>Security question</string>
+  </property>
+  <layout class="QGridLayout" name="gridLayout">
+   <property name="sizeConstraint">
+    <enum>QLayout::SetFixedSize</enum>
+   </property>
+   <item row="6" column="0" colspan="2">
+    <widget class="QDialogButtonBox" name="buttonBox">
+     <property name="orientation">
+      <enum>Qt::Horizontal</enum>
+     </property>
+     <property name="standardButtons">
+      <set>QDialogButtonBox::No|QDialogButtonBox::Yes</set>
+     </property>
+    </widget>
+   </item>
+   <item row="3" column="0" colspan="2">
+    <widget class="QLabel" name="label_sha1">
+     <property name="sizePolicy">
+      <sizepolicy hsizetype="Preferred" vsizetype="Minimum">
+       <horstretch>0</horstretch>
+       <verstretch>0</verstretch>
+      </sizepolicy>
+     </property>
+     <property name="text">
+      <string>SHA1 (deprecated, compare to old servers only)</string>
+     </property>
+    </widget>
+   </item>
+   <item row="5" column="0" colspan="2">
+    <widget class="QLabel" name="label_explanation">
+     <property name="sizePolicy">
+      <sizepolicy hsizetype="Preferred" vsizetype="Minimum">
+       <horstretch>0</horstretch>
+       <verstretch>0</verstretch>
+      </sizepolicy>
+     </property>
+     <property name="text">
+      <string/>
+     </property>
+     <property name="wordWrap">
+      <bool>true</bool>
+     </property>
+     <property name="margin">
+      <number>10</number>
+     </property>
+    </widget>
+   </item>
+   <item row="4" column="0" colspan="2">
+    <widget class="QLabel" name="label_sha1_fingerprint_full">
+     <property name="sizePolicy">
+      <sizepolicy hsizetype="Preferred" vsizetype="Minimum">
+       <horstretch>0</horstretch>
+       <verstretch>0</verstretch>
+      </sizepolicy>
+     </property>
+     <property name="text">
+      <string/>
+     </property>
+     <property name="textInteractionFlags">
+      <set>Qt::LinksAccessibleByMouse|Qt::TextSelectableByMouse</set>
+     </property>
+    </widget>
+   </item>
+   <item row="0" column="0" colspan="2">
+    <widget class="QLabel" name="label_title">
+     <property name="text">
+      <string>Do you trust this fingerprint?</string>
+     </property>
+    </widget>
+   </item>
+   <item row="2" column="1">
+    <widget class="QLabel" name="label_sha256_fingerprint_randomart">
+     <property name="sizePolicy">
+      <sizepolicy hsizetype="Minimum" vsizetype="Minimum">
+       <horstretch>0</horstretch>
+       <verstretch>0</verstretch>
+      </sizepolicy>
+     </property>
+     <property name="font">
+      <font>
+       <family>Courier</family>
+       <weight>75</weight>
+       <bold>true</bold>
+      </font>
+     </property>
+     <property name="text">
+      <string/>
+     </property>
+     <property name="alignment">
+      <set>Qt::AlignCenter</set>
+     </property>
+     <property name="textInteractionFlags">
+      <set>Qt::LinksAccessibleByMouse|Qt::TextSelectableByMouse</set>
+     </property>
+    </widget>
+   </item>
+   <item row="2" column="0">
+    <widget class="QLabel" name="label_sha256_fingerprint_full">
+     <property name="text">
+      <string/>
+     </property>
+     <property name="alignment">
+      <set>Qt::AlignCenter</set>
+     </property>
+     <property name="textInteractionFlags">
+      <set>Qt::LinksAccessibleByMouse|Qt::TextSelectableByMouse</set>
+     </property>
+    </widget>
+   </item>
+   <item row="1" column="0" colspan="2">
+    <widget class="QLabel" name="label_sha256">
+     <property name="text">
+      <string>SHA256:</string>
+     </property>
+    </widget>
+   </item>
+  </layout>
+ </widget>
+ <resources/>
+ <connections>
+  <connection>
+   <sender>buttonBox</sender>
+   <signal>accepted()</signal>
+   <receiver>FingerprintAcceptDialog</receiver>
+   <slot>accept()</slot>
+   <hints>
+    <hint type="sourcelabel">
+     <x>248</x>
+     <y>254</y>
+    </hint>
+    <hint type="destinationlabel">
+     <x>157</x>
+     <y>274</y>
+    </hint>
+   </hints>
+  </connection>
+  <connection>
+   <sender>buttonBox</sender>
+   <signal>rejected()</signal>
+   <receiver>FingerprintAcceptDialog</receiver>
+   <slot>reject()</slot>
+   <hints>
+    <hint type="sourcelabel">
+     <x>316</x>
+     <y>260</y>
+    </hint>
+    <hint type="destinationlabel">
+     <x>286</x>
+     <y>274</y>
+    </hint>
+   </hints>
+  </connection>
+ </connections>
+</ui>