Client certs: Store pkcs12 in config, password in keychain

It still reads and writes the old format too, but all newly stored
client certs will be in the new form.

For #6776 because Windows limits credential data to 512 bytes in older
versions.

src/gui/addcertificatedialog.ui

@@ -9,8 +9,8 @@
  <rect>
  <x>0</x>
  <y>0</y>
- <width>462</width>
- <height>188</height>
+ <width>478</width>
+ <height>225</height>
  </rect>
  </property>
  <property name="windowTitle">
@@ -73,11 +73,30 @@
  </item>
  </layout>
  </item>
+ <item>
+ <widget class="QLabel" name="label">
+ <property name="text">
+ <string>An encrypted pkcs12 bundle is strongly recommended as a copy will be stored in the configuration file.</string>
+ </property>
+ <property name="wordWrap">
+ <bool>true</bool>
+ </property>
+ </widget>
+ </item>
  <item>
  <widget class="QLabel" name="labelErrorCertif">
  <property name="palette">
  <palette>
  <active>
+ <colorrole role="WindowText">
+ <brush brushstyle="SolidPattern">
+ <color alpha="255">
+ <red>255</red>
+ <green>0</green>
+ <blue>0</blue>
+ </color>
+ </brush>
+ </colorrole>
  <colorrole role="Text">
  <brush brushstyle="SolidPattern">
  <color alpha="255">
@@ -89,6 +108,15 @@
  </colorrole>
  </active>
  <inactive>
+ <colorrole role="WindowText">
+ <brush brushstyle="SolidPattern">
+ <color alpha="255">
+ <red>255</red>
+ <green>0</green>
+ <blue>0</blue>
+ </color>
+ </brush>
+ </colorrole>
  <colorrole role="Text">
  <brush brushstyle="SolidPattern">
  <color alpha="255">
@@ -100,6 +128,15 @@
  </colorrole>
  </inactive>
  <disabled>
+ <colorrole role="WindowText">
+ <brush brushstyle="SolidPattern">
+ <color alpha="255">
+ <red>119</red>
+ <green>120</green>
+ <blue>120</blue>
+ </color>
+ </brush>
+ </colorrole>
  <colorrole role="Text">
  <brush brushstyle="SolidPattern">
  <color alpha="255">
@@ -112,9 +149,6 @@
  </disabled>
  </palette>
  </property>
- <property name="text">
- <string/>
- </property>
  <property name="wordWrap">
  <bool>true</bool>
  </property>

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;
  }

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);
 }
 
 

src/gui/wizard/owncloudoauthcredspage.cpp

@@ -123,7 +123,7 @@ AbstractCredentials *OwncloudOAuthCredsPage::getCredentials() const
  OwncloudWizard *ocWizard = qobject_cast<OwncloudWizard *>(wizard());
  Q_ASSERT(ocWizard);
  return new HttpCredentialsGui(_user, _token, _refreshToken,
- ocWizard->_clientSslCertificate, ocWizard->_clientSslKey);
+ ocWizard->_clientCertBundle, ocWizard->_clientCertPassword);
 }
 
 bool OwncloudOAuthCredsPage::isComplete() const

src/gui/wizard/owncloudsetuppage.cpp

@@ -22,6 +22,7 @@
 #include <QSsl>
 #include <QSslCertificate>
 #include <QNetworkAccessManager>
+#include <QBuffer>
 
 #include "QProgressIndicator.h"
 
@@ -295,12 +296,20 @@ void OwncloudSetupPage::slotCertificateAccepted()
  QList<QSslCertificate> 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?"));

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);

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;
  }

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<ReadPasswordJob *>(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<ReadPasswordJob *>(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<QSslCertificate> clientCaCertificates;
+ return QSslCertificate::importPkcs12(
+ &certBuffer, &_clientSslKey, &_clientSslCertificate, &clientCaCertificates, _clientCertPassword);
+}
+
 } // namespace OCC