From 907ebc19593b7cdcf0558500a4c2a98842a34c51 Mon Sep 17 00:00:00 2001 From: alex-z Date: Mon, 25 Oct 2021 18:04:29 +0300 Subject: [PATCH 1/2] Request OCSP validation data from the server during the SSL handshake. Signed-off-by: alex-z --- NEXTCLOUD.cmake | 1 + config.h.in | 1 + src/gui/sslerrordialog.cpp | 16 ++++++++++++++++ src/libsync/account.cpp | 2 ++ src/libsync/theme.cpp | 9 +++++++++ src/libsync/theme.h | 7 +++++++ 6 files changed, 36 insertions(+) diff --git a/NEXTCLOUD.cmake b/NEXTCLOUD.cmake index 82f1bd4b7..11f43aa1d 100644 --- a/NEXTCLOUD.cmake +++ b/NEXTCLOUD.cmake @@ -11,6 +11,7 @@ set( APPLICATION_SERVER_URL "" CACHE STRING "URL for the server to use. If enter set( APPLICATION_SERVER_URL_ENFORCE ON ) # If set and APPLICATION_SERVER_URL is defined, the server can only connect to the pre-defined URL set( APPLICATION_REV_DOMAIN "com.nextcloud.desktopclient" ) set( APPLICATION_VIRTUALFILE_SUFFIX "nextcloud" CACHE STRING "Virtual file suffix (not including the .)") +set( APPLICATION_OCSP_STAPLING_ENABLED OFF ) set( LINUX_PACKAGE_SHORTNAME "nextcloud" ) set( LINUX_APPLICATION_ID "${APPLICATION_REV_DOMAIN}.${LINUX_PACKAGE_SHORTNAME}") diff --git a/config.h.in b/config.h.in index 13d4cdf08..48236399c 100644 --- a/config.h.in +++ b/config.h.in @@ -29,6 +29,7 @@ #cmakedefine APPLICATION_WIZARD_HEADER_TITLE_COLOR "@APPLICATION_WIZARD_HEADER_TITLE_COLOR@" #cmakedefine APPLICATION_WIZARD_USE_CUSTOM_LOGO "@APPLICATION_WIZARD_USE_CUSTOM_LOGO@" #cmakedefine APPLICATION_VIRTUALFILE_SUFFIX "@APPLICATION_VIRTUALFILE_SUFFIX@" +#cmakedefine APPLICATION_OCSP_STAPLING_ENABLED "@APPLICATION_OCSP_STAPLING_ENABLED@" #define APPLICATION_DOTVIRTUALFILE_SUFFIX "." APPLICATION_VIRTUALFILE_SUFFIX #cmakedefine ZLIB_FOUND @ZLIB_FOUND@ diff --git a/src/gui/sslerrordialog.cpp b/src/gui/sslerrordialog.cpp index 5c148dc85..429f29b32 100644 --- a/src/gui/sslerrordialog.cpp +++ b/src/gui/sslerrordialog.cpp @@ -105,6 +105,8 @@ bool SslErrorDialog::checkFailingCertsKnown(const QList &errors) QStringList errorStrings; + QStringList additionalErrorStrings; + QList trustedCerts = _account->approvedCerts(); for (int i = 0; i < errors.count(); ++i) { @@ -115,6 +117,8 @@ bool SslErrorDialog::checkFailingCertsKnown(const QList &errors) errorStrings += error.errorString(); if (!error.certificate().isNull()) { _unknownCerts.append(error.certificate()); + } else { + additionalErrorStrings.append(error.errorString()); } } @@ -132,6 +136,7 @@ bool SslErrorDialog::checkFailingCertsKnown(const QList &errors) msg += QL("

") + tr("Cannot connect securely to %1:").arg(host) + QL("

"); // loop over the unknown certs and line up their errors. msg += QL("
"); + foreach (const QSslCertificate &cert, _unknownCerts) { msg += QL("
"); // add the errors for this cert @@ -146,6 +151,17 @@ bool SslErrorDialog::checkFailingCertsKnown(const QList &errors) msg += QL("
"); } } + + if (!additionalErrorStrings.isEmpty()) { + msg += QL("

") + tr("Additional errors:") + QL("

"); + + for (const auto &errorString : additionalErrorStrings) { + msg += QL("
"); + msg += QL("

") + errorString + QL("

"); + msg += QL("
"); + } + } + msg += QL("
"); auto *doc = new QTextDocument(nullptr); diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp index a58c61a40..b6be1b111 100644 --- a/src/libsync/account.cpp +++ b/src/libsync/account.cpp @@ -389,6 +389,8 @@ QSslConfiguration Account::getOrCreateSslConfig() sslConfig.setSslOption(QSsl::SslOptionDisableSessionSharing, false); sslConfig.setSslOption(QSsl::SslOptionDisableSessionPersistence, false); + sslConfig.setOcspStaplingEnabled(Theme::instance()->enableStaplingOCSP()); + return sslConfig; } diff --git a/src/libsync/theme.cpp b/src/libsync/theme.cpp index d8fdd609e..37eb4568a 100644 --- a/src/libsync/theme.cpp +++ b/src/libsync/theme.cpp @@ -399,6 +399,15 @@ bool Theme::forceOverrideServerUrl() const #endif } +bool Theme::enableStaplingOCSP() const +{ +#ifdef APPLICATION_OCSP_STAPLING_ENABLED + return true; +#else + return false; +#endif +} + QString Theme::forceConfigAuthType() const { return QString(); diff --git a/src/libsync/theme.h b/src/libsync/theme.h index ec9a5dd42..29db46414 100644 --- a/src/libsync/theme.h +++ b/src/libsync/theme.h @@ -239,6 +239,13 @@ public: * When true, the respective UI controls will be disabled */ virtual bool forceOverrideServerUrl() const; + + /** + * Enable OCSP stapling for SSL handshakes + * + * When true, peer will be requested for Online Certificate Status Protocol response + */ + virtual bool enableStaplingOCSP() const; /** * This is only usefull when previous version had a different overrideServerUrl From db4e54025a53e24eabd6dc4ad0750fd70e9ec056 Mon Sep 17 00:00:00 2001 From: alex-z Date: Tue, 26 Oct 2021 14:27:22 +0300 Subject: [PATCH 2/2] Forbid trusting the untrusted certificate. Signed-off-by: alex-z --- NEXTCLOUD.cmake | 1 + config.h.in | 1 + src/gui/sslerrordialog.cpp | 6 ++++-- src/libsync/theme.cpp | 9 +++++++++ src/libsync/theme.h | 7 +++++++ 5 files changed, 22 insertions(+), 2 deletions(-) diff --git a/NEXTCLOUD.cmake b/NEXTCLOUD.cmake index 11f43aa1d..ce246e8e8 100644 --- a/NEXTCLOUD.cmake +++ b/NEXTCLOUD.cmake @@ -12,6 +12,7 @@ set( APPLICATION_SERVER_URL_ENFORCE ON ) # If set and APPLICATION_SERVER_URL is set( APPLICATION_REV_DOMAIN "com.nextcloud.desktopclient" ) set( APPLICATION_VIRTUALFILE_SUFFIX "nextcloud" CACHE STRING "Virtual file suffix (not including the .)") set( APPLICATION_OCSP_STAPLING_ENABLED OFF ) +set( APPLICATION_FORBID_BAD_SSL OFF ) set( LINUX_PACKAGE_SHORTNAME "nextcloud" ) set( LINUX_APPLICATION_ID "${APPLICATION_REV_DOMAIN}.${LINUX_PACKAGE_SHORTNAME}") diff --git a/config.h.in b/config.h.in index 48236399c..1c7d921cd 100644 --- a/config.h.in +++ b/config.h.in @@ -30,6 +30,7 @@ #cmakedefine APPLICATION_WIZARD_USE_CUSTOM_LOGO "@APPLICATION_WIZARD_USE_CUSTOM_LOGO@" #cmakedefine APPLICATION_VIRTUALFILE_SUFFIX "@APPLICATION_VIRTUALFILE_SUFFIX@" #cmakedefine APPLICATION_OCSP_STAPLING_ENABLED "@APPLICATION_OCSP_STAPLING_ENABLED@" +#cmakedefine APPLICATION_FORBID_BAD_SSL "@APPLICATION_FORBID_BAD_SSL@" #define APPLICATION_DOTVIRTUALFILE_SUFFIX "." APPLICATION_VIRTUALFILE_SUFFIX #cmakedefine ZLIB_FOUND @ZLIB_FOUND@ diff --git a/src/gui/sslerrordialog.cpp b/src/gui/sslerrordialog.cpp index 429f29b32..7b97a657a 100644 --- a/src/gui/sslerrordialog.cpp +++ b/src/gui/sslerrordialog.cpp @@ -13,6 +13,7 @@ */ #include "configfile.h" #include "sslerrordialog.h" +#include "theme.h" #include #include @@ -68,6 +69,8 @@ SslErrorDialog::SslErrorDialog(AccountPtr account, QWidget *parent) QPushButton *cancelButton = _ui->_dialogButtonBox->button(QDialogButtonBox::Cancel); okButton->setEnabled(false); + + _ui->_cbTrustConnect->setEnabled(!Theme::instance()->forbidBadSSL()); connect(_ui->_cbTrustConnect, &QAbstractButton::clicked, okButton, &QWidget::setEnabled); @@ -136,7 +139,6 @@ bool SslErrorDialog::checkFailingCertsKnown(const QList &errors) msg += QL("

") + tr("Cannot connect securely to %1:").arg(host) + QL("

"); // loop over the unknown certs and line up their errors. msg += QL("
"); - foreach (const QSslCertificate &cert, _unknownCerts) { msg += QL("
"); // add the errors for this cert @@ -153,7 +155,7 @@ bool SslErrorDialog::checkFailingCertsKnown(const QList &errors) } if (!additionalErrorStrings.isEmpty()) { - msg += QL("

") + tr("Additional errors:") + QL("

"); + msg += QL("

") + tr("Additional errors:") + QL("

"); for (const auto &errorString : additionalErrorStrings) { msg += QL("
"); diff --git a/src/libsync/theme.cpp b/src/libsync/theme.cpp index 37eb4568a..8fd5b1a3f 100644 --- a/src/libsync/theme.cpp +++ b/src/libsync/theme.cpp @@ -408,6 +408,15 @@ bool Theme::enableStaplingOCSP() const #endif } +bool Theme::forbidBadSSL() const +{ +#ifdef APPLICATION_FORBID_BAD_SSL + return true; +#else + return false; +#endif +} + QString Theme::forceConfigAuthType() const { return QString(); diff --git a/src/libsync/theme.h b/src/libsync/theme.h index 29db46414..f07c0de4a 100644 --- a/src/libsync/theme.h +++ b/src/libsync/theme.h @@ -247,6 +247,13 @@ public: */ virtual bool enableStaplingOCSP() const; + /** + * Enforce SSL validity + * + * When true, trusting the untrusted certificate is not allowed + */ + virtual bool forbidBadSSL() const; + /** * This is only usefull when previous version had a different overrideServerUrl * with a different auth type in that case You should then specify "http" or "shibboleth".