From 72be80cbd972113930f84529764f3a1a3ada0292 Mon Sep 17 00:00:00 2001 From: Michael Schuster Date: Sat, 7 Dec 2019 05:27:50 +0100 Subject: [PATCH] Windows: Workaround for storing >= 4k (4096 bit) client-cert SSL keys With QtKeychain on Windows, storing larger keys in one keychain entry causes the following error due to limits in the Windows APIs: Error: "Credential size exceeds maximum size of 2560" To avoid overhead on the other platforms and balance code duplication, this approach puts some read- and write-parts into Windows-only defines. For reference also see previous fixes: - https://github.com/nextcloud/desktop/pull/1389 - https://github.com/nextcloud/desktop/pull/1394 This (again) fixes the re-opened issue: - https://github.com/nextcloud/desktop/issues/863 Signed-off-by: Michael Schuster --- src/gui/creds/webflowcredentials.cpp | 114 +++++++++++++++++++++++++-- src/gui/creds/webflowcredentials.h | 16 +++- 2 files changed, 123 insertions(+), 7 deletions(-) diff --git a/src/gui/creds/webflowcredentials.cpp b/src/gui/creds/webflowcredentials.cpp index d69bb125f..e55b95a02 100644 --- a/src/gui/creds/webflowcredentials.cpp +++ b/src/gui/creds/webflowcredentials.cpp @@ -246,15 +246,77 @@ void WebFlowCredentials::slotWriteClientCertPEMJobDone() { // write ssl key if there is one if (!_clientSslKey.isNull()) { + QByteArray sslKey = _clientSslKey.toPem(); + +#if defined(Q_OS_WIN) + // Workaround: Split the private key into chunks of 2048 bytes, + // to allow 4k (4096 bit) keys to be saved (obey Windows's limits) + while (!sslKey.isEmpty()) { + QByteArray chunk = sslKey.left(_clientSslCaKeyChunkSize); + + _clientSslCaKeyWriteQueue.append(chunk); + + sslKey = sslKey.right(sslKey.size() - chunk.size()); + chunk.clear(); + } + + _clientSslCaKeyChunkCount = _clientSslCaKeyWriteQueue.count(); +#else + // write full key in one slot on non-Windows, as usual + _clientSslCaKeyWriteQueue.append(sslKey); + _clientSslCaKeyChunkCount = 1; +#endif + + writeSingleClientCaKeyPEM(nullptr); + + sslKey.clear(); + } else { + // no key, just write credentials + slotWriteClientKeyPEMJobDone(); + } +} + +void WebFlowCredentials::writeSingleClientCaKeyPEM(QKeychain::Job *incomingJob) +{ + // errors? + if (incomingJob) { + WritePasswordJob *writeJob = static_cast(incomingJob); + + if (writeJob->error() != NoError) { + qCWarning(lcWebFlowCredentials) << "Error while writing client CA key chunk" << writeJob->errorString(); + + _clientSslCaKeyWriteQueue.clear(); + } + } + + // write a ca key chunk if there is any in the queue + if (!_clientSslCaKeyWriteQueue.isEmpty()) { + // grab and remove the first chunk from the queue + auto chunk = _clientSslCaKeyWriteQueue.dequeue(); + + auto index = (_clientSslCaKeyChunkCount - _clientSslCaKeyWriteQueue.count()) - 1; + + // keep the limit + if (index > (_clientSslCaKeyMaxChunks - 1)) { + qCWarning(lcWebFlowCredentials) << "Maximum client CA key chunk count exceeded while writing slot" << QString::number(index) << "), cutting off after " << QString::number(_clientSslCaKeyMaxChunks) << "chunks"; + + _clientSslCaKeyWriteQueue.clear(); + + slotWriteClientKeyPEMJobDone(); + return; + } + WritePasswordJob *job = new WritePasswordJob(Theme::instance()->appName()); addSettingsToJob(_account, job); job->setInsecureFallback(false); - connect(job, &Job::finished, this, &WebFlowCredentials::slotWriteClientKeyPEMJobDone); - job->setKey(keychainKey(_account->url().toString(), _user + clientKeyPEMC, _account->id())); - job->setBinaryData(_clientSslKey.toPem()); + connect(job, &Job::finished, this, &WebFlowCredentials::writeSingleClientCaKeyPEM); + // only add the "index" after the first element, to stay compatible with older versions and non-Windows + job->setKey(keychainKey(_account->url().toString(), _user + clientKeyPEMC + (index > 0 ? QString::number(index) : QString()), _account->id())); + job->setBinaryData(chunk); job->start(); + + chunk.clear(); } else { - // no key, just write credentials slotWriteClientKeyPEMJobDone(); } } @@ -472,6 +534,9 @@ void WebFlowCredentials::slotReadClientCertPEMJobDone(QKeychain::Job *incomingJo } // Load key too + _clientSslCaKeyChunkCount = 0; + _clientSslCaKeyReadBuffer.clear(); + const QString kck = keychainKey( _account->url().toString(), _user + clientKeyPEMC, @@ -487,11 +552,45 @@ void WebFlowCredentials::slotReadClientCertPEMJobDone(QKeychain::Job *incomingJo void WebFlowCredentials::slotReadClientKeyPEMJobDone(QKeychain::Job *incomingJob) { - // Store key in memory + // Errors or next ca key chunk? ReadPasswordJob *readJob = static_cast(incomingJob); + if (readJob) { if (readJob->error() == NoError && readJob->binaryData().length() > 0) { - QByteArray clientKeyPEM = readJob->binaryData(); + _clientSslCaKeyReadBuffer.append(readJob->binaryData()); + _clientSslCaKeyChunkCount++; + +#if defined(Q_OS_WIN) + // try to fetch next chunk + if (_clientSslCaKeyChunkCount < _clientSslCaKeyMaxChunks) { + const QString kck = keychainKey( + _account->url().toString(), + _user + clientKeyPEMC + QString::number(_clientSslCaKeyChunkCount), + _keychainMigration ? QString() : _account->id()); + + ReadPasswordJob *job = new ReadPasswordJob(Theme::instance()->appName()); + addSettingsToJob(_account, job); + job->setInsecureFallback(false); + job->setKey(kck); + connect(job, &Job::finished, this, &WebFlowCredentials::slotReadClientKeyPEMJobDone); + job->start(); + + return; + } else { + qCWarning(lcWebFlowCredentials) << "Maximum client CA key chunk count reached, ignoring after " << _clientSslCaKeyMaxChunks; + } +#endif + } else { + if (readJob->error() != QKeychain::Error::EntryNotFound || + ((readJob->error() == QKeychain::Error::EntryNotFound) && _clientSslCaKeyChunkCount == 0)) { + qCWarning(lcWebFlowCredentials) << "Unable to read client CA key chunk slot " << QString::number(_clientSslCaKeyChunkCount) << readJob->errorString(); + } + } + } + + // Store key in memory + if (_clientSslCaKeyReadBuffer.size() > 0) { + QByteArray clientKeyPEM = _clientSslCaKeyReadBuffer; // FIXME Unfortunately Qt has a bug and we can't just use QSsl::Opaque to let it // load whatever we have. So we try until it works. _clientSslKey = QSslKey(clientKeyPEM, QSsl::Rsa); @@ -504,6 +603,9 @@ void WebFlowCredentials::slotReadClientKeyPEMJobDone(QKeychain::Job *incomingJob if (_clientSslKey.isNull()) { qCWarning(lcWebFlowCredentials) << "Could not load SSL key into Qt!"; } + clientKeyPEM.clear(); + _clientSslCaKeyChunkCount = 0; + _clientSslCaKeyReadBuffer.clear(); } // Start fetching client CA certs diff --git a/src/gui/creds/webflowcredentials.h b/src/gui/creds/webflowcredentials.h index 0e02568ec..32742be86 100644 --- a/src/gui/creds/webflowcredentials.h +++ b/src/gui/creds/webflowcredentials.h @@ -83,7 +83,7 @@ private: void writeSingleClientCaCertPEM(); /* - * Since we're limited by Windows limits we just create our own + * Since we're limited by Windows limits, we just create our own * limit to avoid evil things happening by endless recursion * * Better than storing the count and relying on maybe-hacked values @@ -91,6 +91,20 @@ private: static constexpr int _clientSslCaCertificatesMaxCount = 10; QQueue _clientSslCaCertificatesWriteQueue; + /* + * Workaround: ...and this time only on Windows: + * + * Split the private key into chunks of 2048 bytes, + * to allow 4k (4096 bit) keys to be saved (see limits above) + */ + void writeSingleClientCaKeyPEM(QKeychain::Job *incomingJob); + + static constexpr int _clientSslCaKeyChunkSize = 2048; + static constexpr int _clientSslCaKeyMaxChunks = 10; + QQueue _clientSslCaKeyWriteQueue; + int _clientSslCaKeyChunkCount = 0; + QByteArray _clientSslCaKeyReadBuffer; + protected: /** Reads data from keychain locations *