From 4f1fd49ddee51f829325cda34989f71867ffdafa Mon Sep 17 00:00:00 2001 From: Jamie Newbon Date: Tue, 28 Jul 2020 13:00:20 +0100 Subject: [PATCH] #6669 Completed TLS selection in GUI and generation - Also added key length detection --- src/gui/src/AppConfig.cpp | 2 +- src/gui/src/MainWindow.cpp | 1 + src/gui/src/SettingsDialog.cpp | 57 +++++++++++++++++++++++++++++-- src/gui/src/SettingsDialog.h | 17 +++++++++ src/gui/src/SettingsDialogBase.ui | 3 +- src/gui/src/SslCertificate.cpp | 35 ++++++++++++++++--- src/gui/src/SslCertificate.h | 6 ++++ 7 files changed, 112 insertions(+), 9 deletions(-) diff --git a/src/gui/src/AppConfig.cpp b/src/gui/src/AppConfig.cpp index dab12f894a..029c326fc5 100644 --- a/src/gui/src/AppConfig.cpp +++ b/src/gui/src/AppConfig.cpp @@ -257,7 +257,7 @@ void AppConfig::loadSettings() "Synergy.pem"); m_TLSCertificatePath = loadSetting(kTLSCertPath, certificateFilename).toString(); - m_TLSKeyLength = loadSetting(kTLSKeyLength, 2048).toInt(); + m_TLSKeyLength = loadSetting(kTLSKeyLength, "2048").toString(); } diff --git a/src/gui/src/MainWindow.cpp b/src/gui/src/MainWindow.cpp index 72b715f14d..4cc156abba 100644 --- a/src/gui/src/MainWindow.cpp +++ b/src/gui/src/MainWindow.cpp @@ -670,6 +670,7 @@ void MainWindow::startSynergy() if (m_AppConfig->getCryptoEnabled()) { args << "--enable-crypto"; + args << "--tls-cert" << m_AppConfig->getTLSCertPath(); } #if defined(Q_OS_WIN) diff --git a/src/gui/src/SettingsDialog.cpp b/src/gui/src/SettingsDialog.cpp index 3965b56a4d..9f44a9a758 100644 --- a/src/gui/src/SettingsDialog.cpp +++ b/src/gui/src/SettingsDialog.cpp @@ -128,9 +128,16 @@ void SettingsDialog::loadFromConfig() { setIndexFromItemData(m_pComboLanguage, appConfig().language()); m_pCheckBoxAutoHide->setChecked(appConfig().getAutoHide()); m_pCheckBoxMinimizeToTray->setChecked(appConfig().getMinimizeToTray()); - m_pCheckBoxEnableCrypto->setChecked(m_appConfig.getCryptoEnabled()); m_pLineEditCertificatePath->setText(appConfig().getTLSCertPath()); - m_pComboBoxKeyLength->setCurrentIndex(m_pComboBoxKeyLength->findText(appConfig().getTLSKeyLength())); + m_pCheckBoxEnableCrypto->setChecked(m_appConfig.getCryptoEnabled()); + + //If the tls file exists test its key length + if (QFile(appConfig().getTLSCertPath()).exists()) { + updateKeyLengthOnFile(appConfig().getTLSCertPath()); + } else { + m_pComboBoxKeyLength->setCurrentIndex(m_pComboBoxKeyLength->findText(appConfig().getTLSKeyLength())); + } + if (m_appConfig.isSystemScoped()) { m_pRadioSystemScope->setChecked(true); @@ -157,6 +164,8 @@ void SettingsDialog::loadFromConfig() { #endif m_pCheckBoxEnableCrypto->setChecked(m_appConfig.getCryptoEnabled()); + m_pGroupBoxTLS->setVisible(m_appConfig.getCryptoEnabled()); + #ifdef SYNERGY_ENTERPRISE @@ -175,6 +184,7 @@ void SettingsDialog::loadFromConfig() { m_pCheckBoxAutoConfig->setChecked(appConfig().autoConfig()); #endif + adjustSize(); } @@ -217,7 +227,7 @@ void SettingsDialog::on_m_pCheckBoxEnableCrypto_toggled(bool checked) m_appConfig.setCryptoEnabled(checked); if (checked) { SslCertificate sslCertificate; - sslCertificate.generateCertificate(); + sslCertificate.generateCertificate(m_pLineEditCertificatePath->text(), m_pComboBoxKeyLength->currentText()); m_pMainWindow->updateLocalFingerprint(); verticalSpacer_4->changeSize(10, 10, QSizePolicy::Minimum); } else { @@ -249,5 +259,46 @@ void SettingsDialog::on_m_pPushButtonBrowseCert_clicked() { if (!fileName.isEmpty()) { m_pLineEditCertificatePath->setText(fileName); + //If the tls file exists test its key length and update + if (QFile(appConfig().getTLSCertPath()).exists()) { + updateKeyLengthOnFile(fileName); + } } + updateRegenButton(); +} + +void SettingsDialog::regenerateSSLCert() { + SslCertificate sslCertificate; + sslCertificate.generateCertificate(appConfig().getTLSCertPath(), + appConfig().getTLSKeyLength(), + true); + + m_pMainWindow->updateLocalFingerprint(); +} + +void SettingsDialog::on_m_pComboBoxKeyLength_currentIndexChanged(int index) { + updateRegenButton(); +} + +void SettingsDialog::updateRegenButton() { + // Disable the Regenerate cert button if the key length is different to saved + auto keyChanged = appConfig().getTLSKeyLength() != m_pComboBoxKeyLength->currentText(); + auto pathChanged = appConfig().getTLSCertPath() != m_pLineEditCertificatePath->text(); + auto cryptoChanged = appConfig().getCryptoEnabled() != m_pCheckBoxEnableCrypto->isChecked(); + //NOR the above bools, if any have changed regen should be disabled as it will be done on save + auto nor = !(keyChanged || pathChanged || cryptoChanged); + m_pPushButtonRegenCert->setEnabled(nor); +} + +void SettingsDialog::on_m_pPushButtonRegenCert_clicked() { + regenerateSSLCert(); +} + +void SettingsDialog::updateKeyLengthOnFile(const QString &path) { + SslCertificate ssl; + auto length = ssl.getCertKeyLength(path); + auto index = m_pComboBoxKeyLength->findText(length); + m_pComboBoxKeyLength->setCurrentIndex(index); + //Also update what is in the appconfig to match the file itself + appConfig().setTLSKeyLength(length); } diff --git a/src/gui/src/SettingsDialog.h b/src/gui/src/SettingsDialog.h index d0b36df171..1b03dcd9bf 100644 --- a/src/gui/src/SettingsDialog.h +++ b/src/gui/src/SettingsDialog.h @@ -48,6 +48,16 @@ class SettingsDialog : public QDialog, public Ui::SettingsDialogBase /// @brief Causes the dialog to load all the settings from m_appConfig void loadFromConfig(); + /// @brief Forces the regeneration of the TLS cert from the saved settings + void regenerateSSLCert(); + + /// @brief Check if the regenerate button should be enabled or disabled and sets it + void updateRegenButton(); + + /// @brief Updates the key length value based on the loaded file + /// @param [in] QString path The path to the file to test + void updateKeyLengthOnFile(const QString& path); + private: MainWindow* m_pMainWindow; AppConfig& m_appConfig; @@ -69,6 +79,13 @@ class SettingsDialog : public QDialog, public Ui::SettingsDialogBase /// @brief Handles the click event of the Cert Path browse button /// displaying a file browser void on_m_pPushButtonBrowseCert_clicked(); + + /// @brief Handles the TLS cert key length changed event + void on_m_pComboBoxKeyLength_currentIndexChanged(int index); + + /// @brief handels the regenerate cert button event + /// This will regenerate the TLS certificate as long as the settings haven't changed + void on_m_pPushButtonRegenCert_clicked(); }; #endif diff --git a/src/gui/src/SettingsDialogBase.ui b/src/gui/src/SettingsDialogBase.ui index 7b240536cf..a79c59226d 100644 --- a/src/gui/src/SettingsDialogBase.ui +++ b/src/gui/src/SettingsDialogBase.ui @@ -357,7 +357,7 @@ - + Regenerate Cert @@ -510,6 +510,7 @@ m_pComboBoxKeyLength m_pLineEditCertificatePath m_pPushButtonBrowseCert + m_pPushButtonRegenCert m_pComboLogLevel m_pCheckBoxLogToFile m_pLineEditLogFilename diff --git a/src/gui/src/SslCertificate.cpp b/src/gui/src/SslCertificate.cpp index 8a569567ef..3e48cd732c 100644 --- a/src/gui/src/SslCertificate.cpp +++ b/src/gui/src/SslCertificate.cpp @@ -107,8 +107,10 @@ void SslCertificate::generateCertificate(const QString& path, const QString& key QString keySize = kCertificateKeyLength + keyLength; + const QString pathToUse = path.isEmpty() ? filename : path; + //If path is empty use filename - QFile file(path.isEmpty() ? filename : path); + QFile file(pathToUse); if (!file.exists() || forceGen) { QStringList arguments; @@ -138,11 +140,11 @@ void SslCertificate::generateCertificate(const QString& path, const QString& key // key output filename arguments.append("-keyout"); - arguments.append(filename); + arguments.append(pathToUse); // certificate output filename arguments.append("-out"); - arguments.append(filename); + arguments.append(pathToUse); if (!runTool(arguments)) { return; @@ -151,7 +153,7 @@ void SslCertificate::generateCertificate(const QString& path, const QString& key emit info(tr("SSL certificate generated.")); } - generateFingerprint(filename); + generateFingerprint(pathToUse); emit generateFinished(); } @@ -184,3 +186,28 @@ void SslCertificate::generateFingerprint(const QString& certificateFilename) emit error(tr("Failed to find SSL fingerprint.")); } } + +QString SslCertificate::getCertKeyLength(const QString &path) { + + QStringList arguments; + arguments.append("rsa"); + arguments.append("-in"); + arguments.append(path); + arguments.append("-text"); + arguments.append("-noout"); + + if (!runTool(arguments)) { + return QString(); + } + const QString searchStart("Private-Key: ("); + const QString searchEnd(" bit"); + + //Get the line that contains the key length from the output + const auto indexStart = m_ToolOutput.indexOf(searchStart); + const auto indexEnd = m_ToolOutput.indexOf(searchEnd, indexStart); + const auto start = indexStart + searchStart.length(); + const auto end = indexEnd - (indexStart + searchStart.length()); + auto keyLength = m_ToolOutput.mid(start, end); + + return keyLength; +} diff --git a/src/gui/src/SslCertificate.h b/src/gui/src/SslCertificate.h index 968f65b693..a88a41af1c 100644 --- a/src/gui/src/SslCertificate.h +++ b/src/gui/src/SslCertificate.h @@ -20,6 +20,7 @@ #include "CoreInterface.h" #include +#include class SslCertificate : public QObject { @@ -35,6 +36,11 @@ public slots: /// @param [in] bool Should the file be created regardless of if the file already exists void generateCertificate(const QString& path = QString(), const QString& keyLength = "2048", bool forceGen = false); + /// @brief Get the key length of a TLS private key + /// @param [in] QString path The path of the file to checked + /// @return QString The key legnth as a string + QString getCertKeyLength(const QString& path); + signals: void error(QString e); void info(QString i);