From 0a03a29e303d386ce5ff563971c8c96563150f34 Mon Sep 17 00:00:00 2001 From: LordGrey <48840279+Lord-Grey@users.noreply.github.com> Date: Wed, 18 Oct 2023 21:30:33 +0200 Subject: [PATCH] Fix self-signed certificate handling --- CHANGELOG.md | 1 + .../leddevice/dev_net/LedDevicePhilipsHue.cpp | 6 +- libsrc/leddevice/dev_net/ProviderRestApi.cpp | 73 ++++++++++++++++++- libsrc/leddevice/dev_net/ProviderRestApi.h | 2 + 4 files changed, 75 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 061f7b09..e4024253 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,7 @@ Note: The wizard will configure an APIv2 capable bridge always with Entertainmen - Changed default build from Stretch to Buster - Support Qt 6.7, Update to Protobuf 23.4.0, Update mbedTLS to v3.4.0, Update flatbuffers to v23.5.26 - Use C++17 standard as default +- Added Pull Request (PR) installation script, allowing users to test development builds savely on Linux - Fixed missing include limits in QJsonSchemaChecker - Thanks @Portisch - Fixed dependencies for deb packages in Debian Bookworm (#1579) - Thanks @hg42, @Psirus - Fixed git version identification when run in docker and local code diff --git a/libsrc/leddevice/dev_net/LedDevicePhilipsHue.cpp b/libsrc/leddevice/dev_net/LedDevicePhilipsHue.cpp index 14f9c39a..77712594 100644 --- a/libsrc/leddevice/dev_net/LedDevicePhilipsHue.cpp +++ b/libsrc/leddevice/dev_net/LedDevicePhilipsHue.cpp @@ -584,11 +584,7 @@ int LedDevicePhilipsHueBridge::close() bool LedDevicePhilipsHueBridge::configureSsl() { _restApi->setAlternateServerIdentity(_deviceBridgeId); - - if (_isDiyHue) - { - _restApi->acceptSelfSignedCertificates(true); - } + _restApi->acceptSelfSignedCertificates(true); bool success = _restApi->setCaCertificate(API_SSL_CA_CERTIFICATE_RESSOURCE); if (!success) diff --git a/libsrc/leddevice/dev_net/ProviderRestApi.cpp b/libsrc/leddevice/dev_net/ProviderRestApi.cpp index e2d07475..7321810f 100644 --- a/libsrc/leddevice/dev_net/ProviderRestApi.cpp +++ b/libsrc/leddevice/dev_net/ProviderRestApi.cpp @@ -11,6 +11,8 @@ #include #include #include +#include +#include #include @@ -451,6 +453,63 @@ bool ProviderRestApi::checkServerIdentity(const QSslConfiguration& sslConfig) co return isServerIdentified; } +bool ProviderRestApi::matchesPinnedCertificate(const QSslCertificate& certificate) +{ + bool isMatching {false}; + + QList certificateInfos = certificate.subjectInfo(QSslCertificate::CommonName); + + if (certificateInfos.isEmpty()) + { + return false; + } + QString identifier = certificateInfos.constFirst(); + + QString appDataDir = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation); + QString certDir = appDataDir + "/certificates"; + QDir().mkpath(certDir); + + QString filePath(certDir + "/" + identifier + ".pem"); + QFile file(filePath); + if (file.open(QIODevice::ReadOnly)) + { + QList certificates = QSslCertificate::fromDevice(&file, QSsl::Pem); + if (!certificates.isEmpty()) + { + Debug (_log,"First used certificate loaded successfully"); + QSslCertificate pinnedeCertificate = certificates.constFirst(); + if (pinnedeCertificate == certificate) + { + isMatching = true; + } + } + else + { + Debug (_log,"Error reading first used certificate file: %s", QSTRING_CSTR(filePath)); + } + file.close(); + } + else + { + if (file.open(QIODevice::WriteOnly)) + { + QByteArray pemData = certificate.toPem(); + qint64 bytesWritten = file.write(pemData); + if (bytesWritten == pemData.size()) + { + Debug (_log,"First used certificate saved to file: %s", QSTRING_CSTR(filePath)); + isMatching = true; + } + else + { + Debug (_log,"Error writing first used certificate file: %s", QSTRING_CSTR(filePath)); + } + file.close(); + } + } + return isMatching; +} + void ProviderRestApi::onSslErrors(QNetworkReply* reply, const QList& errors) { int ignoredErrorCount {0}; @@ -466,11 +525,21 @@ void ProviderRestApi::onSslErrors(QNetworkReply* reply, const QList& } break; case QSslError::SelfSignedCertificate : - if (_isSeflSignedCertificateAccpeted) + if (_isSeflSignedCertificateAccpeted) + { + // Get the peer certificate associated with the error + QSslCertificate certificate = error.certificate(); + if (matchesPinnedCertificate(certificate)) { + Debug (_log,"'Trust on first use' - Certificate received matches pinned certificate"); ignoreSslError = true; } - break; + else + { + Error (_log,"'Trust on first use' - Certificate received does not match pinned certificate"); + } + } + break; default: break; } diff --git a/libsrc/leddevice/dev_net/ProviderRestApi.h b/libsrc/leddevice/dev_net/ProviderRestApi.h index b93d13ea..db4f9bd7 100644 --- a/libsrc/leddevice/dev_net/ProviderRestApi.h +++ b/libsrc/leddevice/dev_net/ProviderRestApi.h @@ -444,6 +444,8 @@ private: bool checkServerIdentity(const QSslConfiguration& sslConfig) const; + bool matchesPinnedCertificate(const QSslCertificate& certificate); + Logger* _log; /// QNetworkAccessManager object for sending REST-requests.