Skip to content

Commit

Permalink
Improve OPVault handling and replace test opvault
Browse files Browse the repository at this point in the history 
* Fix various bugs in opvault parsing to include: TOTP parsing, date handling, naming convention, attachments, and multiple url's.

* Remove category groups that don't have any entries.

* Simplify tests by focusing on the resulting database instead of the parsing mechanics.

* Remove proprietary "freddy" opvault in favor of self-made "keepassxc" opvault.

* Fix #4069, select opvault file on macOS
  • Loading branch information
@droidmonkey
droidmonkey committed May 14, 2020
1 parent 5602095 commit 612f8d2
Show file tree
Hide file tree
Showing 38 changed files with 180 additions and 941 deletions.
16 changes: 12 additions & 4 deletions src/format/OpVaultReader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,21 @@ Database* OpVaultReader::readDatabase(QDir& opdataDir, const QString& password)
return nullptr;
}

auto vaultName = opdataDir.dirName();

auto key = QSharedPointer<CompositeKey>::create();
key->addKey(QSharedPointer<PasswordKey>::create(password));

QScopedPointer<Database> db(new Database());
db->setKdf(KeePass2::uuidToKdf(KeePass2::KDF_ARGON2));
db->setCipher(KeePass2::CIPHER_AES256);
db->setKey(key, true, false);
db->metadata()->setName(opdataDir.dirName());
db->metadata()->setName(vaultName);

auto rootGroup = db->rootGroup();
rootGroup->setTimeInfo({});
rootGroup->setUpdateTimeinfo(false);
rootGroup->setName("OPVault Root Group");
rootGroup->setName(vaultName.remove(".opvault"));
rootGroup->setUuid(QUuid::createUuid());

populateCategoryGroups(rootGroup);
Expand Down Expand Up @@ -110,7 +112,6 @@ Database* OpVaultReader::readDatabase(QDir& opdataDir, const QString& password)
for (QChar ch : bandChars) {
QFile bandFile(defaultDir.filePath(bandPattern.arg(ch)));
if (!bandFile.exists()) {
qWarning() << "Skipping missing file \"" << bandFile.fileName() << "\"";
continue;
}
// https://support.1password.com/opvault-design/#band-files
Expand All @@ -137,13 +138,20 @@ Database* OpVaultReader::readDatabase(QDir& opdataDir, const QString& password)
continue;
}
// https://support.1password.com/opvault-design/#items
Entry* entry = processBandEntry(bandEnt, defaultDir, rootGroup);
auto entry = processBandEntry(bandEnt, defaultDir, rootGroup);
if (!entry) {
qWarning() << "Unable to process Band Entry " << uuid;
}
}
}

// Remove empty categories (groups)
for (auto group : rootGroup->children()) {
if (group->isEmpty()) {
delete group;
}
}

zeroKeys();
return db.take();
}
Expand Down
16 changes: 1 addition & 15 deletions src/format/OpVaultReaderAttachments.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,22 +125,8 @@ bool OpVaultReader::readAttachment(const QString& filePath,
return false;
}

if (!metadata.contains("contentsSize")) {
qWarning() << "Expected attachment metadata to contain \"contentsSize\" but nope: " << metadata;
return false;
} else if (!metadata["contentsSize"].isDouble()) {
qWarning() << "Expected attachment metadata to contain numeric \"contentsSize\" but nope: " << metadata;
return false;
}
int bytesLen = metadata["contentsSize"].toInt();
const QByteArray encData = file.readAll();
if (encData.size() < bytesLen) {
qCritical() << "Unable to read all of the attachment payload; wanted " << bytesLen << "but got"
<< encData.size();
return false;
}

OpData01 att01;
const QByteArray encData = file.readAll();
if (!att01.decode(encData, itemKey, itemHmacKey)) {
qCritical() << "Unable to decipher attachment payload: " << att01.errorString();
return false;
Expand Down
49 changes: 23 additions & 26 deletions src/format/OpVaultReaderBandEntry.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <QJsonArray>
#include <QJsonDocument>
#include <QJsonObject>
#include <QScopedPointer>
#include <QUuid>

bool OpVaultReader::decryptBandEntry(const QJsonObject& bandEntry,
Expand Down Expand Up @@ -112,9 +113,12 @@ Entry* OpVaultReader::processBandEntry(const QJsonObject& bandEntry, const QDir&
return nullptr;
}

const auto entry = new Entry();
QScopedPointer<Entry> entry(new Entry());

if (bandEntry.contains("category")) {
if (bandEntry.contains("trashed") && bandEntry["trashed"].toBool()) {
// Send this entry to the recycle bin
rootGroup->database()->recycleEntry(entry.data());
} else if (bandEntry.contains("category")) {
const QJsonValue& categoryValue = bandEntry["category"];
if (categoryValue.isString()) {
bool found = false;
Expand Down Expand Up @@ -162,8 +166,7 @@ Entry* OpVaultReader::processBandEntry(const QJsonObject& bandEntry, const QDir&
}
entry->setUuid(Tools::hexToUuid(uuid));

if (!fillAttributes(entry, bandEntry)) {
delete entry;
if (!fillAttributes(entry.data(), bandEntry)) {
return nullptr;
}

Expand All @@ -184,7 +187,7 @@ Entry* OpVaultReader::processBandEntry(const QJsonObject& bandEntry, const QDir&
entry->setPassword(data.value("password").toString());
}

for (const auto& fieldValue : data.value("fields").toArray()) {
for (const auto fieldValue : data.value("fields").toArray()) {
if (!fieldValue.isObject()) {
continue;
}
Expand All @@ -208,11 +211,11 @@ Entry* OpVaultReader::processBandEntry(const QJsonObject& bandEntry, const QDir&
}
const QJsonObject& section = sectionValue.toObject();

fillFromSection(entry, section);
fillFromSection(entry.data(), section);
}

fillAttachments(entry, attachmentDir, entryKey, entryHmacKey);
return entry;
fillAttachments(entry.data(), attachmentDir, entryKey, entryHmacKey);
return entry.take();
}

bool OpVaultReader::fillAttributes(Entry* entry, const QJsonObject& bandEntry)
Expand All @@ -225,9 +228,9 @@ bool OpVaultReader::fillAttributes(Entry* entry, const QJsonObject& bandEntry)
return false;
}

QByteArray overviewJsonBytes = entOver01.getClearText();
QJsonDocument overviewDoc = QJsonDocument::fromJson(overviewJsonBytes);
QJsonObject overviewJson = overviewDoc.object();
auto overviewJsonBytes = entOver01.getClearText();
auto overviewDoc = QJsonDocument::fromJson(overviewJsonBytes);
auto overviewJson = overviewDoc.object();

QString title = overviewJson.value("title").toString();
entry->setTitle(title);
Expand All @@ -236,26 +239,20 @@ bool OpVaultReader::fillAttributes(Entry* entry, const QJsonObject& bandEntry)
entry->setUrl(url);

int i = 1;
for (const auto& urlV : overviewJson["URLs"].toArray()) {
auto urlName = QString("URL_%1").arg(i);
auto urlValue = urlV.toString();
if (urlV.isObject()) {
const auto& urlObj = urlV.toObject();
if (urlObj["l"].isString() && urlObj["u"].isString()) {
urlName = urlObj["l"].toString();
urlValue = urlObj["u"].toString();
} else {
continue;
for (const auto urlV : overviewJson["URLs"].toArray()) {
const auto& urlObj = urlV.toObject();
if (urlObj.contains("u")) {
auto newUrl = urlObj["u"].toString();
if (newUrl != url) {
// Add this url if it isn't the base one
entry->attributes()->set(QString("KP2A_URL_%1").arg(i), newUrl);
++i;
}
}
if (!urlValue.isEmpty() && urlValue != url) {
entry->attributes()->set(urlName, urlValue);
++i;
}
}

QStringList tagsList;
for (const auto& tagV : overviewJson["tags"].toArray()) {
for (const auto tagV : overviewJson["tags"].toArray()) {
if (tagV.isString()) {
tagsList << tagV.toString();
}
Expand Down
64 changes: 48 additions & 16 deletions src/format/OpVaultReaderSections.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,29 @@
#include <QUrlQuery>
#include <QUuid>

namespace
{
QDateTime resolveDate(const QString& kind, const QJsonValue& value)
{
QDateTime date;
if (kind == "monthYear") {
// 1Password programmers are sadistic...
auto dateValue = QString::number(value.toInt());
date = QDateTime::fromString(dateValue, "yyyyMM");
date.setTimeSpec(Qt::UTC);
} else if (value.isString()) {
date = QDateTime::fromTime_t(value.toString().toUInt(), Qt::UTC);
} else {
date = QDateTime::fromTime_t(value.toInt(), Qt::UTC);
}
return date;
}
} // namespace

void OpVaultReader::fillFromSection(Entry* entry, const QJsonObject& section)
{
const auto uuid = entry->uuid();
const QString& sectionName = section["name"].toString();
QString sectionName = section["name"].toString();

if (!section.contains("fields")) {
auto sectionNameLC = sectionName.toLower();
Expand All @@ -47,6 +66,12 @@ void OpVaultReader::fillFromSection(Entry* entry, const QJsonObject& section)
qWarning() << R"(Skipping non-Array "fields" in UUID ")" << uuid << "\"\n";
return;
}

// If we have a default section name then replace with the section title if not empty
if (sectionName.startsWith("Section_") && !section["title"].toString().isEmpty()) {
sectionName = section["title"].toString();
}

QJsonArray sectionFields = section["fields"].toArray();
for (const QJsonValue sectionField : sectionFields) {
if (!sectionField.isObject()) {
Expand All @@ -68,7 +93,7 @@ void OpVaultReader::fillFromSectionField(Entry* entry, const QString& sectionNam
// Ignore "a" and "inputTraits" fields, they don't apply to KPXC

auto attrName = resolveAttributeName(sectionName, field["n"].toString(), field["t"].toString());
auto attrValue = field.value("v").toVariant().toString();
auto attrValue = field.value("v").toString();
auto kind = field["k"].toString();

if (attrName.startsWith("TOTP_")) {
Expand All @@ -82,30 +107,37 @@ void OpVaultReader::fillFromSectionField(Entry* entry, const QString& sectionNam
query.addQueryItem("period", QString("%1").arg(Totp::DEFAULT_STEP));
}
attrValue = query.toString(QUrl::FullyEncoded);
}
entry->setTotp(Totp::parseSettings(attrValue));
} else if (attrName.startsWith("expir", Qt::CaseInsensitive)) {
QDateTime expiry;
if (kind == "date") {
expiry = QDateTime::fromTime_t(attrValue.toUInt(), Qt::UTC);
entry->setTotp(Totp::parseSettings(attrValue));
} else {
expiry = QDateTime::fromString(attrValue, "yyyyMM");
expiry.setTimeSpec(Qt::UTC);
entry->setTotp(Totp::parseSettings({}, attrValue));
}

} else if (attrName.startsWith("expir", Qt::CaseInsensitive)) {
QDateTime expiry = resolveDate(kind, field.value("v"));
if (expiry.isValid()) {
entry->setExpiryTime(expiry);
entry->setExpires(true);
} else {
qWarning() << QString("[%1] Invalid expiration date found: %2").arg(entry->title(), attrValue);
}
} else {
if (kind == "date") {
auto date = QDateTime::fromTime_t(attrValue.toUInt(), Qt::UTC);
if (kind == "date" || kind == "monthYear") {
QDateTime date = resolveDate(kind, field.value("v"));
if (date.isValid()) {
attrValue = date.toString();
entry->attributes()->set(attrName, date.toString(Qt::SystemLocaleShortDate));
} else {
qWarning()
<< QString("[%1] Invalid date attribute found: %2 = %3").arg(entry->title(), attrName, attrValue);
}
} else if (kind == "address") {
// Expand address into multiple attributes
auto addrFields = field.value("v").toObject().toVariantMap();
for (auto part : addrFields.keys()) {
entry->attributes()->set(attrName + QString("_%1").arg(part), addrFields.value(part).toString());
}
} else {
entry->attributes()->set(attrName, attrValue, (kind == "password" || kind == "concealed"));
}

entry->attributes()->set(attrName, attrValue, (kind == "password" || kind == "concealed"));
}
}

Expand All @@ -118,7 +150,7 @@ QString OpVaultReader::resolveAttributeName(const QString& section, const QStrin

auto lowName = name.toLower();
auto lowText = text.toLower();
if (section.isEmpty()) {
if (section.isEmpty() || name.startsWith("address")) {
// Empty section implies these are core attributes
// try to find username, password, url
if (lowName == "password" || lowText == "password") {
Expand Down
6 changes: 5 additions & 1 deletion src/gui/DatabaseTabWidget.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,11 @@ void DatabaseTabWidget::importKeePass1Database()

void DatabaseTabWidget::importOpVaultDatabase()
{
QString fileName = fileDialog()->getExistingDirectory(this, "Open .opvault database");
#ifdef Q_MACOS
QString fileName = fileDialog()->getOpenFileName(this, tr("Open OPVault"), {}, "OPVault (*.opvault)");
#else
QString fileName = fileDialog()->getExistingDirectory(this, tr("Open OPVault"));
#endif

if (fileName.isEmpty()) {
return;
Expand Down
Loading

0 comments on commit 612f8d2

Please sign in to comment.