Martchus
/
syncthingtray
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Check whether self-signed certificate actually matches the expected one 

This concerned only the built-in web view using Qt WebEngine. This change
has only effect when using Qt >= 5.14 because the API did not expose the
certificate chain before.
This commit is contained in:
Martchus 2020-03-29 20:38:05 +02:00
parent 3d4fcaea0f
commit 7dea132a87
2 changed files with 49 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
README.md
View File

@ -196,7 +196,9 @@ can be passed to CMake to influence the build.
### Further dependencies
The following Qt 5 modules are requried (version 5.6 or newer): core network dbus gui widgets svg webenginewidgets/webkitwidgets
The built-in web view is optional (see section "Select Qt module for WebView").
It is recommended to use at least Qt 5.14 to avoid limitations in previous versions (see *Known bugs* section).
The built-in web view and therefore the modules webenginewidgets/webkitwidgets are optional (see section *Select Qt module for WebView*).
To build the plugin for Dolphin integration KIO is also requried. Additionally, the Dolphin plugin requires Qt 5.8 or newer. To skip
building the plugin, add `-DNO_FILE_ITEM_ACTION_PLUGIN:BOOL=ON` to the CMake arguments.
@ -351,13 +353,16 @@ on GitHub.
Note that the Plasmoid is not affected by this limitation.
* While the tray menu is shown its entry is shown in the taskbar. Not sure whether there is a way to avoid this.
* Qt bugs
* Any self-signed certificate is accepted when using Qt WebEngine due to Qt bug https://bugreports.qt.io/browse/QTBUG-51176.
* Pausing/resuming folders and devices doesn't work when using scan-intervalls with a lot of zeros because of Syncthing bug
https://github.com/syncthing/syncthing/issues/4001. This has already been fixed on the Qt-side with
https://codereview.qt-project.org/#/c/187069/. However, the fix is only available in Qt 5.9 and above.
* The tray disconnects from the local instance when the network connection goes down. The network connection must be restored or
the tray restarted to be able to connect to local Syncthing again. This is caused by Qt bug
https://bugreports.qt.io/browse/QTBUG-60949.
* Qt < 5.14
* Any self-signed certificate is accepted when using Qt WebEngine due to https://bugreports.qt.io/browse/QTBUG-51176.
* Qt < 5.9:
* Pausing/resuming folders and devices doesn't work when using scan-intervalls with a lot of zeros because of
Syncthing bug https://github.com/syncthing/syncthing/issues/4001. This has already been fixed on the Qt-side with
https://codereview.qt-project.org/#/c/187069/. However, the fix is only available in Qt 5.9 and above.
* any Qt version:
* The tray disconnects from the local instance when the network connection goes down. The network connection must be restored
or the tray restarted to be able to connect to local Syncthing again. This is caused by Qt bug
https://bugreports.qt.io/browse/QTBUG-60949.
## Attribution for 3rd party content
* Some icons are taken from the Syncthing project.

39
widgets/webview/webpage.cpp
View File

@ -18,6 +18,7 @@
#include <QWebEngineCertificateError>
#include <QWebEngineSettings>
#include <QWebEngineView>
#include <QtWebEngineWidgetsVersion>
#elif defined(SYNCTHINGWIDGETS_USE_WEBKIT)
#include <QNetworkRequest>
#include <QSslError>
@ -101,18 +102,50 @@ SYNCTHINGWIDGETS_WEB_PAGE *WebPage::createWindow(SYNCTHINGWIDGETS_WEB_PAGE::WebW
#ifdef SYNCTHINGWIDGETS_USE_WEBENGINE
/*!
* \brief Accepts self-signed certificate.
* \todo Only ignore the error if the used certificate matches the certificate known to be used by the Syncthing GUI.
* \brief Accepts self-signed certificates used by the Syncthing GUI as configured.
* \remarks Before Qt 5.14 any self-signed certificates are accepted.
*/
bool WebPage::certificateError(const QWebEngineCertificateError &certificateError)
{
// never ignore errors other than CertificateCommonNameInvalid and CertificateAuthorityInvalid
switch (certificateError.error()) {
case QWebEngineCertificateError::CertificateCommonNameInvalid:
case QWebEngineCertificateError::CertificateAuthorityInvalid:
return true;
break;
default:
return false;
}
// don't ignore the error if there are no expected self-signed SSL certificates configured
if (!m_dlg || m_dlg->connectionSettings().expectedSslErrors.isEmpty()) {
return false;
}
// ignore only certificate errors matching the expected URL of the Syncthing instance
const auto urlWithError = certificateError.url();
const auto expectedUrl = m_view->url();
if (urlWithError.scheme() != expectedUrl.scheme() || urlWithError.host() != expectedUrl.host()) {
return false;
}
#if (QTWEBENGINEWIDGETS_VERSION >= QT_VERSION_CHECK(5, 14, 0))
// don't ignore the error if no certificate is provided at all (possible?)
const auto certificateChain = certificateError.certificateChain();
if (certificateChain.isEmpty()) {
return false;
}
// don't ignore the error if the first certificate in the chain (the peer's immediate certificate) does
// not match the expected SSL certificate
// note: All the SSL errors in the settings refer to the same certificate so it is sufficient to just pick
// the first one.
if (certificateChain.first() != m_dlg->connectionSettings().expectedSslErrors.first().certificate()) {
return false;
}
#endif
// accept the self-signed certificate
return true;
}
/*!