diff --git a/src/gui/addcertificatedialog.ui b/src/gui/addcertificatedialog.ui index a10ee27ab3d..92f6dbc7c19 100644 --- a/src/gui/addcertificatedialog.ui +++ b/src/gui/addcertificatedialog.ui @@ -9,8 +9,8 @@ 0 0 - 462 - 188 + 478 + 225 @@ -73,11 +73,30 @@ + + + + An encrypted pkcs12 bundle is strongly recommended as a copy will be stored in the configuration file. + + + true + + + + + + + 255 + 0 + 0 + + + @@ -89,6 +108,15 @@ + + + + 255 + 0 + 0 + + + @@ -100,6 +128,15 @@ + + + + 119 + 120 + 120 + + + @@ -112,9 +149,6 @@ - - - true diff --git a/src/gui/creds/httpcredentialsgui.h b/src/gui/creds/httpcredentialsgui.h index 6ceebafc649..a4973ce94f6 100644 --- a/src/gui/creds/httpcredentialsgui.h +++ b/src/gui/creds/httpcredentialsgui.h @@ -33,13 +33,14 @@ class HttpCredentialsGui : public HttpCredentials : HttpCredentials() { } - HttpCredentialsGui(const QString &user, const QString &password, const QSslCertificate &certificate, const QSslKey &key) - : HttpCredentials(user, password, certificate, key) + HttpCredentialsGui(const QString &user, const QString &password, + const QByteArray &clientCertBundle, const QByteArray &clientCertPassword) + : HttpCredentials(user, password, clientCertBundle, clientCertPassword) { } HttpCredentialsGui(const QString &user, const QString &password, const QString &refreshToken, - const QSslCertificate &certificate, const QSslKey &key) - : HttpCredentials(user, password, certificate, key) + const QByteArray &clientCertBundle, const QByteArray &clientCertPassword) + : HttpCredentials(user, password, clientCertBundle, clientCertPassword) { _refreshToken = refreshToken; } diff --git a/src/gui/wizard/owncloudhttpcredspage.cpp b/src/gui/wizard/owncloudhttpcredspage.cpp index 265d3bf3c55..6e4528d9332 100644 --- a/src/gui/wizard/owncloudhttpcredspage.cpp +++ b/src/gui/wizard/owncloudhttpcredspage.cpp @@ -191,7 +191,7 @@ void OwncloudHttpCredsPage::setErrorString(const QString &err) AbstractCredentials *OwncloudHttpCredsPage::getCredentials() const { - return new HttpCredentialsGui(_ui.leUsername->text(), _ui.lePassword->text(), _ocWizard->_clientSslCertificate, _ocWizard->_clientSslKey); + return new HttpCredentialsGui(_ui.leUsername->text(), _ui.lePassword->text(), _ocWizard->_clientCertBundle, _ocWizard->_clientCertPassword); } diff --git a/src/gui/wizard/owncloudoauthcredspage.cpp b/src/gui/wizard/owncloudoauthcredspage.cpp index c5b96d5279e..f1f2fcacf88 100644 --- a/src/gui/wizard/owncloudoauthcredspage.cpp +++ b/src/gui/wizard/owncloudoauthcredspage.cpp @@ -123,7 +123,7 @@ AbstractCredentials *OwncloudOAuthCredsPage::getCredentials() const OwncloudWizard *ocWizard = qobject_cast(wizard()); Q_ASSERT(ocWizard); return new HttpCredentialsGui(_user, _token, _refreshToken, - ocWizard->_clientSslCertificate, ocWizard->_clientSslKey); + ocWizard->_clientCertBundle, ocWizard->_clientCertPassword); } bool OwncloudOAuthCredsPage::isComplete() const diff --git a/src/gui/wizard/owncloudsetuppage.cpp b/src/gui/wizard/owncloudsetuppage.cpp index a1c180ab1e5..3dc8156ad6e 100644 --- a/src/gui/wizard/owncloudsetuppage.cpp +++ b/src/gui/wizard/owncloudsetuppage.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include "QProgressIndicator.h" @@ -295,12 +296,20 @@ void OwncloudSetupPage::slotCertificateAccepted() QList clientCaCertificates; QFile certFile(addCertDial->getCertificatePath()); certFile.open(QFile::ReadOnly); - if (QSslCertificate::importPkcs12(&certFile, + 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, - addCertDial->getCertificatePasswd().toLocal8Bit())) { - // The SSL cert gets added to the QSslConfiguration in checkServer() + &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?")); diff --git a/src/gui/wizard/owncloudwizard.h b/src/gui/wizard/owncloudwizard.h index 229f0edcd2b..8031a707273 100644 --- a/src/gui/wizard/owncloudwizard.h +++ b/src/gui/wizard/owncloudwizard.h @@ -78,8 +78,10 @@ class OwncloudWizard : public QWizard // FIXME: Can those be local variables? // Set from the OwncloudSetupPage, later used from OwncloudHttpCredsPage - QSslKey _clientSslKey; - QSslCertificate _clientSslCertificate; + 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 public slots: void setAuthType(DetermineAuthTypeJob::AuthType type); diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp index f9005151501..80ace028005 100644 --- a/src/libsync/account.cpp +++ b/src/libsync/account.cpp @@ -327,9 +327,9 @@ QVariant Account::credentialSetting(const QString &key) const { if (_credentials) { QString prefix = _credentials->authType(); - QString value = _settingsMap.value(prefix + "_" + key).toString(); - if (value.isEmpty()) { - value = _settingsMap.value(key).toString(); + QVariant value = _settingsMap.value(prefix + "_" + key); + if (value.isNull()) { + value = _settingsMap.value(key); } return value; } diff --git a/src/libsync/creds/httpcredentials.cpp b/src/libsync/creds/httpcredentials.cpp index 2e3ea00ace0..1dcd0263125 100644 --- a/src/libsync/creds/httpcredentials.cpp +++ b/src/libsync/creds/httpcredentials.cpp @@ -42,6 +42,8 @@ Q_LOGGING_CATEGORY(lcHttpCredentials, "sync.credentials.http", QtInfoMsg) namespace { const char userC[] = "user"; const char isOAuthC[] = "oauth"; + const char clientCertBundleC[] = "clientCertPkcs12"; + const char clientCertPasswordC[] = "_clientCertPassword"; const char clientCertificatePEMC[] = "_clientCertificatePEM"; const char clientKeyPEMC[] = "_clientKeyPEM"; const char authenticationFailedC[] = "owncloud-authentication-failed"; @@ -112,15 +114,18 @@ static void addSettingsToJob(Account *account, QKeychain::Job *job) } // From wizard -HttpCredentials::HttpCredentials(const QString &user, const QString &password, const QSslCertificate &certificate, const QSslKey &key) +HttpCredentials::HttpCredentials(const QString &user, const QString &password, const QByteArray &clientCertBundle, const QByteArray &clientCertPassword) : _user(user) , _password(password) , _ready(true) - , _clientSslKey(key) - , _clientSslCertificate(certificate) + , _clientCertBundle(clientCertBundle) + , _clientCertPassword(clientCertPassword) , _keychainMigration(false) , _retryOnKeyChainError(false) { + if (!unpackClientCertBundle()) { + ASSERT(false, "pkcs12 client cert bundle passed to HttpCredentials must be valid"); + } } QString HttpCredentials::authType() const @@ -191,7 +196,20 @@ void HttpCredentials::fetchFromKeychain() void HttpCredentials::fetchFromKeychainHelper() { - // Read client cert from keychain + _clientCertBundle = _account->credentialSetting(QLatin1String(clientCertBundleC)).toByteArray(); + if (!_clientCertBundle.isEmpty()) { + // New case (>=2.6): We have a bundle in the settings and read the password from + // the keychain + ReadPasswordJob *job = new ReadPasswordJob(Theme::instance()->appName()); + addSettingsToJob(_account, job); + job->setInsecureFallback(false); + job->setKey(keychainKey(_account->url().toString(), _user + clientCertPasswordC, _account->id())); + connect(job, &Job::finished, this, &HttpCredentials::slotReadClientCertPasswordJobDone); + job->start(); + return; + } + + // Old case (pre 2.6): Read client cert and then key from keychain const QString kck = keychainKey( _account->url().toString(), _user + clientCertificatePEMC, @@ -220,7 +238,7 @@ void HttpCredentials::deleteOldKeychainEntries() startDeleteJob(_user + clientCertificatePEMC); } -void HttpCredentials::slotReadClientCertPEMJobDone(QKeychain::Job *incoming) +bool HttpCredentials::keychainUnavailableRetryLater(QKeychain::Job *incoming) { #if defined(Q_OS_UNIX) && !defined(Q_OS_MAC) Q_ASSERT(!incoming->insecureFallback()); // If insecureFallback is set, the next test would be pointless @@ -232,10 +250,38 @@ void HttpCredentials::slotReadClientCertPEMJobDone(QKeychain::Job *incoming) qCInfo(lcHttpCredentials) << "Backend unavailable (yet?) Retrying in a few seconds." << incoming->errorString(); QTimer::singleShot(10000, this, &HttpCredentials::fetchFromKeychainHelper); _retryOnKeyChainError = false; - return; + return true; } - _retryOnKeyChainError = false; #endif + _retryOnKeyChainError = false; + return false; +} + +void HttpCredentials::slotReadClientCertPasswordJobDone(QKeychain::Job *job) +{ + if (keychainUnavailableRetryLater(job)) + return; + + ReadPasswordJob *readJob = static_cast(job); + if (readJob->error() == NoError) { + _clientCertPassword = readJob->binaryData(); + } else { + qCWarning(lcHttpCredentials) << "Could not retrieve client cert password from keychain" << job->errorString(); + } + + if (!unpackClientCertBundle()) { + qCWarning(lcHttpCredentials) << "Could not unpack client cert bundle"; + } + _clientCertBundle.clear(); + _clientCertPassword.clear(); + + slotReadPasswordFromKeychain(); +} + +void HttpCredentials::slotReadClientCertPEMJobDone(QKeychain::Job *incoming) +{ + if (keychainUnavailableRetryLater(incoming)) + return; // Store PEM in memory ReadPasswordJob *readJob = static_cast(incoming); @@ -281,7 +327,11 @@ void HttpCredentials::slotReadClientKeyPEMJobDone(QKeychain::Job *incoming) } } - // Now fetch the actual server password + slotReadPasswordFromKeychain(); +} + +void HttpCredentials::slotReadPasswordFromKeychain() +{ const QString kck = keychainKey( _account->url().toString(), _user, @@ -467,10 +517,32 @@ void HttpCredentials::persist() _account->setCredentialSetting(QLatin1String(userC), _user); _account->setCredentialSetting(QLatin1String(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(QLatin1String(clientCertBundleC), _clientCertBundle); + } _account->wantsAccountSaved(_account); - // write cert if there is one - if (!_clientSslCertificate.isNull()) { + // write secrets to the keychain + if (!_clientCertBundle.isEmpty()) { + // Option 1: 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. + WritePasswordJob *job = new WritePasswordJob(Theme::instance()->appName()); + addSettingsToJob(_account, job); + job->setInsecureFallback(false); + connect(job, &Job::finished, this, &HttpCredentials::slotWriteClientCertPasswordJobDone); + job->setKey(keychainKey(_account->url().toString(), _user + clientCertPasswordC, _account->id())); + job->setBinaryData(_clientCertPassword); + job->start(); + _clientCertBundle.clear(); + _clientCertPassword.clear(); + } else if (_account->credentialSetting(QLatin1String(clientCertBundleC)).isNull() && !_clientSslCertificate.isNull()) { + // Option 2, pre 2.6 configs: We used to store the raw cert/key in the keychain and + // still do so if no bundle is available. We can't currently migrate to Option 1 + // because we have no functions for creating an encrypted pkcs12 bundle. WritePasswordJob *job = new WritePasswordJob(Theme::instance()->appName()); addSettingsToJob(_account, job); job->setInsecureFallback(false); @@ -479,10 +551,21 @@ void HttpCredentials::persist() job->setBinaryData(_clientSslCertificate.toPem()); job->start(); } else { - slotWriteClientCertPEMJobDone(nullptr); + // Option 3: no client certificate at all (or doesn't need to be written) + slotWritePasswordToKeychain(); } } +void HttpCredentials::slotWriteClientCertPasswordJobDone(Job *finishedJob) +{ + if (finishedJob && finishedJob->error() != QKeychain::NoError) { + qCWarning(lcHttpCredentials) << "Could not write client cert password to credentials" + << finishedJob->error() << finishedJob->errorString(); + } + + slotWritePasswordToKeychain(); +} + void HttpCredentials::slotWriteClientCertPEMJobDone(Job *finishedJob) { if (finishedJob && finishedJob->error() != QKeychain::NoError) { @@ -511,6 +594,11 @@ void HttpCredentials::slotWriteClientKeyPEMJobDone(Job *finishedJob) << finishedJob->error() << finishedJob->errorString(); } + slotWritePasswordToKeychain(); +} + +void HttpCredentials::slotWritePasswordToKeychain() +{ WritePasswordJob *job = new WritePasswordJob(Theme::instance()->appName()); addSettingsToJob(_account, job); job->setInsecureFallback(false); @@ -560,4 +648,16 @@ bool HttpCredentials::retryIfNeeded(AbstractNetworkJob *job) return true; } +bool HttpCredentials::unpackClientCertBundle() +{ + if (_clientCertBundle.isEmpty()) + return true; + + QBuffer certBuffer(&_clientCertBundle); + certBuffer.open(QIODevice::ReadOnly); + QList clientCaCertificates; + return QSslCertificate::importPkcs12( + &certBuffer, &_clientSslKey, &_clientSslCertificate, &clientCaCertificates, _clientCertPassword); +} + } // namespace OCC diff --git a/src/libsync/creds/httpcredentials.h b/src/libsync/creds/httpcredentials.h index 86325e0520d..d1f4156015f 100644 --- a/src/libsync/creds/httpcredentials.h +++ b/src/libsync/creds/httpcredentials.h @@ -80,7 +80,8 @@ class OWNCLOUDSYNC_EXPORT HttpCredentials : public AbstractCredentials static constexpr QNetworkRequest::Attribute DontAddCredentialsAttribute = QNetworkRequest::User; HttpCredentials() = default; - explicit HttpCredentials(const QString &user, const QString &password, const QSslCertificate &certificate = QSslCertificate(), const QSslKey &key = QSslKey()); + explicit HttpCredentials(const QString &user, const QString &password, + const QByteArray &clientCertBundle = QByteArray(), const QByteArray &clientCertPassword = QByteArray()); QString authType() const Q_DECL_OVERRIDE; QNetworkAccessManager *createQNAM() const Q_DECL_OVERRIDE; @@ -112,12 +113,18 @@ class OWNCLOUDSYNC_EXPORT HttpCredentials : public AbstractCredentials private Q_SLOTS: void slotAuthentication(QNetworkReply *, QAuthenticator *); + void slotReadClientCertPasswordJobDone(QKeychain::Job *); void slotReadClientCertPEMJobDone(QKeychain::Job *); void slotReadClientKeyPEMJobDone(QKeychain::Job *); + + void slotReadPasswordFromKeychain(); void slotReadJobDone(QKeychain::Job *); + void slotWriteClientCertPasswordJobDone(QKeychain::Job *); void slotWriteClientCertPEMJobDone(QKeychain::Job *); void slotWriteClientKeyPEMJobDone(QKeychain::Job *); + + void slotWritePasswordToKeychain(); void slotWriteJobDone(QKeychain::Job *); protected: @@ -133,6 +140,22 @@ private Q_SLOTS: /// Wipes legacy keychain locations void deleteOldKeychainEntries(); + /** Whether to bow out now because a retry will happen later + * + * Sometimes the keychain needs a while to become available. + * This function should be called on first keychain-read to check + * whether it errored because the keychain wasn't available yet. + * If that happens, this function will schedule another try and + * return true. + */ + bool keychainUnavailableRetryLater(QKeychain::Job *); + + /** Takes client cert pkcs12 and unwraps the key/cert. + * + * Returns false on failure. + */ + bool unpackClientCertBundle(); + QString _user; QString _password; // user's password, or access_token for OAuth QString _refreshToken; // OAuth _refreshToken, set if OAuth is used. @@ -141,6 +164,8 @@ private Q_SLOTS: QString _fetchErrorString; bool _ready = false; bool _isRenewingOAuthToken = false; + QByteArray _clientCertBundle; + QByteArray _clientCertPassword; QSslKey _clientSslKey; QSslCertificate _clientSslCertificate; bool _keychainMigration = false;