Skip to content

Commit

Permalink
Bitwarden and 1PUX importer improvements
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@droidmonkey
droidmonkey committed Apr 29, 2024
1 parent 2dfc0e5 commit d14821f
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 21 deletions.
4 changes: 4 additions & 0 deletions share/translations/keepassxc_en.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8466,6 +8466,10 @@ This option is deprecated, use --set-key-file instead.</source>
<source>Cannot remove file key: The database does not have a file key.</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Unsupported KDF type, cannot decrypt json file</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>QtIOCompressor</name>
Expand Down
73 changes: 57 additions & 16 deletions src/format/BitwardenReader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "core/Totp.h"
#include "crypto/CryptoHash.h"
#include "crypto/SymmetricCipher.h"
#include "crypto/kdf/Argon2Kdf.h"

#include <botan/kdf.h>
#include <botan/pwdhash.h>
Expand All @@ -36,6 +37,7 @@
#include <QJsonParseError>
#include <QMap>
#include <QScopedPointer>
#include <QUrl>

namespace
{
Expand All @@ -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> entry(new Entry());
Expand All @@ -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)
Expand Down Expand Up @@ -160,14 +176,20 @@ namespace

void writeVaultToDatabase(const QJsonObject& vault, QSharedPointer<Database> 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<QString, Group*> 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());
Expand Down Expand Up @@ -232,24 +254,43 @@ QSharedPointer<Database> 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<uint8_t*>(key.data()),
key.size(),
password.toUtf8().data(),
password.toUtf8().size(),
reinterpret_cast<uint8_t*>(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<uint8_t*>(key.data()),
key.size(),
password.toUtf8().data(),
password.toUtf8().size(),
reinterpret_cast<uint8_t*>(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<const uint8_t*>(key.data()), key.size(), "", "mac");
auto stretched_mac = hkdf->derive_key(32, reinterpret_cast<const uint8_t*>(key.data()), key.size(), "", "mac");
auto mac = QByteArray(reinterpret_cast<const char*>(stretched_mac.data()), stretched_mac.size());

// Stretch the Master Key
auto stretched_key = kdf->derive_key(32, reinterpret_cast<const uint8_t*>(key.data()), key.size(), "", "enc");
auto stretched_key = hkdf->derive_key(32, reinterpret_cast<const uint8_t*>(key.data()), key.size(), "", "enc");
key = QByteArray(reinterpret_cast<const char*>(stretched_key.data()), stretched_key.size());

// Validate the encryption key
Expand Down
16 changes: 11 additions & 5 deletions src/format/OPUXReader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
12 changes: 12 additions & 0 deletions tests/TestImports.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"));

Expand All @@ -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);
}
11 changes: 11 additions & 0 deletions tests/data/bitwarden_encrypted_argon2id_export.json
View file
Original file line number Diff line number Diff line change
@@ -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="
}

0 comments on commit d14821f

Please sign in to comment.