diff --git a/changelog/unreleased/8864 b/changelog/unreleased/8864 new file mode 100644 index 00000000000..eb1dc10dd2a --- /dev/null +++ b/changelog/unreleased/8864 @@ -0,0 +1,5 @@ +Bugfix: Removed support for client side certificates + +Client side certificates where never officially supported and where untested in many scenarios. + +https://github.com/owncloud/client/pull/8864 diff --git a/src/cmd/cmd.cpp b/src/cmd/cmd.cpp index e0a425efb02..3270f49e291 100644 --- a/src/cmd/cmd.cpp +++ b/src/cmd/cmd.cpp @@ -274,9 +274,8 @@ class HttpCredentialsText : public HttpCredentials { public: HttpCredentialsText(const QString &user, const QString &password) - : HttpCredentials(DetermineAuthTypeJob::AuthType::Basic ,user, password) - , // FIXME: not working with client certs yet (qknight) - _sslTrusted(false) + : HttpCredentials(DetermineAuthTypeJob::AuthType::Basic, user, password) + , _sslTrusted(false) { } diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt index f12e2ef1a03..c3968e21800 100644 --- a/src/gui/CMakeLists.txt +++ b/src/gui/CMakeLists.txt @@ -21,7 +21,6 @@ set(client_UI_SRCS shareusergroupwidget.ui shareuserline.ui sslerrordialog.ui - addcertificatedialog.ui proxyauthdialog.ui notificationwidget.ui logbrowser.ui @@ -75,7 +74,6 @@ set(client_SRCS thumbnailjob.cpp quotainfo.cpp accountstate.cpp - addcertificatedialog.cpp authenticationdialog.cpp proxyauthhandler.cpp proxyauthdialog.cpp diff --git a/src/gui/accountstate.cpp b/src/gui/accountstate.cpp index 44a3db37b22..766729e86de 100644 --- a/src/gui/accountstate.cpp +++ b/src/gui/accountstate.cpp @@ -257,7 +257,7 @@ void AccountState::checkConnectivity(bool blockJobs) } // If we never fetched credentials, do that now - otherwise connection attempts - // make little sense, we might be missing client certs. + // make little sense. if (!account()->credentials()->wasFetched()) { _waitingForNewCredentials = true; account()->credentials()->fetchFromKeychain(); diff --git a/src/gui/addcertificatedialog.cpp b/src/gui/addcertificatedialog.cpp deleted file mode 100644 index fabaeec5b6d..00000000000 --- a/src/gui/addcertificatedialog.cpp +++ /dev/null @@ -1,63 +0,0 @@ -/* - * Copyright (C) 2015 by nocteau - * Copyright (C) 2015 by Daniel Molkentin - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY - * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * for more details. - */ - -#include "ui_addcertificatedialog.h" -#include "addcertificatedialog.h" -#include -#include - - -namespace OCC { -AddCertificateDialog::AddCertificateDialog(QWidget *parent) - : QDialog(parent) - , ui(new Ui::AddCertificateDialog) -{ - ui->setupUi(this); - ui->labelErrorCertif->setText(""); -} - -AddCertificateDialog::~AddCertificateDialog() -{ - delete ui; -} - -void AddCertificateDialog::on_pushButtonBrowseCertificate_clicked() -{ - QString fileName = QFileDialog::getOpenFileName(this, tr("Select a certificate"), "", tr("Certificate files (*.p12 *.pfx)")); - ui->lineEditCertificatePath->setText(fileName); -} - -QString AddCertificateDialog::getCertificatePath() -{ - return ui->lineEditCertificatePath->text(); -} - -QString AddCertificateDialog::getCertificatePasswd() -{ - return ui->lineEditPWDCertificate->text(); -} - -void AddCertificateDialog::showErrorMessage(const QString message) -{ - ui->labelErrorCertif->setText(message); -} - -void AddCertificateDialog::reinit() -{ - ui->labelErrorCertif->clear(); - ui->lineEditCertificatePath->clear(); - ui->lineEditPWDCertificate->clear(); -} -} diff --git a/src/gui/addcertificatedialog.h b/src/gui/addcertificatedialog.h deleted file mode 100644 index bf7322ec89d..00000000000 --- a/src/gui/addcertificatedialog.h +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright (C) 2015 by nocteau - * Copyright (C) 2015 by Daniel Molkentin - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY - * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * for more details. - */ - -#ifndef ADDCERTIFICATEDIALOG_H -#define ADDCERTIFICATEDIALOG_H - -#include -#include - -namespace OCC { - -namespace Ui { - class AddCertificateDialog; -} - -/** - * @brief The AddCertificateDialog class - * @ingroup gui - */ -class AddCertificateDialog : public QDialog -{ - Q_OBJECT - -public: - explicit AddCertificateDialog(QWidget *parent = nullptr); - ~AddCertificateDialog() override; - QString getCertificatePath(); - QString getCertificatePasswd(); - void showErrorMessage(const QString message); - void reinit(); - -private slots: - void on_pushButtonBrowseCertificate_clicked(); - -private: - Ui::AddCertificateDialog *ui; -}; - -} //End namespace OCC - -#endif // ADDCERTIFICATEDIALOG_H diff --git a/src/gui/addcertificatedialog.ui b/src/gui/addcertificatedialog.ui deleted file mode 100644 index 8f815ba04f0..00000000000 --- a/src/gui/addcertificatedialog.ui +++ /dev/null @@ -1,244 +0,0 @@ - - - OCC::AddCertificateDialog - - - Qt::ApplicationModal - - - - 0 - 0 - 586 - 341 - - - - SSL client certificate authentication - - - - - - This server probably requires a SSL client certificate. - - - true - - - - - - - - - - - - - - - - - - Browse... - - - - - - - - - Certificate & Key (pkcs12) : - - - - - - - Certificate password : - - - - - - - - - - QLineEdit::Password - - - - - - - - - An encrypted pkcs12 bundle is strongly recommended as a copy will be stored in the configuration file. - - - true - - - - - - - - - - - - 255 - 0 - 0 - - - - - - - 231 - 23 - 12 - - - - - - - 231 - 23 - 12 - - - - - - - - - 255 - 0 - 0 - - - - - - - 231 - 23 - 12 - - - - - - - 231 - 23 - 12 - - - - - - - - - 119 - 120 - 120 - - - - - - - 127 - 127 - 127 - - - - - - - 231 - 23 - 12 - - - - - - - - true - - - - - - - Qt::Vertical - - - - 20 - 40 - - - - - - - - Qt::Horizontal - - - QDialogButtonBox::Cancel|QDialogButtonBox::Ok - - - - - - - - - buttonBox - accepted() - OCC::AddCertificateDialog - accept() - - - 248 - 254 - - - 157 - 274 - - - - - buttonBox - rejected() - OCC::AddCertificateDialog - reject() - - - 316 - 260 - - - 286 - 274 - - - - - diff --git a/src/gui/creds/httpcredentialsgui.h b/src/gui/creds/httpcredentialsgui.h index e34dd0c23a3..18231f16509 100644 --- a/src/gui/creds/httpcredentialsgui.h +++ b/src/gui/creds/httpcredentialsgui.h @@ -33,15 +33,13 @@ class HttpCredentialsGui : public HttpCredentials : HttpCredentials() { } - HttpCredentialsGui(const QString &user, const QString &password, - const QByteArray &clientCertBundle, const QByteArray &clientCertPassword) - : HttpCredentials(DetermineAuthTypeJob::AuthType::Basic, user, password, clientCertBundle, clientCertPassword) + HttpCredentialsGui(const QString &user, const QString &password) + : HttpCredentials(DetermineAuthTypeJob::AuthType::Basic, user, password) { } - HttpCredentialsGui(const QString &user, const QString &password, const QString &refreshToken, - const QByteArray &clientCertBundle, const QByteArray &clientCertPassword) - : HttpCredentials(DetermineAuthTypeJob::AuthType::OAuth, user, password, clientCertBundle, clientCertPassword) + HttpCredentialsGui(const QString &user, const QString &password, const QString &refreshToken) + : HttpCredentials(DetermineAuthTypeJob::AuthType::OAuth, user, password) { _refreshToken = refreshToken; } diff --git a/src/gui/owncloudsetupwizard.cpp b/src/gui/owncloudsetupwizard.cpp index 93e9721a0db..44da1545dfb 100644 --- a/src/gui/owncloudsetupwizard.cpp +++ b/src/gui/owncloudsetupwizard.cpp @@ -121,10 +121,6 @@ void OwncloudSetupWizard::slotCheckServer(const QString &urlString) // Here the client certificate is added, if any. Later it'll be in HttpCredentials account->setSslConfiguration(QSslConfiguration()); auto sslConfiguration = account->getOrCreateSslConfig(); // let Account set defaults - if (!_ocWizard->_clientSslCertificate.isNull()) { - sslConfiguration.setLocalCertificate(_ocWizard->_clientSslCertificate); - sslConfiguration.setPrivateKey(_ocWizard->_clientSslKey); - } account->setSslConfiguration(sslConfiguration); // Make sure TCP connections get re-established diff --git a/src/gui/wizard/owncloudhttpcredspage.cpp b/src/gui/wizard/owncloudhttpcredspage.cpp index 281ac0f9163..a2332ad28c1 100644 --- a/src/gui/wizard/owncloudhttpcredspage.cpp +++ b/src/gui/wizard/owncloudhttpcredspage.cpp @@ -182,7 +182,7 @@ void OwncloudHttpCredsPage::setErrorString(const QString &err) AbstractCredentials *OwncloudHttpCredsPage::getCredentials() const { - return new HttpCredentialsGui(_ui.leUsername->text(), _ui.lePassword->text(), _ocWizard->_clientCertBundle, _ocWizard->_clientCertPassword); + return new HttpCredentialsGui(_ui.leUsername->text(), _ui.lePassword->text()); } diff --git a/src/gui/wizard/owncloudoauthcredspage.cpp b/src/gui/wizard/owncloudoauthcredspage.cpp index 46c1f1b4591..89bfbf68989 100644 --- a/src/gui/wizard/owncloudoauthcredspage.cpp +++ b/src/gui/wizard/owncloudoauthcredspage.cpp @@ -121,8 +121,7 @@ AbstractCredentials *OwncloudOAuthCredsPage::getCredentials() const { OwncloudWizard *ocWizard = qobject_cast(wizard()); Q_ASSERT(ocWizard); - return new HttpCredentialsGui(_user, _token, _refreshToken, - ocWizard->_clientCertBundle, ocWizard->_clientCertPassword); + return new HttpCredentialsGui(_user, _token, _refreshToken); } bool OwncloudOAuthCredsPage::isComplete() const diff --git a/src/gui/wizard/owncloudsetuppage.cpp b/src/gui/wizard/owncloudsetuppage.cpp index de0c2a65ec1..dfb7f33c47e 100644 --- a/src/gui/wizard/owncloudsetuppage.cpp +++ b/src/gui/wizard/owncloudsetuppage.cpp @@ -66,24 +66,9 @@ OwncloudSetupPage::OwncloudSetupPage(QWidget *parent) _ui->resultLayout->addWidget(_progressIndi); stopSpinner(); - setupCustomization(); - - slotUrlChanged(QLatin1String("")); // don't jitter UI + slotUrlChanged(QString()); // don't jitter UI connect(_ui->leUrl, &QLineEdit::textChanged, this, &OwncloudSetupPage::slotUrlChanged); connect(_ui->leUrl, &QLineEdit::editingFinished, this, &OwncloudSetupPage::slotUrlEditFinished); - - addCertDial = new AddCertificateDialog(this); - connect(addCertDial, &QDialog::accepted, this, &OwncloudSetupPage::slotCertificateAccepted); - - _ui->more_toolButton->setIcon(Utility::getCoreIcon(QStringLiteral("more"))); - connect(_ui->more_toolButton, &QToolButton::clicked, this, [this] { - auto menu = new QMenu(this); - menu->setAttribute(Qt::WA_DeleteOnClose); - menu->addAction(tr("Configure client-side cerificate"), [this] { - addCertDial->open(); - }); - menu->popup(QCursor::pos()); - }); } void OwncloudSetupPage::setServerUrl(const QString &newUrl) @@ -97,13 +82,6 @@ void OwncloudSetupPage::setServerUrl(const QString &newUrl) _ui->leUrl->setText(_oCUrl); } -void OwncloudSetupPage::setupCustomization() -{ - // set defaults for the customize labels. - _ui->topLabel->hide(); - _ui->bottomLabel->hide(); -} - // slot hit from textChanged of the url entry field. void OwncloudSetupPage::slotUrlChanged(const QString &url) { @@ -258,38 +236,6 @@ void OwncloudSetupPage::stopSpinner() _progressIndi->stopAnimation(); } -QString subjectInfoHelper(const QSslCertificate &cert, const QByteArray &qa) -{ - return cert.subjectInfo(qa).join(QLatin1Char('/')); -} - -//called during the validation of the client certificate. -void OwncloudSetupPage::slotCertificateAccepted() -{ - QList clientCaCertificates; - QFile certFile(addCertDial->getCertificatePath()); - certFile.open(QFile::ReadOnly); - QByteArray certData = certFile.readAll(); - QByteArray certPassword = addCertDial->getCertificatePasswd().toLocal8Bit(); - - QBuffer certDataBuffer(&certData); - certDataBuffer.open(QIODevice::ReadOnly); - if (QSslCertificate::importPkcs12(&certDataBuffer, - &_ocWizard->_clientSslKey, &_ocWizard->_clientSslCertificate, - &clientCaCertificates, certPassword)) { - _ocWizard->_clientCertBundle = certData; - _ocWizard->_clientCertPassword = certPassword; - - addCertDial->reinit(); // FIXME: Why not just have this only created on use? - - // The extracted SSL key and cert gets added to the QSslConfiguration in checkServer() - validatePage(); - } else { - addCertDial->showErrorMessage(tr("Could not load certificate. Maybe wrong password?")); - addCertDial->show(); - } -} - OwncloudSetupPage::~OwncloudSetupPage() { } diff --git a/src/gui/wizard/owncloudsetuppage.h b/src/gui/wizard/owncloudsetuppage.h index 2ad04325d17..8caa9d7ec6a 100644 --- a/src/gui/wizard/owncloudsetuppage.h +++ b/src/gui/wizard/owncloudsetuppage.h @@ -22,8 +22,6 @@ #include "wizard/owncloudwizardcommon.h" #include "wizard/owncloudwizard.h" -#include "../addcertificatedialog.h" - class QLabel; class QVariant; class QProgressIndicator; @@ -59,14 +57,11 @@ public slots: void setErrorString(const QString &); void startSpinner(); void stopSpinner(); - void slotCertificateAccepted(); protected slots: void slotUrlChanged(const QString &); void slotUrlEditFinished(); - void setupCustomization(); - signals: void determineAuthType(const QString &); @@ -82,7 +77,6 @@ protected slots: QProgressIndicator *_progressIndi; QButtonGroup *_selectiveSyncButtons; QString _remoteFolder; - AddCertificateDialog *addCertDial; OwncloudWizard *_ocWizard; }; diff --git a/src/gui/wizard/owncloudsetuppage.ui b/src/gui/wizard/owncloudsetuppage.ui index fb286cd3136..959eb0c1c26 100644 --- a/src/gui/wizard/owncloudsetuppage.ui +++ b/src/gui/wizard/owncloudsetuppage.ui @@ -20,28 +20,6 @@ Form - - - - - 0 - 0 - - - - TextLabel - - - Qt::RichText - - - Qt::AlignCenter - - - true - - - @@ -103,17 +81,6 @@ - - - - ... - - - - :/client/resources/light/more.svg:/client/resources/light/more.svg - - - @@ -225,16 +192,6 @@ - - - - TextLabel - - - Qt::RichText - - - diff --git a/src/gui/wizard/owncloudwizard.h b/src/gui/wizard/owncloudwizard.h index 5370a2300c2..2fd6a446712 100644 --- a/src/gui/wizard/owncloudwizard.h +++ b/src/gui/wizard/owncloudwizard.h @@ -70,13 +70,6 @@ class OwncloudWizard : public QWizard */ static void askExperimentalVirtualFilesFeature(QWidget *receiver, const std::function &callback); - // FIXME: Can those be local variables? - // Set from the OwncloudSetupPage, later used from OwncloudHttpCredsPage - QByteArray _clientCertBundle; // raw, potentially encrypted pkcs12 bundle provided by the user - QByteArray _clientCertPassword; // password for the pkcs12 - QSslKey _clientSslKey; // key extracted from pkcs12 - QSslCertificate _clientSslCertificate; // cert extracted from pkcs12 - DetermineAuthTypeJob::AuthType authType() const; public slots: diff --git a/src/libsync/account.h b/src/libsync/account.h index 033402093a6..49e846017b4 100644 --- a/src/libsync/account.h +++ b/src/libsync/account.h @@ -165,9 +165,6 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject QVariant credentialSetting(const QString &key) const; void setCredentialSetting(const QString &key, const QVariant &value); - /** Assign a client certificate */ - void setCertificate(const QByteArray certficate = QByteArray(), const QString privateKey = QString()); - /** Access the server capabilities */ const Capabilities &capabilities() const; void setCapabilities(const QVariantMap &caps); diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index 9d6dd06d843..60ba3a208df 100644 --- a/src/libsync/configfile.cpp +++ b/src/libsync/configfile.cpp @@ -91,13 +91,9 @@ const QString newBigFolderSizeLimitC() { return QStringLiteral("newBigFolderSize const QString useNewBigFolderSizeLimitC() { return QStringLiteral("useNewBigFolderSizeLimit"); } const QString confirmExternalStorageC() { return QStringLiteral("confirmExternalStorage"); } const QString moveToTrashC() { return QStringLiteral("moveToTrash"); } - -const QString certPath() { return QStringLiteral("http_certificatePath"); } -const QString certPasswd() { return QStringLiteral("http_certificatePasswd"); } } QString ConfigFile::_confDir = QString(); -bool ConfigFile::_askedUser = false; static chrono::milliseconds millisecondsValue(const QSettings &setting, const QString &key, chrono::milliseconds defaultValue) @@ -395,15 +391,6 @@ void ConfigFile::storeData(const QString &group, const QString &key, const QVari settings.sync(); } -QVariant ConfigFile::retrieveData(const QString &group, const QString &key) const -{ - const QString con(group.isEmpty() ? defaultConnection() : group); - QSettings settings(configFile(), QSettings::IniFormat); - - settings.beginGroup(con); - return settings.value(key); -} - void ConfigFile::removeData(const QString &group, const QString &key) { const QString con(group.isEmpty() ? defaultConnection() : group); @@ -845,30 +832,6 @@ bool ConfigFile::showExperimentalOptions() const return settings.value(showExperimentalOptionsC(), false).toBool(); } -QString ConfigFile::certificatePath() const -{ - return retrieveData(QString(), certPath()).toString(); -} - -void ConfigFile::setCertificatePath(const QString &cPath) -{ - QSettings settings(configFile(), QSettings::IniFormat); - settings.setValue(certPath(), cPath); - settings.sync(); -} - -QString ConfigFile::certificatePasswd() const -{ - return retrieveData(QString(), certPasswd()).toString(); -} - -void ConfigFile::setCertificatePasswd(const QString &cPasswd) -{ - QSettings settings(configFile(), QSettings::IniFormat); - settings.setValue(certPasswd(), cPasswd); - settings.sync(); -} - QString ConfigFile::clientVersionString() const { QSettings settings(configFile(), QSettings::IniFormat); diff --git a/src/libsync/configfile.h b/src/libsync/configfile.h index 11de1b94856..f8a89e909c3 100644 --- a/src/libsync/configfile.h +++ b/src/libsync/configfile.h @@ -61,10 +61,6 @@ class OWNCLOUDSYNC_EXPORT ConfigFile QString defaultConnection() const; - // the certs do not depend on a connection. - QByteArray caCerts(); - void setCaCerts(const QByteArray &); - bool passwordStorageAllowed(const QString &connection = QString()); /* Server poll interval in milliseconds */ @@ -178,11 +174,6 @@ class OWNCLOUDSYNC_EXPORT ConfigFile void saveGeometryHeader(QHeaderView *header); void restoreGeometryHeader(QHeaderView *header); - QString certificatePath() const; - void setCertificatePath(const QString &cPath); - QString certificatePasswd() const; - void setCertificatePasswd(const QString &cPasswd); - /** The client version that last used this settings file. Updated by configVersionMigration() at client startup. */ QString clientVersionString() const; @@ -198,7 +189,6 @@ class OWNCLOUDSYNC_EXPORT ConfigFile protected: QVariant getPolicySetting(const QString &policy, const QVariant &defaultValue = QVariant()) const; void storeData(const QString &group, const QString &key, const QVariant &value); - QVariant retrieveData(const QString &group, const QString &key) const; void removeData(const QString &group, const QString &key); bool dataExists(const QString &group, const QString &key) const; @@ -210,7 +200,6 @@ class OWNCLOUDSYNC_EXPORT ConfigFile private: typedef QSharedPointer SharedCreds; - static bool _askedUser; static QString _oCVersion; static QString _confDir; }; diff --git a/src/libsync/creds/httpcredentials.cpp b/src/libsync/creds/httpcredentials.cpp index 566b0072fab..25285597171 100644 --- a/src/libsync/creds/httpcredentials.cpp +++ b/src/libsync/creds/httpcredentials.cpp @@ -108,15 +108,6 @@ class HttpCredentialsAccessManager : public AccessManager req.setRawHeader("Authorization", "Basic " + credHash); } } - - if (_cred && !_cred->_clientSslKey.isNull() && !_cred->_clientSslCertificate.isNull()) { - // SSL configuration - QSslConfiguration sslConfiguration = req.sslConfiguration(); - sslConfiguration.setLocalCertificate(_cred->_clientSslCertificate); - sslConfiguration.setPrivateKey(_cred->_clientSslKey); - req.setSslConfiguration(sslConfiguration); - } - return AccessManager::createRequest(op, req, outgoingData); } @@ -126,18 +117,13 @@ class HttpCredentialsAccessManager : public AccessManager QPointer _cred; }; -HttpCredentials::HttpCredentials(DetermineAuthTypeJob::AuthType authType, const QString &user, const QString &password, const QByteArray &clientCertBundle, const QByteArray &clientCertPassword) +HttpCredentials::HttpCredentials(DetermineAuthTypeJob::AuthType authType, const QString &user, const QString &password) : _user(user) , _password(password) , _ready(true) - , _clientCertBundle(clientCertBundle) - , _clientCertPassword(clientCertPassword) , _retryOnKeyChainError(false) , _authType(authType) { - if (!unpackClientCertBundle(clientCertPassword)) { - OC_ASSERT_X(false, "pkcs12 client cert bundle passed to HttpCredentials must be valid"); - } } QString HttpCredentials::authType() const @@ -249,25 +235,6 @@ void HttpCredentials::fetchFromKeychainHelper() } }); }; - _clientCertBundle = _account->credentialSetting(clientCertBundleKeyC()).toByteArray(); - if (!_clientCertBundle.isEmpty()) { - // New case (>=2.6): We have a bundle in the settings and read the password from - // the keychain - auto job = _account->credentialManager()->get(clientCertPasswordKeyC()); - connect(job, &CredentialJob::finished, this, [job, readPassword, this] { - const auto clientCertPassword = job->data().toByteArray(); - if (job->error() != QKeychain::NoError) { - qCWarning(lcHttpLegacyCredentials) << "Could not retrieve client cert password from keychain" << job->errorString(); - } - // might be an empty passowrd - if (!unpackClientCertBundle(clientCertPassword)) { - qCWarning(lcHttpCredentials) << "Could not unpack client cert bundle"; - } - _clientCertBundle.clear(); - readPassword(); - }); - return; - } readPassword(); } @@ -364,22 +331,10 @@ void HttpCredentials::persist() _account->setCredentialSetting(CredentialVersionKey(), CredentialVersion); _account->setCredentialSetting(userC(), _user); _account->setCredentialSetting(isOAuthC(), isUsingOAuth()); - if (!_clientCertBundle.isEmpty()) { - // Note that the _clientCertBundle will often be cleared after usage, - // it's just written if it gets passed into the constructor. - _account->setCredentialSetting(clientCertBundleKeyC(), _clientCertBundle); - } Q_EMIT _account->wantsAccountSaved(_account); // write secrets to the keychain - if (!_clientCertBundle.isEmpty()) { - // If we have a pkcs12 bundle, that'll be written to the config file - // and we'll just store the bundle password in the keychain. That's prefered - // since the keychain on older Windows platforms can only store a limited number - // of bytes per entry and key/cert may exceed that. - _account->credentialManager()->set(clientCertPasswordKeyC(), _clientCertPassword); - _clientCertBundle.clear(); - } else if (isUsingOAuth()) { + if (isUsingOAuth()) { _account->credentialManager()->set(refreshTokenKeyC(), _refreshToken); } else { _account->credentialManager()->set(passwordKeyC(), _password); @@ -402,18 +357,6 @@ void HttpCredentials::slotAuthentication(QNetworkReply *reply, QAuthenticator *a } } -bool HttpCredentials::unpackClientCertBundle(const QByteArray &clientCertPassword) -{ - if (_clientCertBundle.isEmpty()) - return true; - - QBuffer certBuffer(&_clientCertBundle); - certBuffer.open(QIODevice::ReadOnly); - QList clientCaCertificates; - return QSslCertificate::importPkcs12( - &certBuffer, &_clientSslKey, &_clientSslCertificate, &clientCaCertificates, clientCertPassword); -} - } // namespace OCC #include "httpcredentials.moc" diff --git a/src/libsync/creds/httpcredentials.h b/src/libsync/creds/httpcredentials.h index fea320c2c35..2e0e20e4ace 100644 --- a/src/libsync/creds/httpcredentials.h +++ b/src/libsync/creds/httpcredentials.h @@ -56,8 +56,7 @@ class OWNCLOUDSYNC_EXPORT HttpCredentials : public AbstractCredentials /// Don't add credentials if this is set on a QNetworkRequest static constexpr QNetworkRequest::Attribute DontAddCredentialsAttribute = QNetworkRequest::User; - explicit HttpCredentials(DetermineAuthTypeJob::AuthType authType, const QString &user, const QString &password, - const QByteArray &clientCertBundle = QByteArray(), const QByteArray &clientCertPassword = QByteArray()); + explicit HttpCredentials(DetermineAuthTypeJob::AuthType authType, const QString &user, const QString &password); QString authType() const override; QNetworkAccessManager *createQNAM() const override; @@ -88,13 +87,6 @@ class OWNCLOUDSYNC_EXPORT HttpCredentials : public AbstractCredentials void deleteOldKeychainEntries(); void slotAuthentication(QNetworkReply *reply, QAuthenticator *authenticator); - - /** Takes client cert pkcs12 and unwraps the key/cert. - * - * Returns false on failure. - */ - bool unpackClientCertBundle(const QByteArray &clientCertPassword); - void fetchFromKeychainHelper(); QString _user; @@ -105,11 +97,6 @@ class OWNCLOUDSYNC_EXPORT HttpCredentials : public AbstractCredentials QString _fetchErrorString; bool _ready = false; bool _isRenewingOAuthToken = false; - QByteArray _clientCertBundle; - // used when called from the wizard - QByteArray _clientCertPassword; - QSslKey _clientSslKey; - QSslCertificate _clientSslCertificate; bool _retryOnKeyChainError = true; // true if we haven't done yet any reading from keychain DetermineAuthTypeJob::AuthType _authType = DetermineAuthTypeJob::AuthType::Unknown; diff --git a/src/libsync/creds/httpcredentials_p.h b/src/libsync/creds/httpcredentials_p.h index ccab5aa7cb1..73d403c5327 100644 --- a/src/libsync/creds/httpcredentials_p.h +++ b/src/libsync/creds/httpcredentials_p.h @@ -30,23 +30,6 @@ class HttpLegacyCredentials : public QObject { Q_OBJECT - const QString clientCertBundleC() - { - return QStringLiteral("clientCertPkcs12"); - } - const QString clientCertPasswordC() - { - return QStringLiteral("_clientCertPassword"); - } - const QString clientCertificatePEMC() - { - return QStringLiteral("_clientCertificatePEM"); - } - const QString clientKeyPEMC() - { - return QStringLiteral("_clientKeyPEM"); - } - auto isOAuthC() { return QStringLiteral("oauth"); @@ -61,37 +44,6 @@ class HttpLegacyCredentials : public QObject void fetchFromKeychainHelper() { - _parent->_clientCertBundle = _parent->_account->credentialSetting(clientCertBundleC()).toByteArray(); - - if (!_parent->_clientCertBundle.isEmpty()) { - // New case (>=2.6): We have a bundle in the settings and read the password from - // the keychain - QKeychain::ReadPasswordJob *job = new QKeychain::ReadPasswordJob(Theme::instance()->appName()); - job->setKey(_parent->keychainKey(_parent->_account->url().toString(), _parent->_user + clientCertPasswordC(), _parent->_account->id())); - job->setInsecureFallback(false); - connect(job, &QKeychain::ReadPasswordJob::finished, this, &HttpLegacyCredentials::slotReadClientCertPasswordJobDone); - job->start(); - return; - } - // Old case (pre 2.6): Read client cert and then key from keychain - const QString kck = _parent->keychainKey( - _parent->_account->url().toString(), - _parent->_user + clientCertificatePEMC(), - _keychainMigration ? QString() : _parent->_account->id()); - - QKeychain::ReadPasswordJob *job = new QKeychain::ReadPasswordJob(Theme::instance()->appName()); - addSettingsToJob(job); - job->setInsecureFallback(false); - job->setKey(kck); - connect(job, &QKeychain::ReadPasswordJob::finished, this, [job] { - if (job->error() == QKeychain::NoError && !job->binaryData().isEmpty()) { - const auto msg = tr("The support of client side certificate saved in the keychain was removed, please start the setup wizard again and follow the instructions."); - QMetaObject::invokeMethod(qApp, "slotShowGuiMessage", Qt::QueuedConnection, Q_ARG(QString, tr("Credentials")), Q_ARG(QString, msg)); - qCWarning(lcHttpLegacyCredentials) << msg; - } - }); - job->start(); - slotReadPasswordFromKeychain(); } @@ -121,29 +73,6 @@ class HttpLegacyCredentials : public QObject return false; } - void slotReadClientCertPasswordJobDone(QKeychain::Job *job) - { - auto readJob = qobject_cast(job); - if (keychainUnavailableRetryLater(readJob)) - return; - - if (readJob->error() == QKeychain::NoError) { - _parent->_clientCertPassword = readJob->binaryData(); - } else { - qCWarning(lcHttpLegacyCredentials) << "Could not retrieve client cert password from keychain" << readJob->errorString(); - } - - if (!_parent->unpackClientCertBundle(_parent->_clientCertPassword)) { - qCWarning(lcHttpLegacyCredentials) << "Could not unpack client cert bundle"; - } - - // don't clear the password, we are migrating the old settings - // _parent->_clientCertPassword.clear(); - // _parent->_clientCertBundle.clear(); - - slotReadPasswordFromKeychain(); - } - void slotReadPasswordFromKeychain() { const QString kck = _parent->keychainKey( @@ -222,14 +151,6 @@ class HttpLegacyCredentials : public QObject } } - void slotWriteClientCertPasswordJobDone(QKeychain::Job *finishedJob) - { - if (finishedJob && finishedJob->error() != QKeychain::NoError) { - qCWarning(lcHttpLegacyCredentials) << "Could not write client cert password to credentials" - << finishedJob->error() << finishedJob->errorString(); - } - } - void deleteOldKeychainEntries() { // oooold @@ -248,12 +169,7 @@ class HttpLegacyCredentials : public QObject // old startDeleteJob(_parent->_user, true); - startDeleteJob(_parent->_user + clientKeyPEMC(), true); - startDeleteJob(_parent->_user + clientCertificatePEMC(), true); - // pre 2.8 - startDeleteJob(_parent->_user + clientCertPasswordC()); - startDeleteJob(_parent->_user + clientKeyPEMC()); startDeleteJob(_parent->_user); } };