Skip to content

Commit 6d98e73

Browse files
mgallienmgallien
authored
Merge pull request #9614 from nextcloud/backport/9514/stable-4.0
[stable-4.0] Bugfix/e2ee fixes for hardware certificate end to end encryption
2 parents 2868893 + bbb33aa commit 6d98e73

File tree

5 files changed

+64
-55
lines changed

5 files changed

+64
-55
lines changed

‎src/libsync/clientsideencryption.cpp‎

Lines changed: 40 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ QDebug operator<<(QDebug out, const std::string& str)
6262
}
6363

6464
using namespace QKeychain;
65+
using namespace Qt::StringLiterals;
6566

6667
namespace OCC
6768
{
@@ -590,9 +591,9 @@ std::optional<QByteArray> encryptStringAsymmetric(const CertificateInformation &
590591
}
591592

592593
if (encryptionEngine.useTokenBasedEncryption()) {
593-
qCDebug(lcCseEncryption()) << "use certificate on hardware token";
594+
qCDebug(lcCseDecryption()) << "use certificate on hardware token" << selectedCertificate.sha256Fingerprint();
594595
} else {
595-
qCDebug(lcCseEncryption()) << "use certificate on software storage";
596+
qCDebug(lcCseDecryption()) << "use certificate on software storage" << selectedCertificate.sha256Fingerprint();
596597
}
597598

598599
auto encryptedBase64Result = QByteArray{};
@@ -639,9 +640,9 @@ std::optional<QByteArray> decryptStringAsymmetric(const CertificateInformation &
639640
}
640641

641642
if (encryptionEngine.useTokenBasedEncryption()) {
642-
qCDebug(lcCseDecryption()) << "use certificate on hardware token";
643+
qCDebug(lcCseDecryption()) << "use certificate on hardware token" << selectedCertificate.sha256Fingerprint();
643644
} else {
644-
qCDebug(lcCseDecryption()) << "use certificate on software storage";
645+
qCDebug(lcCseDecryption()) << "use certificate on software storage" << selectedCertificate.sha256Fingerprint();
645646
}
646647

647648
auto decryptBase64Result = QByteArray{};
@@ -1034,11 +1035,7 @@ bool ClientSideEncryption::userCertificateNeedsMigration() const
10341035

10351036
QByteArray ClientSideEncryption::certificateSha256Fingerprint() const
10361037
{
1037-
if (useTokenBasedEncryption()) {
1038-
return _encryptionCertificate.sha256Fingerprint();
1039-
}
1040-
1041-
return {};
1038+
return _encryptionCertificate.sha256Fingerprint();
10421039
}
10431040

10441041
void ClientSideEncryption::setAccount(const AccountPtr &account)
@@ -1275,32 +1272,34 @@ void ClientSideEncryption::initializeHardwareTokenEncryption(QWidget *settingsDi
12751272
return;
12761273
}
12771274

1278-
qCDebug(lcCse) << "checking the type of the key associated to the certificate";
1279-
qCDebug(lcCse) << "key type" << Qt::hex << PKCS11_get_key_type(certificateKey);
1275+
qCDebug(lcCse) << "checking the type of the key associated to the certificate" << sslCertificate.digest(QCryptographicHash::Sha256).toBase64();
1276+
qCDebug(lcCse) << "key type" << Qt::hex << PKCS11_get_key_type(certificateKey) << certificateKey;
1277+
1278+
auto newCertificateInformation = CertificateInformation{currentCertificate, std::move(sslCertificate)};
1279+
1280+
if (newCertificateInformation.isSelfSigned()) {
1281+
qCDebug(lcCse()) << "newly found certificate is self signed: goint to ignore it";
1282+
continue;
1283+
}
12801284

1281-
_otherCertificates.emplace_back(certificateKey, std::move(sslCertificate));
1285+
const auto &sslErrors = newCertificateInformation.verify();
1286+
for (const auto &sslError : sslErrors) {
1287+
qCInfo(lcCse()) << "certificate validation error" << sslError;
1288+
}
1289+
1290+
_otherCertificates.push_back(std::move(newCertificateInformation));
12821291
}
12831292
}
12841293

12851294
for (const auto &oneCertificateInformation : _otherCertificates) {
1286-
if (oneCertificateInformation.isSelfSigned()) {
1287-
qCDebug(lcCse()) << "newly found certificate is self signed: goint to ignore it";
1288-
continue;
1289-
}
1290-
12911295
if (!_usbTokenInformation.sha256Fingerprint().isEmpty() && oneCertificateInformation.sha256Fingerprint() != _usbTokenInformation.sha256Fingerprint()) {
12921296
qCDebug(lcCse()) << "skipping certificate from" << "with fingerprint" << oneCertificateInformation.sha256Fingerprint() << "different from" << _usbTokenInformation.sha256Fingerprint();
12931297
continue;
12941298
}
12951299

1296-
const auto &sslErrors = oneCertificateInformation.verify();
1297-
for (const auto &sslError : sslErrors) {
1298-
qCInfo(lcCse()) << "certificate validation error" << sslError;
1299-
}
1300-
13011300
setEncryptionCertificate(oneCertificateInformation);
13021301

1303-
if (canEncrypt() && !checkEncryptionIsWorking()) {
1302+
if (oneCertificateInformation.canEncrypt() && !checkEncryptionIsWorking(oneCertificateInformation)) {
13041303
qCWarning(lcCse()) << "encryption is not properly setup";
13051304

13061305
failedToInitialize();
@@ -1362,26 +1361,26 @@ void ClientSideEncryption::fetchPublicKeyFromKeyChain()
13621361
job->start();
13631362
}
13641363

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

1370-
auto encryptedData = EncryptionHelper::encryptStringAsymmetric(getCertificateInformation(), paddingMode(), *this, data);
1369+
auto encryptedData = EncryptionHelper::encryptStringAsymmetric(currentCertificate, paddingMode(), *this, data);
13711370
if (!encryptedData) {
13721371
qCWarning(lcCse()) << "encryption error";
13731372
return false;
13741373
}
13751374

1376-
qCDebug(lcCse) << "encryption is working with" << getCertificateInformation().sha256Fingerprint();
1375+
qCDebug(lcCse) << "encryption is working with" << currentCertificate.sha256Fingerprint();
13771376

1378-
const auto decryptionResult = EncryptionHelper::decryptStringAsymmetric(getCertificateInformation(), paddingMode(), *this, *encryptedData);
1377+
const auto decryptionResult = EncryptionHelper::decryptStringAsymmetric(currentCertificate, paddingMode(), *this, *encryptedData);
13791378
if (!decryptionResult) {
13801379
qCWarning(lcCse()) << "encryption error";
13811380
return false;
13821381
}
13831382

1384-
qCDebug(lcCse) << "decryption is working with" << getCertificateInformation().sha256Fingerprint();
1383+
qCDebug(lcCse) << "decryption is working with" << currentCertificate.sha256Fingerprint();
13851384

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

@@ -1390,7 +1389,7 @@ bool ClientSideEncryption::checkEncryptionIsWorking()
13901389
return false;
13911390
}
13921391

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

13951394
return true;
13961395
}
@@ -2317,7 +2316,7 @@ void ClientSideEncryption::decryptPrivateKey(const QByteArray &key) {
23172316
}
23182317
}
23192318

2320-
if (!getPrivateKey().isNull() && checkEncryptionIsWorking()) {
2319+
if (!getPrivateKey().isNull() && checkEncryptionIsWorking(_encryptionCertificate)) {
23212320
writePrivateKey();
23222321
writeCertificate();
23232322
writeMnemonic([] () {});
@@ -3033,9 +3032,9 @@ CertificateInformation::CertificateInformation()
30333032
checkEncryptionCertificate();
30343033
}
30353034

3036-
CertificateInformation::CertificateInformation(PKCS11_KEY *hardwarePrivateKey,
3035+
CertificateInformation::CertificateInformation(PKCS11_CERT *hardwareCertificate,
30373036
QSslCertificate &&certificate)
3038-
: _hardwarePrivateKey{hardwarePrivateKey}
3037+
: _hardwareCertificate{hardwareCertificate}
30393038
, _certificate{std::move(certificate)}
30403039
, _certificateType{CertificateType::HardwareCertificate}
30413040
{
@@ -3045,7 +3044,7 @@ CertificateInformation::CertificateInformation(PKCS11_KEY *hardwarePrivateKey,
30453044
CertificateInformation::CertificateInformation(CertificateType certificateType,
30463045
const QByteArray &privateKey,
30473046
QSslCertificate &&certificate)
3048-
: _hardwarePrivateKey()
3047+
: _hardwareCertificate()
30493048
, _privateKeyData()
30503049
, _certificate(std::move(certificate))
30513050
, _certificateType{certificateType}
@@ -3072,7 +3071,7 @@ bool CertificateInformation::operator==(const CertificateInformation &other) con
30723071

30733072
void CertificateInformation::clear()
30743073
{
3075-
_hardwarePrivateKey = nullptr;
3074+
_hardwareCertificate = nullptr;
30763075
_privateKeyData.clear();
30773076
_certificate.clear();
30783077
_certificateExpired = true;
@@ -3140,13 +3139,13 @@ PKey CertificateInformation::getEvpPublicKey() const
31403139

31413140
PKCS11_KEY *CertificateInformation::getPkcs11PrivateKey() const
31423141
{
3143-
return canDecrypt() ? _hardwarePrivateKey : nullptr;
3142+
return canDecrypt() && _hardwareCertificate ? PKCS11_find_key(_hardwareCertificate) : nullptr;
31443143
}
31453144

31463145
PKey CertificateInformation::getEvpPrivateKey() const
31473146
{
3148-
if (_hardwarePrivateKey) {
3149-
return PKey::readHardwarePrivateKey(_hardwarePrivateKey);
3147+
if (_hardwareCertificate) {
3148+
return PKey::readHardwarePrivateKey(PKCS11_find_key(_hardwareCertificate));
31503149
} else {
31513150
const auto privateKeyPem = _privateKeyData;
31523151
Q_ASSERT(!privateKeyPem.isEmpty());
@@ -3167,23 +3166,23 @@ const QSslCertificate &CertificateInformation::getCertificate() const
31673166

31683167
bool CertificateInformation::canEncrypt() const
31693168
{
3170-
return (_hardwarePrivateKey || !_certificate.isNull()) && !_certificateExpired && !_certificateNotYetValid && !_certificateRevoked && !_certificateInvalid;
3169+
return (_hardwareCertificate || !_certificate.isNull()) && !_certificateExpired && !_certificateNotYetValid && !_certificateRevoked && !_certificateInvalid;
31713170
}
31723171

31733172
bool CertificateInformation::canDecrypt() const
31743173
{
3175-
return _hardwarePrivateKey || !_privateKeyData.isEmpty();
3174+
return _hardwareCertificate || !_privateKeyData.isEmpty();
31763175
}
31773176

31783177
bool CertificateInformation::userCertificateNeedsMigration() const
31793178
{
3180-
return _hardwarePrivateKey &&
3179+
return _hardwareCertificate &&
31813180
(_certificateExpired || _certificateNotYetValid || _certificateRevoked || _certificateInvalid);
31823181
}
31833182

31843183
bool CertificateInformation::sensitiveDataRemaining() const
31853184
{
3186-
return _hardwarePrivateKey && !_privateKeyData.isEmpty() && !_certificate.isNull();
3185+
return _hardwareCertificate && !_privateKeyData.isEmpty() && !_certificate.isNull();
31873186
}
31883187

31893188
QByteArray CertificateInformation::sha256Fingerprint() const

‎src/libsync/clientsideencryption.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class CertificateInformation {
5555

5656
CertificateInformation();
5757

58-
explicit CertificateInformation(PKCS11_KEY *hardwarePrivateKey,
58+
explicit CertificateInformation(PKCS11_CERT *hardwareCertificate,
5959
QSslCertificate &&certificate);
6060

6161
explicit CertificateInformation(CertificateType certificateType,
@@ -99,7 +99,7 @@ class CertificateInformation {
9999

100100
void doNotCheckEncryptionCertificate();
101101

102-
PKCS11_KEY* _hardwarePrivateKey = nullptr;
102+
PKCS11_CERT* _hardwareCertificate = nullptr;
103103

104104
QByteArray _privateKeyData;
105105

@@ -391,7 +391,7 @@ private slots:
391391
[[nodiscard]] bool checkServerPublicKeyValidity(const QByteArray &serverPublicKeyString) const;
392392
[[nodiscard]] bool sensitiveDataRemaining() const;
393393

394-
[[nodiscard]] bool checkEncryptionIsWorking();
394+
[[nodiscard]] bool checkEncryptionIsWorking(const CertificateInformation &currentCertificate);
395395

396396
void failedToInitialize();
397397

‎src/libsync/foldermetadata.cpp‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,8 @@ void FolderMetadata::setupExistingMetadata(const QByteArray &metadata)
181181

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

@@ -279,7 +280,7 @@ void FolderMetadata::setupExistingMetadataLegacy(const QByteArray &metadata)
279280
const auto metadataKeyFromJson = fullMetaDataObj[metadataKeyKey].toString().toLocal8Bit();
280281
if (!metadataKeyFromJson.isEmpty()) {
281282
// parse version 1.1 and 1.2 (both must have a single "metadataKey"), not "metadataKeys" as 1.0
282-
const auto decryptedMetadataKeyBase64 = decryptDataWithPrivateKey(metadataKeyFromJson);
283+
const auto decryptedMetadataKeyBase64 = decryptDataWithPrivateKey(metadataKeyFromJson, _account->e2e()->certificateSha256Fingerprint());
283284
if (!decryptedMetadataKeyBase64.isEmpty()) {
284285
// fromBase64() multiple times just to stick with the old wrong way
285286
_metadataKeyForDecryption = QByteArray::fromBase64(QByteArray::fromBase64(decryptedMetadataKeyBase64));
@@ -302,7 +303,7 @@ void FolderMetadata::setupExistingMetadataLegacy(const QByteArray &metadata)
302303
if (!lastMetadataKeyFromJson.isEmpty()) {
303304
const auto lastMetadataKeyValueFromJson = metadataKeys.value(lastMetadataKeyFromJson).toString().toLocal8Bit();
304305
if (!lastMetadataKeyValueFromJson.isEmpty()) {
305-
const auto lastMetadataKeyValueFromJsonBase64 = decryptDataWithPrivateKey(lastMetadataKeyValueFromJson);
306+
const auto lastMetadataKeyValueFromJsonBase64 = decryptDataWithPrivateKey(lastMetadataKeyValueFromJson, _account->e2e()->certificateSha256Fingerprint());
306307
if (!lastMetadataKeyValueFromJsonBase64.isEmpty()) {
307308
_metadataKeyForDecryption = QByteArray::fromBase64(QByteArray::fromBase64(lastMetadataKeyValueFromJsonBase64));
308309
}
@@ -442,9 +443,10 @@ QByteArray FolderMetadata::encryptDataWithPublicKey(const QByteArray &binaryData
442443
return {};
443444
}
444445

445-
QByteArray FolderMetadata::decryptDataWithPrivateKey(const QByteArray &base64Data) const
446+
QByteArray FolderMetadata::decryptDataWithPrivateKey(const QByteArray &base64Data,
447+
const QByteArray &base64CertificateSha256Hash) const
446448
{
447-
const auto decryptBase64Result = EncryptionHelper::decryptStringAsymmetric(_account->e2e()->getCertificateInformation(), _account->e2e()->paddingMode(), *_account->e2e(), base64Data);
449+
const auto decryptBase64Result = EncryptionHelper::decryptStringAsymmetric(_account->e2e()->getCertificateInformationByFingerprint(base64CertificateSha256Hash), _account->e2e()->paddingMode(), *_account->e2e(), base64Data);
448450
if (!decryptBase64Result) {
449451
qCWarning(lcCseMetadata()) << "ERROR. Could not decrypt the metadata key";
450452
_account->reportClientStatus(OCC::ClientStatusReportingStatus::E2EeError_GeneralError);
@@ -654,7 +656,12 @@ QByteArray FolderMetadata::encryptedMetadata()
654656
QJsonArray folderUsers;
655657
if (_isRootEncryptedFolder) {
656658
for (auto it = _folderUsers.constBegin(), end = _folderUsers.constEnd(); it != end; ++it) {
657-
const auto folderUser = it.value();
659+
auto folderUser = it.value();
660+
661+
if (folderUser.userId == _account->davUser()) {
662+
folderUser.certificatePem = _account->e2e()->getCertificate().toPem();
663+
updateUsersEncryptedMetadataKey();
664+
}
658665

659666
const QJsonObject folderUserJson{{usersUserIdKey, folderUser.userId},
660667
{usersCertificateKey, QJsonValue::fromVariant(folderUser.certificatePem)},
@@ -794,7 +801,7 @@ bool FolderMetadata::parseFileDropPart(const QJsonDocument &doc)
794801
if (userParsedId == _account->davUser()) {
795802
const auto fileDropEntryUser = UserWithFileDropEntryAccess{
796803
userParsedId,
797-
QByteArray::fromBase64(decryptDataWithPrivateKey(userParsed.value(usersEncryptedFiledropKey).toByteArray()))
804+
QByteArray::fromBase64(decryptDataWithPrivateKey(userParsed.value(usersEncryptedFiledropKey).toByteArray(), _account->e2e()->certificateSha256Fingerprint()))
798805
};
799806
if (!fileDropEntryUser.isValid()) {
800807
qCWarning(lcCseMetadata()) << "Could not parse filedrop data. encryptedFiledropKey decryption failed";

‎src/libsync/foldermetadata.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ class OWNCLOUDSYNC_EXPORT FolderMetadata : public QObject
122122
// removes a user from this folder and removes and generates a new metadata key
123123
[[nodiscard]] bool removeUser(const QString &userId);
124124

125+
[[nodiscard]] bool updateUser(const QString &userId, const QSslCertificate &certificate, CertificateType certificateType);
126+
125127
[[nodiscard]] const QByteArray metadataKeyForEncryption() const;
126128
[[nodiscard]] const QByteArray metadataKeyForDecryption() const;
127129
[[nodiscard]] const QSet<QByteArray> &keyChecksums() const;
@@ -151,7 +153,8 @@ public slots:
151153

152154
[[nodiscard]] QByteArray encryptDataWithPublicKey(const QByteArray &data,
153155
const CertificateInformation &shareUserCertificate) const;
154-
[[nodiscard]] QByteArray decryptDataWithPrivateKey(const QByteArray &data) const;
156+
[[nodiscard]] QByteArray decryptDataWithPrivateKey(const QByteArray &data,
157+
const QByteArray &base64CertificateSha256Hash) const;
155158

156159
[[nodiscard]] QByteArray encryptJsonObject(const QByteArray& obj, const QByteArray pass) const;
157160
[[nodiscard]] QByteArray decryptJsonObject(const QByteArray& encryptedJsonBlob, const QByteArray& pass) const;

‎test/testclientsideencryptionv2.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ private slots:
130130
const auto encryptedMetadataKey = QByteArray::fromBase64(folderUserObject.value("encryptedMetadataKey").toString().toUtf8());
131131

132132
if (!encryptedMetadataKey.isEmpty()) {
133-
const auto decryptedMetadataKey = metadata->decryptDataWithPrivateKey(encryptedMetadataKey);
133+
const auto decryptedMetadataKey = metadata->decryptDataWithPrivateKey(encryptedMetadataKey, _account->e2e()->certificateSha256Fingerprint());
134134
if (decryptedMetadataKey.isEmpty()) {
135135
break;
136136
}
@@ -269,7 +269,7 @@ private slots:
269269
const auto encryptedMetadataKey = QByteArray::fromBase64(folderUserObject.value("encryptedMetadataKey").toString().toUtf8());
270270

271271
if (!encryptedMetadataKey.isEmpty()) {
272-
const auto decryptedMetadataKey = metadata->decryptDataWithPrivateKey(encryptedMetadataKey);
272+
const auto decryptedMetadataKey = metadata->decryptDataWithPrivateKey(encryptedMetadataKey, _account->e2e()->certificateSha256Fingerprint());
273273
if (decryptedMetadataKey.isEmpty()) {
274274
break;
275275
}
@@ -370,7 +370,7 @@ private slots:
370370
const auto encryptedMetadataKey = QByteArray::fromBase64(folderUserObject.value("encryptedMetadataKey").toString().toUtf8());
371371

372372
if (!encryptedMetadataKey.isEmpty()) {
373-
const auto decryptedMetadataKey = metadata->decryptDataWithPrivateKey(encryptedMetadataKey);
373+
const auto decryptedMetadataKey = metadata->decryptDataWithPrivateKey(encryptedMetadataKey, _account->e2e()->certificateSha256Fingerprint());
374374
if (decryptedMetadataKey.isEmpty()) {
375375
break;
376376
}

0 commit comments

Comments
 (0)