Merge pull request #803 from p12tic/crypto-cert-fixes

Regenerate server certificate when it is broken or has too small key size

src/gui/CMakeLists.txt

@@ -22,7 +22,7 @@ add_executable (barrier WIN32
 
 include_directories (./src)
 
-target_link_libraries (barrier Qt5::Core Qt5::Widgets Qt5::Network)
+target_link_libraries (barrier Qt5::Core Qt5::Widgets Qt5::Network ${OPENSSL_LIBS})
 target_compile_definitions (barrier PRIVATE -DBARRIER_VERSION_STAGE="${BARRIER_VERSION_STAGE}")
 target_compile_definitions (barrier PRIVATE -DBARRIER_REVISION="${BARRIER_REVISION}")
 

src/gui/src/MainWindow.cpp

@@ -949,6 +949,10 @@ void MainWindow::updateSSLFingerprint()
 {
     if (m_AppConfig->getCryptoEnabled() && m_pSslCertificate == nullptr) {
         m_pSslCertificate = new SslCertificate(this);
+        connect(m_pSslCertificate, &SslCertificate::info, [&](QString info)
+        {
+            appendLogInfo(info);
+        });
         m_pSslCertificate->generateCertificate();
     }
     if (m_AppConfig->getCryptoEnabled() && Fingerprint::local().fileExists()) {

src/gui/src/SslCertificate.cpp

@@ -23,6 +23,12 @@
 #include <QDir>
 #include <QCoreApplication>
 
+#include <openssl/bio.h>
+#include <openssl/err.h>
+#include <openssl/evp.h>
+#include <openssl/pem.h>
+#include <openssl/x509.h>
+
 static const char kCertificateLifetime[] = "365";
 static const char kCertificateSubjectInfo[] = "/CN=Barrier";
 static const char kCertificateFilename[] = "Barrier.pem";
@@ -37,13 +43,13 @@ static const char kConfigFile[] = "barrier.conf";
 SslCertificate::SslCertificate(QObject *parent) :
     QObject(parent)
 {
-    m_ProfileDir = QString::fromStdString(DataDirectories::profile());
+    m_ProfileDir = DataDirectories::profile();
-    if (m_ProfileDir.isEmpty()) {
+    if (m_ProfileDir.empty()) {
         emit error(tr("Failed to get profile directory."));
     }
 }
 
-bool SslCertificate::runTool(const QStringList& args)
+std::pair<bool, std::string> SslCertificate::runTool(const QStringList& args)
 {
     QString program;
 #if defined(Q_OS_WIN)
@@ -66,11 +72,12 @@ bool SslCertificate::runTool(const QStringList& args)
     process.start(program, args);
 
     bool success = process.waitForStarted();
+    std::string output;
 
     QString standardError;
     if (success && process.waitForFinished())
     {
-        m_ToolOutput = process.readAllStandardOutput().trimmed();
+        output = process.readAllStandardOutput().trimmed().toStdString();
         standardError = process.readAllStandardError().trimmed();
     }
 
@@ -82,26 +89,18 @@ bool SslCertificate::runTool(const QStringList& args)
                 .arg(program)
                 .arg(process.exitCode())
                 .arg(standardError.isEmpty() ? "Unknown" : standardError));
-        return false;
+        return {false, output};
     }
 
-    return true;
+    return {true, output};
 }
 
 void SslCertificate::generateCertificate()
 {
-    QString sslDirPath = QString("%1%2%3")
+    auto filename = QString::fromStdString(getCertificatePath());
-        .arg(m_ProfileDir)
-        .arg(QDir::separator())
-        .arg(kSslDir);
-
-    QString filename = QString("%1%2%3")
-        .arg(sslDirPath)
-        .arg(QDir::separator())
-        .arg(kCertificateFilename);
 
     QFile file(filename);
-    if (!file.exists()) {
+    if (!file.exists() || !isCertificateValid(filename)) {
         QStringList arguments;
 
         // self signed certificate
@@ -123,7 +122,7 @@ void SslCertificate::generateCertificate()
         arguments.append("-newkey");
         arguments.append("rsa:2048");
 
-        QDir sslDir(sslDirPath);
+        QDir sslDir(QString::fromStdString(getCertificateDirectory()));
         if (!sslDir.exists()) {
             sslDir.mkpath(".");
         }
@@ -136,7 +135,7 @@ void SslCertificate::generateCertificate()
         arguments.append("-out");
         arguments.append(filename);
 
-        if (!runTool(arguments)) {
+        if (!runTool(arguments).first) {
             return;
         }
 
@@ -158,21 +157,90 @@ void SslCertificate::generateFingerprint(const QString& certificateFilename)
     arguments.append("-in");
     arguments.append(certificateFilename);
 
-    if (!runTool(arguments)) {
+    auto ret = runTool(arguments);
+    bool success = ret.first;
+    std::string output = ret.second;
+
+    if (!success) {
         return;
     }
 
     // find the fingerprint from the tool output
-    int i = m_ToolOutput.indexOf("=");
+    auto i = output.find_first_of('=');
-    if (i != -1) {
+    if (i != std::string::npos) {
         i++;
-        QString fingerprint = m_ToolOutput.mid(
+        auto fingerprint = output.substr(
-            i, m_ToolOutput.size() - i);
+            i, output.size() - i);
 
-        Fingerprint::local().trust(fingerprint, false);
+        Fingerprint::local().trust(QString::fromStdString(fingerprint), false);
         emit info(tr("SSL fingerprint generated."));
     }
     else {
         emit error(tr("Failed to find SSL fingerprint."));
     }
 }
+
+std::string SslCertificate::getCertificatePath()
+{
+    return getCertificateDirectory() + QDir::separator().toLatin1() + kCertificateFilename;
+}
+
+std::string SslCertificate::getCertificateDirectory()
+{
+    return m_ProfileDir + QDir::separator().toLatin1() + kSslDir;
+}
+
+bool SslCertificate::isCertificateValid(const QString& path)
+{
+    OpenSSL_add_all_algorithms();
+    ERR_load_BIO_strings();
+    ERR_load_crypto_strings();
+
+    BIO* bio = BIO_new(BIO_s_file());
+
+    auto ret = BIO_read_filename(bio, path.toStdString().c_str());
+    if (!ret) {
+        emit info(tr("Could not read from default certificate file."));
+        BIO_free_all(bio);
+        return false;
+    }
+
+    X509* cert = PEM_read_bio_X509(bio, NULL, 0, NULL);
+    if (!cert) {
+        emit info(tr("Error loading default certificate file to memory."));
+        BIO_free_all(bio);
+        return false;
+    }
+
+    EVP_PKEY* pubkey = X509_get_pubkey(cert);
+    if (!pubkey) {
+        emit info(tr("Default certificate key file does not contain valid public key"));
+        X509_free(cert);
+        BIO_free_all(bio);
+        return false;
+    }
+
+    auto type = EVP_PKEY_type(EVP_PKEY_id(pubkey));
+    if (type != EVP_PKEY_RSA && type != EVP_PKEY_DSA) {
+        emit info(tr("Public key in default certificate key file is not RSA or DSA"));
+        EVP_PKEY_free(pubkey);
+        X509_free(cert);
+        BIO_free_all(bio);
+        return false;
+    }
+
+    auto bits = EVP_PKEY_bits(pubkey);
+    if (bits < 2048) {
+        // We could have small keys in old barrier installations
+        emit info(tr("Public key in default certificate key file is too small."));
+        EVP_PKEY_free(pubkey);
+        X509_free(cert);
+        BIO_free_all(bio);
+        return false;
+    }
+
+    EVP_PKEY_free(pubkey);
+    X509_free(cert);
+    BIO_free_all(bio);
+    return true;
+}

src/gui/src/SslCertificate.h

@@ -18,6 +18,7 @@
 #pragma once
 
 #include <QObject>
+#include <string>
 
 class SslCertificate : public QObject
 {
@@ -35,10 +36,13 @@ signals:
     void generateFinished();
 
 private:
-    bool runTool(const QStringList& args);
+    std::pair<bool, std::string> runTool(const QStringList& args);
     void generateFingerprint(const QString& certificateFilename);
 
+    std::string getCertificatePath();
+    std::string getCertificateDirectory();
+
+    bool isCertificateValid(const QString& path);
 private:
-    QString m_ProfileDir;
+    std::string m_ProfileDir;
-    QString m_ToolOutput;
 };

src/lib/net/SecureSocket.cpp

@@ -329,7 +329,7 @@ SecureSocket::initSsl(bool server)
     initContext(server);
 }
 
-bool SecureSocket::loadCertificates(std::string& filename)
+bool SecureSocket::loadCertificates(const std::string& filename)
 {
     if (filename.empty()) {
         showError("ssl certificate is not specified");
@@ -341,9 +341,7 @@ bool SecureSocket::loadCertificates(std::string& filename)
         file.close();
 
         if (!exist) {
-            std::string errorMsg("ssl certificate doesn't exist: ");
+            showError("ssl certificate doesn't exist: " + filename);
-            errorMsg.append(filename);
-            showError(errorMsg.c_str());
             return false;
         }
     }
@@ -351,19 +349,19 @@ bool SecureSocket::loadCertificates(std::string& filename)
     int r = 0;
     r = SSL_CTX_use_certificate_file(m_ssl->m_context, filename.c_str(), SSL_FILETYPE_PEM);
     if (r <= 0) {
-        showError("could not use ssl certificate");
+        showError("could not use ssl certificate: " + filename);
         return false;
     }
 
     r = SSL_CTX_use_PrivateKey_file(m_ssl->m_context, filename.c_str(), SSL_FILETYPE_PEM);
     if (r <= 0) {
-        showError("could not use ssl private key");
+        showError("could not use ssl private key: " + filename);
         return false;
     }
 
     r = SSL_CTX_check_private_key(m_ssl->m_context);
     if (!r) {
-        showError("could not verify ssl private key");
+        showError("could not verify ssl private key: " + filename);
         return false;
     }
 
@@ -403,7 +401,7 @@ SecureSocket::initContext(bool server)
     SSL_CTX_set_options(m_ssl->m_context, SSL_OP_NO_SSLv3);
 
     if (m_ssl->m_context == NULL) {
-        showError();
+        showError("");
     }
 }
 
@@ -618,16 +616,15 @@ SecureSocket::checkResult(int status, int& retry)
 
     if (isFatal()) {
         retry = 0;
-        showError();
+        showError("");
         disconnect();
     }
 }
 
-void
+void SecureSocket::showError(const std::string& reason)
-SecureSocket::showError(const char* reason)
 {
-    if (reason != NULL) {
+    if (!reason.empty()) {
-        LOG((CLOG_ERR "%s", reason));
+        LOG((CLOG_ERR "%s", reason.c_str()));
     }
 
     std::string error = getError();

src/lib/net/SecureSocket.h

@@ -55,7 +55,7 @@ public:
     EJobResult            doRead() override;
     EJobResult            doWrite() override;
     void                initSsl(bool server);
-    bool loadCertificates(std::string& CertFile);
+    bool loadCertificates(const std::string& filename);
 
 private:
     // SSL
@@ -65,7 +65,7 @@ private:
     int                    secureConnect(int s);
     bool                showCertificate();
     void                checkResult(int n, int& retry);
-    void                showError(const char* reason = NULL);
+    void                showError(const std::string& reason);
     std::string getError();
     void                disconnect();
     void formatFingerprint(std::string& fingerprint, bool hex = true, bool separator = true);