Skip to content

Commit

Permalink
Add option to verify subsonic server certificate. (#6060)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
ftiede authored and hatstand committed May 23, 2018
1 parent 5bd2c77 commit c01b7bc
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 8 deletions.
8 changes: 6 additions & 2 deletions src/internet/subsonic/subsonicservice.cpp
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 3 additions & 1 deletion src/internet/subsonic/subsonicservice.h
Expand Up @@ -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_; }

Expand Down Expand Up @@ -154,6 +155,7 @@ class SubsonicService : public InternetService {
QString username_;
QString password_;
bool usesslv3_;
bool verifycert_;

LoginState login_state_;
QString working_server_; // The actual server, possibly post-redirect
Expand Down
6 changes: 5 additions & 1 deletion src/internet/subsonic/subsonicsettingspage.cpp
Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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() {
Expand Down
18 changes: 14 additions & 4 deletions src/internet/subsonic/subsonicsettingspage.ui
Expand Up @@ -44,17 +44,17 @@
</property>
</widget>
</item>
<item row="2" column="1" colspan="2">
<item row="2" column="1" colspan="3">
<widget class="QLineEdit" name="password">
<property name="echoMode">
<enum>QLineEdit::Password</enum>
</property>
</widget>
</item>
<item row="1" column="1">
<item row="1" column="1" colspan="2">
<widget class="QLineEdit" name="username"/>
</item>
<item row="0" column="1" colspan="2">
<item row="0" column="1" colspan="3">
<widget class="QLineEdit" name="server"/>
</item>
<item row="3" column="1">
Expand All @@ -64,7 +64,17 @@
</property>
</widget>
</item>
<item row="1" column="2">
<item row="3" column="2">
<widget class="QCheckBox" name="verifycert">
<property name="text">
<string>Verify Server certificate</string>
</property>
<property name="checked">
<bool>true</bool>
</property>
</widget>
</item>
<item row="1" column="3">
<widget class="QPushButton" name="login">
<property name="text">
<string>Login</string>
Expand Down

0 comments on commit c01b7bc

Please sign in to comment.