From 189688338201fa8b5c59083694bac2ec2ec432db Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Mon, 25 Mar 2024 19:52:21 -0400 Subject: [PATCH] Bitwarden and 1PUX importer improvements * Fixes #10400 - Support TOTP entries with bare secrets instead of otpauth urls for Bitwarden. Vice-versa for 1PUX. - Support Bitwarden Argon2id encryption scheme * Fixes #10380 - Support Bitwarden organization collections --- share/translations/keepassxc_en.ts | 4 + src/format/BitwardenReader.cpp | 73 +++++++++++++++---- src/format/OPUXReader.cpp | 16 ++-- tests/TestImports.cpp | 12 +++ .../bitwarden_encrypted_argon2id_export.json | 11 +++ 5 files changed, 95 insertions(+), 21 deletions(-) create mode 100644 tests/data/bitwarden_encrypted_argon2id_export.json diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 4871bc3106..bdc31f13e5 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -8678,6 +8678,10 @@ This option is deprecated, use --set-key-file instead. Shortcuts + + Unsupported KDF type, cannot decrypt json file + + QtIOCompressor diff --git a/src/format/BitwardenReader.cpp b/src/format/BitwardenReader.cpp index 8f7a926fa1..8f9d86b8f3 100644 --- a/src/format/BitwardenReader.cpp +++ b/src/format/BitwardenReader.cpp @@ -25,6 +25,7 @@ #include "core/Totp.h" #include "crypto/CryptoHash.h" #include "crypto/SymmetricCipher.h" +#include "crypto/kdf/Argon2Kdf.h" #include #include @@ -36,6 +37,7 @@ #include #include #include +#include namespace { @@ -44,6 +46,13 @@ namespace // Create the item map and extract the folder id const auto itemMap = item.toVariantMap(); folderId = itemMap.value("folderId").toString(); + if (folderId.isEmpty()) { + // Bitwarden organization vaults use collectionId instead of folderId + auto collectionIds = itemMap.value("collectionIds").toStringList(); + if (!collectionIds.empty()) { + folderId = collectionIds.first(); + } + } // Create entry and assign basic values QScopedPointer entry(new Entry()); @@ -61,8 +70,15 @@ namespace entry->setUsername(loginMap.value("username").toString()); entry->setPassword(loginMap.value("password").toString()); if (loginMap.contains("totp")) { - // Bitwarden stores TOTP as otpauth string - entry->setTotp(Totp::parseSettings(loginMap.value("totp").toString())); + auto totp = loginMap.value("totp").toString(); + if (!totp.startsWith("otpauth://")) { + QUrl url(QString("otpauth://totp/%1:%2?secret=%3") + .arg(QString(QUrl::toPercentEncoding(entry->title())), + QString(QUrl::toPercentEncoding(entry->username())), + QString(QUrl::toPercentEncoding(totp)))); + totp = url.toString(QUrl::FullyEncoded); + } + entry->setTotp(Totp::parseSettings(totp)); } // Set the entry url(s) @@ -160,14 +176,20 @@ namespace void writeVaultToDatabase(const QJsonObject& vault, QSharedPointer db) { - if (!vault.contains("folders") || !vault.contains("items")) { + auto folderField = QString("folders"); + if (!vault.contains(folderField)) { + // Handle Bitwarden organization vaults + folderField = "collections"; + } + + if (!vault.contains(folderField) || !vault.contains("items")) { // Early out if the vault is missing critical items return; } // Create groups from folders and store a temporary map of id -> uuid QMap folderMap; - for (const auto& folder : vault.value("folders").toArray()) { + for (const auto& folder : vault.value(folderField).toArray()) { auto group = new Group(); group->setUuid(QUuid::createUuid()); group->setName(folder.toObject().value("name").toString()); @@ -232,24 +254,43 @@ QSharedPointer BitwardenReader::convert(const QString& path, const QSt QByteArray key(32, '\0'); auto salt = json.value("salt").toString().toUtf8(); - - auto pwd_fam = Botan::PasswordHashFamily::create_or_throw("PBKDF2(SHA-256)"); - auto kdf = Botan::KDF::create_or_throw("HKDF-Expand(SHA-256)"); + auto kdfType = json.value("kdfType").toInt(); // Derive the Master Key - auto pwd_hash = pwd_fam->from_params(json.value("kdfIterations").toInt()); - pwd_hash->derive_key(reinterpret_cast(key.data()), - key.size(), - password.toUtf8().data(), - password.toUtf8().size(), - reinterpret_cast(salt.data()), - salt.size()); + if (kdfType == 0) { + auto pwd_fam = Botan::PasswordHashFamily::create_or_throw("PBKDF2(SHA-256)"); + auto pwd_hash = pwd_fam->from_params(json.value("kdfIterations").toInt()); + pwd_hash->derive_key(reinterpret_cast(key.data()), + key.size(), + password.toUtf8().data(), + password.toUtf8().size(), + reinterpret_cast(salt.data()), + salt.size()); + } else if (kdfType == 1) { + // Bitwarden hashes the salt for Argon2 for some reason + CryptoHash saltHash(CryptoHash::Sha256); + saltHash.addData(salt); + salt = saltHash.result(); + + Argon2Kdf argon2(Argon2Kdf::Type::Argon2id); + argon2.setSeed(salt); + argon2.setRounds(json.value("kdfIterations").toInt()); + argon2.setMemory(json.value("kdfMemory").toInt() * 1024); + argon2.setParallelism(json.value("kdfParallelism").toInt()); + argon2.transform(password.toUtf8(), key); + } else { + m_error = buildError(QObject::tr("Unsupported KDF type, cannot decrypt json file")); + return {}; + } + + auto hkdf = Botan::KDF::create_or_throw("HKDF-Expand(SHA-256)"); + // Derive the MAC Key - auto stretched_mac = kdf->derive_key(32, reinterpret_cast(key.data()), key.size(), "", "mac"); + auto stretched_mac = hkdf->derive_key(32, reinterpret_cast(key.data()), key.size(), "", "mac"); auto mac = QByteArray(reinterpret_cast(stretched_mac.data()), stretched_mac.size()); // Stretch the Master Key - auto stretched_key = kdf->derive_key(32, reinterpret_cast(key.data()), key.size(), "", "enc"); + auto stretched_key = hkdf->derive_key(32, reinterpret_cast(key.data()), key.size(), "", "enc"); key = QByteArray(reinterpret_cast(stretched_key.data()), stretched_key.size()); // Validate the encryption key diff --git a/src/format/OPUXReader.cpp b/src/format/OPUXReader.cpp index d52aa4640c..ee5869b69b 100644 --- a/src/format/OPUXReader.cpp +++ b/src/format/OPUXReader.cpp @@ -126,9 +126,15 @@ namespace const auto valueMap = fieldMap.value("value").toMap(); const auto key = valueMap.firstKey(); if (key == "totp") { - // Build otpauth url - QUrl otpurl(QString("otpauth://totp/%1:%2?secret=%3") - .arg(entry->title(), entry->username(), valueMap.value(key).toString())); + auto totp = valueMap.value(key).toString(); + if (!totp.startsWith("otpauth://")) { + // Build otpauth url + QUrl url(QString("otpauth://totp/%1:%2?secret=%3") + .arg(QString(QUrl::toPercentEncoding(entry->title())), + QString(QUrl::toPercentEncoding(entry->username())), + QString(QUrl::toPercentEncoding(totp)))); + totp = url.toString(QUrl::FullyEncoded); + } if (entry->hasTotp()) { // Store multiple TOTP definitions as additional otp attributes @@ -138,10 +144,10 @@ namespace while (attributes.contains(name)) { name = QString("otp_%1").arg(++i); } - entry->attributes()->set(name, otpurl.toEncoded(), true); + entry->attributes()->set(name, totp, true); } else { // First otp value encountered gets formal storage - entry->setTotp(Totp::parseSettings(otpurl.toEncoded())); + entry->setTotp(Totp::parseSettings(totp)); } } else if (key == "file") { // Add a file to the entry attachments diff --git a/tests/TestImports.cpp b/tests/TestImports.cpp index a809cc3edc..84ef26ccee 100644 --- a/tests/TestImports.cpp +++ b/tests/TestImports.cpp @@ -255,6 +255,8 @@ void TestImports::testBitwarden() void TestImports::testBitwardenEncrypted() { // We already tested the parser so just test that decryption works properly + + // First test PBKDF2 password stretching (KDF Type 0) auto bitwardenPath = QStringLiteral("%1/%2").arg(KEEPASSX_TEST_DATA_DIR, QStringLiteral("/bitwarden_encrypted_export.json")); @@ -264,4 +266,14 @@ void TestImports::testBitwardenEncrypted() QFAIL(qPrintable(reader.errorString())); } QVERIFY(db); + + // Now test Argon2id password stretching (KDF Type 1) + bitwardenPath = QStringLiteral("%1/%2").arg(KEEPASSX_TEST_DATA_DIR, + QStringLiteral("/bitwarden_encrypted_argon2id_export.json")); + + db = reader.convert(bitwardenPath, "a"); + if (reader.hasError()) { + QFAIL(qPrintable(reader.errorString())); + } + QVERIFY(db); } diff --git a/tests/data/bitwarden_encrypted_argon2id_export.json b/tests/data/bitwarden_encrypted_argon2id_export.json new file mode 100644 index 0000000000..67dea8dcef --- /dev/null +++ b/tests/data/bitwarden_encrypted_argon2id_export.json @@ -0,0 +1,11 @@ +{ + "encrypted": true, + "passwordProtected": true, + "salt": "qQAdNzVGDjmIWIz3CLkMOg==", + "kdfType": 1, + "kdfIterations": 3, + "kdfMemory": 64, + "kdfParallelism": 4, + "encKeyValidation_DO_NOT_EDIT": "2.XohQSvVARSkNaL2eQ5cqSg==|EMUESMPFpYH3F7yww2SSfFPLTcerACPz3G29Q3LoQ2iJw33nutDg6lpDhCJ9bKXe|Qxf01Q7YB5kv4lmW58XOOuOWrPfUqocWKfIzRBDREWA=", + "data": "2.eQO+T8wrtxVlTPgQfZtWzg==|PHPF2TwVT4shA+F2LGd+Tz22LFpv0QmGW3PQUdeGxS0mdy5pIT/BL7bMw5+N/njOq/Y0oFru+xNklfVrAUxb4Bl4dgH7XDmW4iFMlcum7/qOThf7hHI2lhyTIzBBlma/dcADMjJcp2R4SQ3gVsZ2FrX5xgZgKfLgE1hL2kiwYrFOJ/mXUKXSCjgyHC4eSNdDOle9OzbcXiq4MaJHzdEAjEKaVqO5KFBDsZ2xCPOe/47wDJ5sxZp2I8/1zaZ56x4bYBp11p5TXDpQ46pLXxElsm3sxaxEzziZum7Boj9f1b/7TpRRzpZuP80OgjeW9CAZJ6UqR7gdtb7Bn00R+GlUyAFzHD9y6LkaF9HGBLlq0kz/lj6OkgePerXO/rJM2kuCGd/YZ6hQvXk75EHpZ3s/5/no1eUuMyhN/cH0SNtFlEPcsWL6wXzkKTeSlzVsq+fzKmlv9oprDjGwVuj0EgIyKQcmMavmE7aob5Ybz+ufkbtMfEVKAWVGo1ljvqCJoMXOehDUnub+p60THafAzlGR/G5/QWm+M1QRnI0pLJL/4aZIj3bk+oBRmfpDC5qdsXkyqmhEzkBwVgD+0zwWJ4UJaxWxgfewUoLhUvMqqjd59PORUWhqCG1GOmEMgCWJN9yI0G7/H6GhemWNQ7y4Kdeb+SS5QG0YQk6OVrvFJ6Ky0Tgaopp/h5CxPDrfJFgc/HdPPC3arEisyZkkO6uq+lVewiNsvFOfTakVeqxtjC1bKHhRQ33PG7UN9qWGHYwvc/NKMwJswJCgcWMwbiGN8VKPjx5/WzO7nPdCQByqCU/MMUcZUGVlDIYxemCPcvgS0cgP9rKc1ie/w6DAgnFmXhdhzKOPvYp7/OUE70+t/PzE8XrVd/ChOHkN3WQEVWFJzSdj0fMzZ8F60yrD0wxulI6u6RWWEHLw5m6zA2AreplkdaByuzRmuP2sHcsNmZ/ShySPivgelq25BxwCU2NThUnohvnTRXW5yB47v7uPn3ZPG2sR6Zt5q9ibrwrcd4N6AcitgiQYVvZmmIuKzzNJ98oDfpDxdPM/fTjlIMHCdM6WyX3C/bcADP6ivZkFRsyWEcnJyBOkuSp8j7MiEDQVgcXjGLU8MdtDL0c8qbPnAt66AADmvsAumN/e5mpXeS3KEVUSeNa/H+oWmy/ExXj17d/6fcDTym3dZ6Vc4CcAkVDlHlsVeFr5fUnSdxKgl/BPBX+gbD6rm7l4MGvjTdLnW9ZIcDwX+Dh4GthSVhrKwWBO//9q6Y2pguO5s1F+MPc/5CY9XFFaOdildr21GFeZh1ClxWARW0r2SdlH19GIz0gTurL4lqAsSwC5i8pxXweAOZLd|LuLnG7meb0T0i6uHmMg6p3Uj8rq4LEI43k6KQaaDnVw=" +} \ No newline at end of file