Improve messages for TLS errors

Before one only gets the generic error "TLS handshake failed". Now one gets
more details error messages plus the problematic certificate. This should
be helpful for debugging.
This commit is contained in:
Martchus 2021-10-07 18:21:48 +02:00
parent af24ead784
commit 8b273b6945
4 changed files with 50 additions and 3 deletions

View File

@ -326,6 +326,7 @@ private Q_SLOTS:
void emitDirStatisticsChanged();
void handleFatalConnectionError();
void handleAdditionalRequestCanceled();
void handleSslErrors(const QList<QSslError> &errors);
void recalculateStatus();
private:
@ -400,6 +401,7 @@ private:
QString m_syncthingVersion;
bool m_lastFileDeleted;
QList<QSslError> m_expectedSslErrors;
QSslCertificate m_certFromLastSslError;
QJsonObject m_rawConfig;
bool m_dirStatsAltered;
bool m_recordFileChanges;

View File

@ -52,10 +52,10 @@ QNetworkReply *SyncthingConnection::requestData(const QString &path, const QUrlQ
{
#ifndef LIB_SYNCTHING_CONNECTOR_CONNECTION_MOCKED
auto *const reply = networkAccessManager().get(prepareRequest(path, query, rest));
QObject::connect(reply, &QNetworkReply::sslErrors, this, &SyncthingConnection::handleSslErrors);
if (loggingFlags() & SyncthingConnectionLoggingFlags::ApiCalls) {
cerr << Phrases::Info << "Querying API: GET " << reply->url().toString().toStdString() << Phrases::EndFlush;
}
reply->ignoreSslErrors(m_expectedSslErrors);
return reply;
#else
return MockedReply::forRequest(QStringLiteral("GET"), path, query, rest);
@ -68,10 +68,10 @@ QNetworkReply *SyncthingConnection::requestData(const QString &path, const QUrlQ
QNetworkReply *SyncthingConnection::postData(const QString &path, const QUrlQuery &query, const QByteArray &data)
{
auto *const reply = networkAccessManager().post(prepareRequest(path, query), data);
QObject::connect(reply, &QNetworkReply::sslErrors, this, &SyncthingConnection::handleSslErrors);
if (loggingFlags() & SyncthingConnectionLoggingFlags::ApiCalls) {
cerr << Phrases::Info << "Querying API: POST " << reply->url().toString().toStdString() << Phrases::EndFlush;
}
reply->ignoreSslErrors(m_expectedSslErrors);
return reply;
}
@ -105,6 +105,50 @@ SyncthingConnection::Reply SyncthingConnection::prepareReply(QList<QNetworkReply
return handleReply(reply, readData, handleAborting);
}
/*!
* \brief Handles SSL errors of replies; just for logging purposes at this point.
*/
void SyncthingConnection::handleSslErrors(const QList<QSslError> &errors)
{
// check SSL errors for replies
auto *const reply = static_cast<QNetworkReply *>(sender());
auto hasUnexpectedErrors = false;
for (const auto &error : errors) {
// skip expected errors
// note: This would be required even when calling reply->ignoreSslErrors(m_expectedSslErrors) before so we
// are omitting that call and just check it here.
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.
auto errorMessage = QStringLiteral("TLS error: ") + error.errorString();
if (const auto cert = error.certificate(); !cert.isNull()) {
errorMessage += QChar('\n');
if (cert == m_certFromLastSslError) {
errorMessage += QStringLiteral("Certificate: same as last");
} else {
errorMessage += cert.toText();
if (!m_expectedSslErrors.isEmpty()) {
errorMessage += QStringLiteral("\nExpected ") + m_expectedSslErrors.front().certificate().toText();
}
m_certFromLastSslError = cert;
}
}
cerr << "TLS ERROR" << endl;
emit this->error(errorMessage, SyncthingErrorCategory::TLS, QNetworkReply::NoError, reply->request());
hasUnexpectedErrors = true;
}
// proceed if all errors are expected
if (!hasUnexpectedErrors) {
reply->ignoreSslErrors();
}
}
/*!
* \brief Handles the specified \a reply; invoked by the prepareReply() functions.
*/

View File

@ -42,6 +42,7 @@ enum class SyncthingErrorCategory {
OverallConnection, /**< an error affecting the overall connection */
SpecificRequest, /**< an error only affecting a specific request */
Parsing, /**< an error when parsing Syncthing's response as a JSON document */
TLS, /**< a TLS error occurred */
};
#if (QT_VERSION >= QT_VERSION_CHECK(5, 8, 0))
Q_ENUM_NS(SyncthingErrorCategory)

View File

@ -19,7 +19,7 @@ namespace QtGui {
bool InternalError::isRelevant(const SyncthingConnection &connection, SyncthingErrorCategory category, int networkError)
{
// ignore overall connection errors when auto reconnect tries >= 1
if (category != SyncthingErrorCategory::OverallConnection && connection.autoReconnectTries() >= 1) {
if (category != SyncthingErrorCategory::OverallConnection && category != SyncthingErrorCategory::TLS && connection.autoReconnectTries() >= 1) {
return false;
}