From c01b7bc4307131f3aa7889e94b24b2c965a7e8fc Mon Sep 17 00:00:00 2001 From: ftiede Date: Wed, 23 May 2018 15:23:53 +0200 Subject: [PATCH] Add option to verify subsonic server certificate. (#6060) * Add option to verify subsonic server certificate. Defaults to true, as it is safer to have a server certificate verified, even more so, if the server is used over an insecure WAN link. During subsonic configuration the checkbox can be deactivated, so that no certificate verification will occur when talking to a subsonic server, allowing for self-signed certificates. With the proliferation of let's encrypt certificates there's probably less need for this option but it has been requested and hard-coding verify-off is IMHO bad security practice. If a valid certificate has been installed, the configuration file can be modified manually and after a restart Clementine will perform a proper server certificate verification. The patch might need some UI polishing and asks for string translations but is operational so far. * Satisfy CLang format checker. * Use QSettings' default value support. * Consistently use QSettings' default value method. --- src/internet/subsonic/subsonicservice.cpp | 8 ++++++-- src/internet/subsonic/subsonicservice.h | 4 +++- src/internet/subsonic/subsonicsettingspage.cpp | 6 +++++- src/internet/subsonic/subsonicsettingspage.ui | 18 ++++++++++++++---- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/internet/subsonic/subsonicservice.cpp b/src/internet/subsonic/subsonicservice.cpp index edb7f74d6..1970d5931 100644 --- a/src/internet/subsonic/subsonicservice.cpp +++ b/src/internet/subsonic/subsonicservice.cpp @@ -195,6 +195,7 @@ void SubsonicService::ReloadSettings() { username_ = s.value("username").toString(); password_ = s.value("password").toString(); usesslv3_ = s.value("usesslv3").toBool(); + verifycert_ = s.value("verifycert", true).toBool(); Login(); } @@ -225,11 +226,13 @@ void SubsonicService::Login() { } void SubsonicService::Login(const QString& server, const QString& username, - const QString& password, const bool& usesslv3) { + const QString& password, const bool& usesslv3, + const bool& verifycert) { UpdateServer(server); username_ = username; password_ = password; usesslv3_ = usesslv3; + verifycert_ = verifycert; Login(); } @@ -263,7 +266,8 @@ QNetworkReply* SubsonicService::Send(const QUrl& url) { // Don't try and check the authenticity of the SSL certificate - it'll almost // certainly be self-signed. QSslConfiguration sslconfig = QSslConfiguration::defaultConfiguration(); - sslconfig.setPeerVerifyMode(QSslSocket::VerifyNone); + sslconfig.setPeerVerifyMode(verifycert_ ? QSslSocket::VerifyPeer + : QSslSocket::VerifyNone); if (usesslv3_) { sslconfig.setProtocol(QSsl::SslV3); } diff --git a/src/internet/subsonic/subsonicservice.h b/src/internet/subsonic/subsonicservice.h index 3a358741f..22ff1bb14 100644 --- a/src/internet/subsonic/subsonicservice.h +++ b/src/internet/subsonic/subsonicservice.h @@ -97,7 +97,8 @@ class SubsonicService : public InternetService { void Login(); void Login(const QString& server, const QString& username, - const QString& password, const bool& usesslv3); + const QString& password, const bool& usesslv3, + const bool& verifycert); LoginState login_state() const { return login_state_; } @@ -154,6 +155,7 @@ signals: QString username_; QString password_; bool usesslv3_; + bool verifycert_; LoginState login_state_; QString working_server_; // The actual server, possibly post-redirect diff --git a/src/internet/subsonic/subsonicsettingspage.cpp b/src/internet/subsonic/subsonicsettingspage.cpp index a48509f3b..9c5fb41d5 100644 --- a/src/internet/subsonic/subsonicsettingspage.cpp +++ b/src/internet/subsonic/subsonicsettingspage.cpp @@ -45,6 +45,7 @@ SubsonicSettingsPage::SubsonicSettingsPage(SettingsDialog* dialog) ui_->login_state->AddCredentialField(ui_->username); ui_->login_state->AddCredentialField(ui_->password); ui_->login_state->AddCredentialField(ui_->usesslv3); + ui_->login_state->AddCredentialField(ui_->verifycert); ui_->login_state->AddCredentialGroup(ui_->server_group); ui_->login_state->SetAccountTypeText( @@ -63,6 +64,7 @@ void SubsonicSettingsPage::Load() { ui_->username->setText(s.value("username").toString()); ui_->password->setText(s.value("password").toString()); ui_->usesslv3->setChecked(s.value("usesslv3").toBool()); + ui_->verifycert->setChecked(s.value("verifycert", true).toBool()); // If the settings are complete, SubsonicService will have used them already // and @@ -80,6 +82,7 @@ void SubsonicSettingsPage::Save() { s.setValue("username", ui_->username->text()); s.setValue("password", ui_->password->text()); s.setValue("usesslv3", ui_->usesslv3->isChecked()); + s.setValue("verifycert", ui_->verifycert->isChecked()); } void SubsonicSettingsPage::LoginStateChanged( @@ -190,7 +193,8 @@ void SubsonicSettingsPage::ServerEditingFinished() { void SubsonicSettingsPage::Login() { ui_->login_state->SetLoggedIn(LoginStateWidget::LoginInProgress); service_->Login(ui_->server->text(), ui_->username->text(), - ui_->password->text(), ui_->usesslv3->isChecked()); + ui_->password->text(), ui_->usesslv3->isChecked(), + ui_->verifycert->isChecked()); } void SubsonicSettingsPage::Logout() { diff --git a/src/internet/subsonic/subsonicsettingspage.ui b/src/internet/subsonic/subsonicsettingspage.ui index 4b7fd08a6..e2aa61a77 100644 --- a/src/internet/subsonic/subsonicsettingspage.ui +++ b/src/internet/subsonic/subsonicsettingspage.ui @@ -44,17 +44,17 @@ - + QLineEdit::Password - + - + @@ -64,7 +64,17 @@ - + + + + Verify Server certificate + + + true + + + + Login