From 07ff8a5c1b47e90472fd518791324c5a04ad42e4 Mon Sep 17 00:00:00 2001 From: Martchus Date: Mon, 15 Jan 2024 21:28:43 +0100 Subject: [PATCH] Avoid TLS errors on Syncthing's automatic certificate renewal * Reload the certificate when running into TLS errors an it looks like the certificate was renewed * See https://github.com/Martchus/syncthingtray/issues/226 --- syncthingconnector/syncthingconnection.cpp | 21 +++++++++++++++-- syncthingconnector/syncthingconnection.h | 4 ++++ .../syncthingconnection_requests.cpp | 23 ++++++++++++++++++- .../syncthingconnectionsettings.cpp | 3 +++ .../syncthingconnectionsettings.h | 2 ++ 5 files changed, 50 insertions(+), 3 deletions(-) diff --git a/syncthingconnector/syncthingconnection.cpp b/syncthingconnector/syncthingconnection.cpp index 59e3638..2a0385c 100644 --- a/syncthingconnector/syncthingconnection.cpp +++ b/syncthingconnector/syncthingconnection.cpp @@ -14,6 +14,7 @@ #include #include +#include #include #include #include @@ -776,6 +777,10 @@ void SyncthingConnection::continueConnecting() * - Loading the certificate is only possible if the connection object is configured * to connect to the locally running Syncthing instance. Otherwise this method will * only do the cleanup of previous certificates but not emit any errors. + * - This function uses m_certificatePath which is set by applySettings() if the user + * specified a certificate path manually. Otherwise the path is detected automatically + * and stored in m_dynamicallyDeterminedCertificatePath so the certificate path is + * known in handleSslErrors(). * \returns Returns whether a certificate could be loaded. */ bool SyncthingConnection::loadSelfSignedCertificate(const QUrl &url) @@ -795,7 +800,9 @@ bool SyncthingConnection::loadSelfSignedCertificate(const QUrl &url) } // find cert - const auto certPath = !m_configDir.isEmpty() ? (m_configDir + QStringLiteral("/https-cert.pem")) : SyncthingConfig::locateHttpsCertificate(); + const auto certPath = !m_certificatePath.isEmpty() + ? m_certificatePath + : (!m_configDir.isEmpty() ? (m_configDir + QStringLiteral("/https-cert.pem")) : SyncthingConfig::locateHttpsCertificate()); if (certPath.isEmpty()) { emit error(tr("Unable to locate certificate used by Syncthing."), SyncthingErrorCategory::OverallConnection, QNetworkReply::NoError); return false; @@ -807,13 +814,21 @@ bool SyncthingConnection::loadSelfSignedCertificate(const QUrl &url) return false; } m_expectedSslErrors = SyncthingConnectionSettings::compileSslErrors(certs.at(0)); + // keep track of the dynamically determined certificate path for handleSslErrors() + if (m_certificatePath.isEmpty()) { + m_dynamicallyDeterminedCertificatePath = certPath; + m_certificateLastModified = QFileInfo(certPath).lastModified(); + } return true; } /*! * \brief Applies the specified configuration. * \remarks - * - The expected SSL errors of the specified configuration are updated accordingly. + * - The expected SSL errors are taken from the specified \a connectionSettings. If empty, this + * function attempts to load expected SSL errors automatically as needed/possible via + * loadSelfSignedCertificate(). It then writes back those SSL errors to \a connectionSettings. + * This way \a connectionSettings can act as a cache for SSL exceptions. * - The configuration is not used instantly. It will be used on the next reconnect. * \returns Returns whether at least one property requiring a reconnect to take effect has changed. * \sa reconnect() @@ -838,6 +853,8 @@ bool SyncthingConnection::applySettings(SyncthingConnectionSettings &connectionS } reconnectRequired = true; } + m_certificatePath = connectionSettings.httpsCertPath; + m_certificateLastModified = connectionSettings.httpCertLastModified; if (connectionSettings.expectedSslErrors.isEmpty()) { const bool previouslyHadExpectedSslErrors = !expectedSslErrors().isEmpty(); const bool ok = loadSelfSignedCertificate(); diff --git a/syncthingconnector/syncthingconnection.h b/syncthingconnector/syncthingconnection.h index 73ac8c3..ff8f5a9 100644 --- a/syncthingconnector/syncthingconnection.h +++ b/syncthingconnector/syncthingconnection.h @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -422,6 +423,9 @@ private: QString m_lastFileName; QString m_syncthingVersion; bool m_lastFileDeleted; + QString m_certificatePath; + QString m_dynamicallyDeterminedCertificatePath; + QDateTime m_certificateLastModified; QList m_expectedSslErrors; QSslCertificate m_certFromLastSslError; QJsonObject m_rawConfig; diff --git a/syncthingconnector/syncthingconnection_requests.cpp b/syncthingconnector/syncthingconnection_requests.cpp index 677a889..f3154a3 100644 --- a/syncthingconnector/syncthingconnection_requests.cpp +++ b/syncthingconnector/syncthingconnection_requests.cpp @@ -146,7 +146,14 @@ static QString certText(const QSslCertificate &cert) /// \endcond /*! - * \brief Handles SSL errors of replies; just for logging purposes at this point. + * \brief Handles SSL errors of replies. + * \remarks + * - Ignores expected errors which are usually assigned via applySettings() or loadSelfSignedCertificate() to handle a self-signed + * certificate. + * - If expected errors have previously been assigned to handle a self-signed certificate this function attempts to reload the + * certificate via loadSelfSignedCertificate() if it appears to be re-generated. This is done because Syncthing might re-generate + * the certificate if it will expire soon (indicated by the log message "Loading HTTPS certificate: certificate will soon expire" + * followed by "Creating new HTTPS certificate"). */ void SyncthingConnection::handleSslErrors(const QList &errors) { @@ -162,6 +169,20 @@ void SyncthingConnection::handleSslErrors(const QList &errors) continue; } + // check whether the certificate has changed and reload it before emitting error + if (const auto &certPath = m_certificatePath.isEmpty() ? m_dynamicallyDeterminedCertificatePath : m_certificatePath; + !certPath.isEmpty() && m_certificateLastModified.isValid()) { + if (const auto lastModified = QFileInfo(certPath).lastModified(); lastModified > m_certificateLastModified) { + if (const auto ok = loadSelfSignedCertificate(); ok && !m_certificatePath.isEmpty()) { + m_certificateLastModified = lastModified; + } + // re-check whether error is expected after reloading and skip it accordingly + if (m_expectedSslErrors.contains(error)) { + continue; + } + } + } + // handle the error by emitting the error signal with all the details including the certificate // note: Of course the failing request would cause a QNetworkReply::SslHandshakeFailedError anyways. However, // at this point the concrete SSL error with the certificate is not accessible anymore. diff --git a/syncthingconnector/syncthingconnectionsettings.cpp b/syncthingconnector/syncthingconnectionsettings.cpp index 12cea23..61adf21 100644 --- a/syncthingconnector/syncthingconnectionsettings.cpp +++ b/syncthingconnector/syncthingconnectionsettings.cpp @@ -1,5 +1,7 @@ #include "./syncthingconnectionsettings.h" +#include + namespace Data { QList SyncthingConnectionSettings::compileSslErrors(const QSslCertificate &trustedCert) @@ -27,6 +29,7 @@ bool SyncthingConnectionSettings::loadHttpsCert() return false; } + httpCertLastModified = QFileInfo(httpsCertPath).lastModified(); expectedSslErrors = compileSslErrors(certs.at(0)); return true; } diff --git a/syncthingconnector/syncthingconnectionsettings.h b/syncthingconnector/syncthingconnectionsettings.h index 80e1d28..a94c67b 100644 --- a/syncthingconnector/syncthingconnectionsettings.h +++ b/syncthingconnector/syncthingconnectionsettings.h @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -48,6 +49,7 @@ struct LIB_SYNCTHING_CONNECTOR_EXPORT SyncthingConnectionSettings { int requestTimeout = defaultRequestTimeout; int longPollingTimeout = defaultLongPollingTimeout; QString httpsCertPath; + QDateTime httpCertLastModified; QList expectedSslErrors; SyncthingStatusComputionFlags statusComputionFlags = SyncthingStatusComputionFlags::Default; bool autoConnect = false;