Skip to content

[stable-4.0] Bugfix/e2ee fixes for hardware certificate end to end encryption #9614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mgallien merged 4 commits into from
Mar 12, 2026
+64 −55
Merged

[stable-4.0] Bugfix/e2ee fixes for hardware certificate end to end encryption #9614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c829566
fix(e2ee): use the correct certificate when recovering folder metadata
mgallien Feb 24, 2026
911987c
fix(e2ee): we need a valid certificate finger print for all types
mgallien Feb 24, 2026
6797bd7
fix(e2ee): store certificate handle rather than key
mgallien Feb 25, 2026
bbb33aa
fix(e2ee): generate valid metadata after certificate migration
mgallien Feb 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 40 additions & 41 deletions src/libsync/clientsideencryption.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
/*
* SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: GPL-2.0-or-later
Expand Down Expand Up @@ -62,6 +62,7 @@
}

using namespace QKeychain;
using namespace Qt::StringLiterals;

namespace OCC
{
Expand Down Expand Up @@ -590,9 +591,9 @@
}

if (encryptionEngine.useTokenBasedEncryption()) {
qCDebug(lcCseEncryption()) << "use certificate on hardware token";
qCDebug(lcCseDecryption()) << "use certificate on hardware token" << selectedCertificate.sha256Fingerprint();
} else {
qCDebug(lcCseEncryption()) << "use certificate on software storage";
qCDebug(lcCseDecryption()) << "use certificate on software storage" << selectedCertificate.sha256Fingerprint();
}

auto encryptedBase64Result = QByteArray{};
Expand Down Expand Up @@ -639,9 +640,9 @@
}

if (encryptionEngine.useTokenBasedEncryption()) {
qCDebug(lcCseDecryption()) << "use certificate on hardware token";
qCDebug(lcCseDecryption()) << "use certificate on hardware token" << selectedCertificate.sha256Fingerprint();
} else {
qCDebug(lcCseDecryption()) << "use certificate on software storage";
qCDebug(lcCseDecryption()) << "use certificate on software storage" << selectedCertificate.sha256Fingerprint();
}

auto decryptBase64Result = QByteArray{};
Expand Down Expand Up @@ -1034,11 +1035,7 @@

QByteArray ClientSideEncryption::certificateSha256Fingerprint() const
{
if (useTokenBasedEncryption()) {
return _encryptionCertificate.sha256Fingerprint();
}

return {};
return _encryptionCertificate.sha256Fingerprint();
}

void ClientSideEncryption::setAccount(const AccountPtr &account)
Expand Down Expand Up @@ -1117,7 +1114,7 @@
}
}

void ClientSideEncryption::initializeHardwareTokenEncryption(QWidget *settingsDialog)

Check failure on line 1117 in src/libsync/clientsideencryption.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 60 to the 25 allowed. 


See more on https://sonarcloud.io/project/issues?id=nextcloud_desktop&issues=AZziIQ9-PhPIe6f1CX8j&open=AZziIQ9-PhPIe6f1CX8j&pullRequest=9614
{
auto ctx = Pkcs11Context{Pkcs11Context::State::CreateContext};
_tokenSlots.reset();
Expand Down Expand Up @@ -1275,32 +1272,34 @@
return;
}

qCDebug(lcCse) << "checking the type of the key associated to the certificate";
qCDebug(lcCse) << "key type" << Qt::hex << PKCS11_get_key_type(certificateKey);
qCDebug(lcCse) << "checking the type of the key associated to the certificate" << sslCertificate.digest(QCryptographicHash::Sha256).toBase64();
qCDebug(lcCse) << "key type" << Qt::hex << PKCS11_get_key_type(certificateKey) << certificateKey;

auto newCertificateInformation = CertificateInformation{currentCertificate, std::move(sslCertificate)};

if (newCertificateInformation.isSelfSigned()) {
qCDebug(lcCse()) << "newly found certificate is self signed: goint to ignore it";
continue;
}

_otherCertificates.emplace_back(certificateKey, std::move(sslCertificate));
const auto &sslErrors = newCertificateInformation.verify();
for (const auto &sslError : sslErrors) {
qCInfo(lcCse()) << "certificate validation error" << sslError;
}

_otherCertificates.push_back(std::move(newCertificateInformation));
}
}

for (const auto &oneCertificateInformation : _otherCertificates) {
if (oneCertificateInformation.isSelfSigned()) {
qCDebug(lcCse()) << "newly found certificate is self signed: goint to ignore it";
continue;
}

if (!_usbTokenInformation.sha256Fingerprint().isEmpty() && oneCertificateInformation.sha256Fingerprint() != _usbTokenInformation.sha256Fingerprint()) {
qCDebug(lcCse()) << "skipping certificate from" << "with fingerprint" << oneCertificateInformation.sha256Fingerprint() << "different from" << _usbTokenInformation.sha256Fingerprint();
continue;
}

const auto &sslErrors = oneCertificateInformation.verify();
for (const auto &sslError : sslErrors) {
qCInfo(lcCse()) << "certificate validation error" << sslError;
}

setEncryptionCertificate(oneCertificateInformation);

if (canEncrypt() && !checkEncryptionIsWorking()) {
if (oneCertificateInformation.canEncrypt() && !checkEncryptionIsWorking(oneCertificateInformation)) {
qCWarning(lcCse()) << "encryption is not properly setup";

failedToInitialize();
Expand Down Expand Up @@ -1362,26 +1361,26 @@
job->start();
}

bool ClientSideEncryption::checkEncryptionIsWorking()
bool ClientSideEncryption::checkEncryptionIsWorking(const CertificateInformation &currentCertificate)
{
qCInfo(lcCse) << "check encryption is working before enabling end-to-end encryption feature";
QByteArray data = EncryptionHelper::generateRandom(64);

auto encryptedData = EncryptionHelper::encryptStringAsymmetric(getCertificateInformation(), paddingMode(), *this, data);
auto encryptedData = EncryptionHelper::encryptStringAsymmetric(currentCertificate, paddingMode(), *this, data);
if (!encryptedData) {
qCWarning(lcCse()) << "encryption error";
return false;
}

qCDebug(lcCse) << "encryption is working with" << getCertificateInformation().sha256Fingerprint();
qCDebug(lcCse) << "encryption is working with" << currentCertificate.sha256Fingerprint();

const auto decryptionResult = EncryptionHelper::decryptStringAsymmetric(getCertificateInformation(), paddingMode(), *this, *encryptedData);
const auto decryptionResult = EncryptionHelper::decryptStringAsymmetric(currentCertificate, paddingMode(), *this, *encryptedData);
if (!decryptionResult) {
qCWarning(lcCse()) << "encryption error";
return false;
}

qCDebug(lcCse) << "decryption is working with" << getCertificateInformation().sha256Fingerprint();
qCDebug(lcCse) << "decryption is working with" << currentCertificate.sha256Fingerprint();

QByteArray decryptResult = QByteArray::fromBase64(*decryptionResult);

Expand All @@ -1390,7 +1389,7 @@
return false;
}

qCInfo(lcCse) << "end-to-end encryption is working with" << getCertificateInformation().sha256Fingerprint();
qCInfo(lcCse) << "end-to-end encryption is working with" << currentCertificate.sha256Fingerprint();

return true;
}
Expand Down Expand Up @@ -2317,7 +2316,7 @@
}
}

if (!getPrivateKey().isNull() && checkEncryptionIsWorking()) {
if (!getPrivateKey().isNull() && checkEncryptionIsWorking(_encryptionCertificate)) {
writePrivateKey();
writeCertificate();
writeMnemonic([] () {});
Expand Down Expand Up @@ -3033,9 +3032,9 @@
checkEncryptionCertificate();
}

CertificateInformation::CertificateInformation(PKCS11_KEY *hardwarePrivateKey,
CertificateInformation::CertificateInformation(PKCS11_CERT *hardwareCertificate,

Check warning on line 3035 in src/libsync/clientsideencryption.cpp

View workflow job for this annotation

GitHub Actions / build

src/libsync/clientsideencryption.cpp:3035:1 [cppcoreguidelines-pro-type-member-init] 

constructor does not initialize these fields: _privateKeyData, _certificate

Check warning on line 3035 in src/libsync/clientsideencryption.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Unmodified variable "hardwareCertificate" of type "struct PKCS11_cert_st *" should be const-qualified. 


See more on https://sonarcloud.io/project/issues?id=nextcloud_desktop&issues=AZziIQ9-PhPIe6f1CX8k&open=AZziIQ9-PhPIe6f1CX8k&pullRequest=9614
QSslCertificate &&certificate)
: _hardwarePrivateKey{hardwarePrivateKey}
: _hardwareCertificate{hardwareCertificate}
, _certificate{std::move(certificate)}
, _certificateType{CertificateType::HardwareCertificate}
{
Expand All @@ -3045,7 +3044,7 @@
CertificateInformation::CertificateInformation(CertificateType certificateType,
const QByteArray &privateKey,
QSslCertificate &&certificate)
: _hardwarePrivateKey()
: _hardwareCertificate()
, _privateKeyData()
, _certificate(std::move(certificate))
, _certificateType{certificateType}
Expand All @@ -3072,7 +3071,7 @@

void CertificateInformation::clear()
{
_hardwarePrivateKey = nullptr;
_hardwareCertificate = nullptr;
_privateKeyData.clear();
_certificate.clear();
_certificateExpired = true;
Expand Down Expand Up @@ -3140,13 +3139,13 @@

PKCS11_KEY *CertificateInformation::getPkcs11PrivateKey() const
{
return canDecrypt() ? _hardwarePrivateKey : nullptr;
return canDecrypt() && _hardwareCertificate ? PKCS11_find_key(_hardwareCertificate) : nullptr;
}

PKey CertificateInformation::getEvpPrivateKey() const
{
if (_hardwarePrivateKey) {
return PKey::readHardwarePrivateKey(_hardwarePrivateKey);
if (_hardwareCertificate) {
return PKey::readHardwarePrivateKey(PKCS11_find_key(_hardwareCertificate));
} else {
const auto privateKeyPem = _privateKeyData;
Q_ASSERT(!privateKeyPem.isEmpty());
Expand All @@ -3167,23 +3166,23 @@

bool CertificateInformation::canEncrypt() const
{
return (_hardwarePrivateKey || !_certificate.isNull()) && !_certificateExpired && !_certificateNotYetValid && !_certificateRevoked && !_certificateInvalid;
return (_hardwareCertificate || !_certificate.isNull()) && !_certificateExpired && !_certificateNotYetValid && !_certificateRevoked && !_certificateInvalid;
}

bool CertificateInformation::canDecrypt() const
{
return _hardwarePrivateKey || !_privateKeyData.isEmpty();
return _hardwareCertificate || !_privateKeyData.isEmpty();
}

bool CertificateInformation::userCertificateNeedsMigration() const
{
return _hardwarePrivateKey &&
return _hardwareCertificate &&
(_certificateExpired || _certificateNotYetValid || _certificateRevoked || _certificateInvalid);
}

bool CertificateInformation::sensitiveDataRemaining() const
{
return _hardwarePrivateKey && !_privateKeyData.isEmpty() && !_certificate.isNull();
return _hardwareCertificate && !_privateKeyData.isEmpty() && !_certificate.isNull();
}

QByteArray CertificateInformation::sha256Fingerprint() const
Expand Down
6 changes: 3 additions & 3 deletions src/libsync/clientsideencryption.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
/*
* SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: GPL-2.0-or-later
Expand All @@ -6,7 +6,7 @@
#ifndef CLIENTSIDEENCRYPTION_H
#define CLIENTSIDEENCRYPTION_H

#include "owncloudlib.h"

Check failure on line 9 in src/libsync/clientsideencryption.h

View workflow job for this annotation

GitHub Actions / build

src/libsync/clientsideencryption.h:9:10 [clang-diagnostic-error] 

'owncloudlib.h' file not found

#include "clientsideencryptionprimitives.h"
#include "accountfwd.h"
Expand Down Expand Up @@ -55,7 +55,7 @@

CertificateInformation();

explicit CertificateInformation(PKCS11_KEY *hardwarePrivateKey,
explicit CertificateInformation(PKCS11_CERT *hardwareCertificate,
QSslCertificate &&certificate);

explicit CertificateInformation(CertificateType certificateType,
Expand Down Expand Up @@ -99,7 +99,7 @@

void doNotCheckEncryptionCertificate();

PKCS11_KEY* _hardwarePrivateKey = nullptr;
PKCS11_CERT* _hardwareCertificate = nullptr;

QByteArray _privateKeyData;

Expand Down Expand Up @@ -391,7 +391,7 @@
[[nodiscard]] bool checkServerPublicKeyValidity(const QByteArray &serverPublicKeyString) const;
[[nodiscard]] bool sensitiveDataRemaining() const;

[[nodiscard]] bool checkEncryptionIsWorking();
[[nodiscard]] bool checkEncryptionIsWorking(const CertificateInformation &currentCertificate);

void failedToInitialize();

Expand Down
21 changes: 14 additions & 7 deletions src/libsync/foldermetadata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
/*
* SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: GPL-2.0-or-later
Expand Down Expand Up @@ -181,7 +181,8 @@

if (_folderUsers.contains(_account->davUser())) {
const auto currentFolderUser = _folderUsers.value(_account->davUser());
_metadataKeyForEncryption = QByteArray::fromBase64(decryptDataWithPrivateKey(currentFolderUser.encryptedMetadataKey));
const auto currentUserCertificate = QSslCertificate{currentFolderUser.certificatePem};
_metadataKeyForEncryption = QByteArray::fromBase64(decryptDataWithPrivateKey(currentFolderUser.encryptedMetadataKey, currentUserCertificate.digest(QCryptographicHash::Sha256).toBase64()));
_metadataKeyForDecryption = _metadataKeyForEncryption;
}

Expand Down Expand Up @@ -279,7 +280,7 @@
const auto metadataKeyFromJson = fullMetaDataObj[metadataKeyKey].toString().toLocal8Bit();
if (!metadataKeyFromJson.isEmpty()) {
// parse version 1.1 and 1.2 (both must have a single "metadataKey"), not "metadataKeys" as 1.0
const auto decryptedMetadataKeyBase64 = decryptDataWithPrivateKey(metadataKeyFromJson);
const auto decryptedMetadataKeyBase64 = decryptDataWithPrivateKey(metadataKeyFromJson, _account->e2e()->certificateSha256Fingerprint());
if (!decryptedMetadataKeyBase64.isEmpty()) {
// fromBase64() multiple times just to stick with the old wrong way
_metadataKeyForDecryption = QByteArray::fromBase64(QByteArray::fromBase64(decryptedMetadataKeyBase64));
Expand All @@ -302,7 +303,7 @@
if (!lastMetadataKeyFromJson.isEmpty()) {
const auto lastMetadataKeyValueFromJson = metadataKeys.value(lastMetadataKeyFromJson).toString().toLocal8Bit();
if (!lastMetadataKeyValueFromJson.isEmpty()) {
const auto lastMetadataKeyValueFromJsonBase64 = decryptDataWithPrivateKey(lastMetadataKeyValueFromJson);
const auto lastMetadataKeyValueFromJsonBase64 = decryptDataWithPrivateKey(lastMetadataKeyValueFromJson, _account->e2e()->certificateSha256Fingerprint());

Check warning on line 306 in src/libsync/foldermetadata.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Rename this identifier to be shorter or equal to 31 characters. 


See more on https://sonarcloud.io/project/issues?id=nextcloud_desktop&issues=AZziIQ22PhPIe6f1CX8h&open=AZziIQ22PhPIe6f1CX8h&pullRequest=9614
if (!lastMetadataKeyValueFromJsonBase64.isEmpty()) {
_metadataKeyForDecryption = QByteArray::fromBase64(QByteArray::fromBase64(lastMetadataKeyValueFromJsonBase64));
}
Expand Down Expand Up @@ -442,9 +443,10 @@
return {};
}

QByteArray FolderMetadata::decryptDataWithPrivateKey(const QByteArray &base64Data) const
QByteArray FolderMetadata::decryptDataWithPrivateKey(const QByteArray &base64Data,
const QByteArray &base64CertificateSha256Hash) const
{
const auto decryptBase64Result = EncryptionHelper::decryptStringAsymmetric(_account->e2e()->getCertificateInformation(), _account->e2e()->paddingMode(), *_account->e2e(), base64Data);
const auto decryptBase64Result = EncryptionHelper::decryptStringAsymmetric(_account->e2e()->getCertificateInformationByFingerprint(base64CertificateSha256Hash), _account->e2e()->paddingMode(), *_account->e2e(), base64Data);
if (!decryptBase64Result) {
qCWarning(lcCseMetadata()) << "ERROR. Could not decrypt the metadata key";
_account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError);
Expand Down Expand Up @@ -580,7 +582,7 @@
emitSetupComplete();
}

QByteArray FolderMetadata::encryptedMetadata()

Check failure on line 585 in src/libsync/foldermetadata.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 33 to the 25 allowed. 


See more on https://sonarcloud.io/project/issues?id=nextcloud_desktop&issues=AZziIQ22PhPIe6f1CX8i&open=AZziIQ22PhPIe6f1CX8i&pullRequest=9614
{
Q_ASSERT(_isMetadataValid);
if (!_isMetadataValid) {
Expand Down Expand Up @@ -654,7 +656,12 @@
QJsonArray folderUsers;
if (_isRootEncryptedFolder) {
for (auto it = _folderUsers.constBegin(), end = _folderUsers.constEnd(); it != end; ++it) {
const auto folderUser = it.value();
auto folderUser = it.value();

if (folderUser.userId == _account->davUser()) {
folderUser.certificatePem = _account->e2e()->getCertificate().toPem();
updateUsersEncryptedMetadataKey();
}

const QJsonObject folderUserJson{{usersUserIdKey, folderUser.userId},
{usersCertificateKey, QJsonValue::fromVariant(folderUser.certificatePem)},
Expand Down Expand Up @@ -794,7 +801,7 @@
if (userParsedId == _account->davUser()) {
const auto fileDropEntryUser = UserWithFileDropEntryAccess{
userParsedId,
QByteArray::fromBase64(decryptDataWithPrivateKey(userParsed.value(usersEncryptedFiledropKey).toByteArray()))
QByteArray::fromBase64(decryptDataWithPrivateKey(userParsed.value(usersEncryptedFiledropKey).toByteArray(), _account->e2e()->certificateSha256Fingerprint()))
};
if (!fileDropEntryUser.isValid()) {
qCWarning(lcCseMetadata()) << "Could not parse filedrop data. encryptedFiledropKey decryption failed";
Expand Down
5 changes: 4 additions & 1 deletion src/libsync/foldermetadata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
#pragma once
/*
* SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
Expand Down Expand Up @@ -122,6 +122,8 @@
// removes a user from this folder and removes and generates a new metadata key
[[nodiscard]] bool removeUser(const QString &userId);

[[nodiscard]] bool updateUser(const QString &userId, const QSslCertificate &certificate, CertificateType certificateType);

[[nodiscard]] const QByteArray metadataKeyForEncryption() const;
[[nodiscard]] const QByteArray metadataKeyForDecryption() const;
[[nodiscard]] const QSet<QByteArray> &keyChecksums() const;
Expand Down Expand Up @@ -151,7 +153,8 @@

[[nodiscard]] QByteArray encryptDataWithPublicKey(const QByteArray &data,
const CertificateInformation &shareUserCertificate) const;
[[nodiscard]] QByteArray decryptDataWithPrivateKey(const QByteArray &data) const;
[[nodiscard]] QByteArray decryptDataWithPrivateKey(const QByteArray &data,
const QByteArray &base64CertificateSha256Hash) const;

[[nodiscard]] QByteArray encryptJsonObject(const QByteArray& obj, const QByteArray pass) const;
[[nodiscard]] QByteArray decryptJsonObject(const QByteArray& encryptedJsonBlob, const QByteArray& pass) const;
Expand Down
6 changes: 3 additions & 3 deletions test/testclientsideencryptionv2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ private slots:
const auto encryptedMetadataKey = QByteArray::fromBase64(folderUserObject.value("encryptedMetadataKey").toString().toUtf8());

if (!encryptedMetadataKey.isEmpty()) {
const auto decryptedMetadataKey = metadata->decryptDataWithPrivateKey(encryptedMetadataKey);
const auto decryptedMetadataKey = metadata->decryptDataWithPrivateKey(encryptedMetadataKey, _account->e2e()->certificateSha256Fingerprint());
if (decryptedMetadataKey.isEmpty()) {
break;
}
Expand Down Expand Up @@ -269,7 +269,7 @@ private slots:
const auto encryptedMetadataKey = QByteArray::fromBase64(folderUserObject.value("encryptedMetadataKey").toString().toUtf8());

if (!encryptedMetadataKey.isEmpty()) {
const auto decryptedMetadataKey = metadata->decryptDataWithPrivateKey(encryptedMetadataKey);
const auto decryptedMetadataKey = metadata->decryptDataWithPrivateKey(encryptedMetadataKey, _account->e2e()->certificateSha256Fingerprint());
if (decryptedMetadataKey.isEmpty()) {
break;
}
Expand Down Expand Up @@ -370,7 +370,7 @@ private slots:
const auto encryptedMetadataKey = QByteArray::fromBase64(folderUserObject.value("encryptedMetadataKey").toString().toUtf8());

if (!encryptedMetadataKey.isEmpty()) {
const auto decryptedMetadataKey = metadata->decryptDataWithPrivateKey(encryptedMetadataKey);
const auto decryptedMetadataKey = metadata->decryptDataWithPrivateKey(encryptedMetadataKey, _account->e2e()->certificateSha256Fingerprint());
if (decryptedMetadataKey.isEmpty()) {
break;
}
Expand Down
Loading