From c34098546d9d8af0693c23e67dc262c6938179ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sami=20V=C3=A4nttinen?= Date: Sun, 31 Mar 2024 23:12:33 +0300 Subject: [PATCH] Passkeys: Fix compatibility with StrongBox (#10420) --- src/browser/BrowserPasskeys.cpp | 5 +++++ src/browser/BrowserPasskeys.h | 2 ++ src/browser/BrowserService.cpp | 21 ++++++++++----------- src/browser/PasskeyUtils.cpp | 24 ++++++++++++++++++++++++ src/browser/PasskeyUtils.h | 3 +++ src/gui/passkeys/PasskeyExporter.cpp | 5 +++-- 6 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/browser/BrowserPasskeys.cpp b/src/browser/BrowserPasskeys.cpp index 74a4368df..b1dfe5120 100644 --- a/src/browser/BrowserPasskeys.cpp +++ b/src/browser/BrowserPasskeys.cpp @@ -59,6 +59,11 @@ const QString BrowserPasskeys::PASSKEYS_ATTESTATION_NONE = QStringLiteral("none" const QString BrowserPasskeys::KPEX_PASSKEY_USERNAME = QStringLiteral("KPEX_PASSKEY_USERNAME"); const QString BrowserPasskeys::KPEX_PASSKEY_CREDENTIAL_ID = QStringLiteral("KPEX_PASSKEY_CREDENTIAL_ID"); + +// For compatibility with StrongBox +const QString BrowserPasskeys::KPEX_PASSKEY_GENERATED_USER_ID = QStringLiteral("KPEX_PASSKEY_GENERATED_USER_ID"); +const QString BrowserPasskeys::KPXC_PASSKEY_USERNAME = QStringLiteral("KPXC_PASSKEY_USERNAME"); + const QString BrowserPasskeys::KPEX_PASSKEY_PRIVATE_KEY_PEM = QStringLiteral("KPEX_PASSKEY_PRIVATE_KEY_PEM"); const QString BrowserPasskeys::KPEX_PASSKEY_RELYING_PARTY = QStringLiteral("KPEX_PASSKEY_RELYING_PARTY"); const QString BrowserPasskeys::KPEX_PASSKEY_USER_HANDLE = QStringLiteral("KPEX_PASSKEY_USER_HANDLE"); diff --git a/src/browser/BrowserPasskeys.h b/src/browser/BrowserPasskeys.h index 806b663f8..0c09e3314 100644 --- a/src/browser/BrowserPasskeys.h +++ b/src/browser/BrowserPasskeys.h @@ -105,8 +105,10 @@ public: static const QString PASSKEYS_ATTESTATION_DIRECT; static const QString PASSKEYS_ATTESTATION_NONE; + static const QString KPXC_PASSKEY_USERNAME; static const QString KPEX_PASSKEY_USERNAME; static const QString KPEX_PASSKEY_CREDENTIAL_ID; + static const QString KPEX_PASSKEY_GENERATED_USER_ID; static const QString KPEX_PASSKEY_PRIVATE_KEY_PEM; static const QString KPEX_PASSKEY_RELYING_PARTY; static const QString KPEX_PASSKEY_USER_HANDLE; diff --git a/src/browser/BrowserService.cpp b/src/browser/BrowserService.cpp index 951f73d4a..ef0e1bfaf 100644 --- a/src/browser/BrowserService.cpp +++ b/src/browser/BrowserService.cpp @@ -722,7 +722,7 @@ QJsonObject BrowserService::showPasskeysAuthenticationPrompt(const QJsonObject& } const auto privateKeyPem = selectedEntry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_PRIVATE_KEY_PEM); - const auto credentialId = selectedEntry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_CREDENTIAL_ID); + const auto credentialId = passkeyUtils()->getCredentialIdFromEntry(selectedEntry); const auto userHandle = selectedEntry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_USER_HANDLE); auto publicKeyCredential = @@ -788,13 +788,12 @@ void BrowserService::addPasskeyToEntry(Entry* entry, // Ask confirmation if entry already contains a Passkey if (entry->hasPasskey()) { - if (MessageBox::question( - m_currentDatabaseWidget, - tr("KeePassXC - Update Passkey"), - tr("Entry already has a Passkey.\nDo you want to overwrite the Passkey in %1 - %2?") - .arg(entry->title(), entry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_USERNAME)), - MessageBox::Overwrite | MessageBox::Cancel, - MessageBox::Cancel) + if (MessageBox::question(m_currentDatabaseWidget, + tr("KeePassXC - Update Passkey"), + tr("Entry already has a Passkey.\nDo you want to overwrite the Passkey in %1 - %2?") + .arg(entry->title(), passkeyUtils()->getUsernameFromEntry(entry)), + MessageBox::Overwrite | MessageBox::Cancel, + MessageBox::Cancel) != MessageBox::Overwrite) { return; } @@ -1129,7 +1128,7 @@ QJsonObject BrowserService::prepareEntry(const Entry* entry) QJsonObject res; #ifdef WITH_XC_BROWSER_PASSKEYS // Use Passkey's username instead if found - res["login"] = entry->hasPasskey() ? entry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_USERNAME) + res["login"] = entry->hasPasskey() ? passkeyUtils()->getUsernameFromEntry(entry) : entry->resolveMultiplePlaceholders(entry->username()); #else res["login"] = entry->resolveMultiplePlaceholders(entry->username()); @@ -1363,7 +1362,7 @@ QList BrowserService::getPasskeyAllowedEntries(const QJsonObject& assert // If allowedCredentials.isEmpty() check if entry contains an extra attribute for user handle. // If that is found, the entry should be allowed. // See: https://w3c.github.io/webauthn/#dom-authenticatorassertionresponse-userhandle - if (allowedCredentials.contains(entry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_CREDENTIAL_ID)) + if (allowedCredentials.contains(passkeyUtils()->getCredentialIdFromEntry(entry)) || (allowedCredentials.isEmpty() && entry->attributes()->hasKey(BrowserPasskeys::KPEX_PASSKEY_USER_HANDLE))) { entries << entry; @@ -1385,7 +1384,7 @@ bool BrowserService::isPasskeyCredentialExcluded(const QJsonArray& excludeCreden const auto passkeyEntries = getPasskeyEntries(rpId, keyList); return std::any_of(passkeyEntries.begin(), passkeyEntries.end(), [&](const auto& entry) { - return allIds.contains(entry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_CREDENTIAL_ID)); + return allIds.contains(passkeyUtils()->getCredentialIdFromEntry(entry)); }); } diff --git a/src/browser/PasskeyUtils.cpp b/src/browser/PasskeyUtils.cpp index a018a6b52..0dda93a6e 100644 --- a/src/browser/PasskeyUtils.cpp +++ b/src/browser/PasskeyUtils.cpp @@ -352,3 +352,27 @@ QStringList PasskeyUtils::getAllowedCredentialsFromAssertionOptions(const QJsonO return allowedCredentials; } + +// For compatibility with StrongBox (and other possible clients in the future) +QString PasskeyUtils::getCredentialIdFromEntry(const Entry* entry) const +{ + if (!entry) { + return {}; + } + + return entry->attributes()->hasKey(BrowserPasskeys::KPEX_PASSKEY_GENERATED_USER_ID) + ? entry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_GENERATED_USER_ID) + : entry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_CREDENTIAL_ID); +} + +// For compatibility with StrongBox (and other possible clients in the future) +QString PasskeyUtils::getUsernameFromEntry(const Entry* entry) const +{ + if (!entry) { + return {}; + } + + return entry->attributes()->hasKey(BrowserPasskeys::KPXC_PASSKEY_USERNAME) + ? entry->attributes()->value(BrowserPasskeys::KPXC_PASSKEY_USERNAME) + : entry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_USERNAME); +} diff --git a/src/browser/PasskeyUtils.h b/src/browser/PasskeyUtils.h index 1a08e295a..0ac689ae6 100644 --- a/src/browser/PasskeyUtils.h +++ b/src/browser/PasskeyUtils.h @@ -24,6 +24,7 @@ #include #include "BrowserCbor.h" +#include "core/Entry.h" #define DEFAULT_TIMEOUT 300000 #define DEFAULT_DISCOURAGED_TIMEOUT 120000 @@ -53,6 +54,8 @@ public: QByteArray buildExtensionData(QJsonObject& extensionObject) const; QJsonObject buildClientDataJson(const QJsonObject& publicKey, const QString& origin, bool get) const; QStringList getAllowedCredentialsFromAssertionOptions(const QJsonObject& assertionOptions) const; + QString getCredentialIdFromEntry(const Entry* entry) const; + QString getUsernameFromEntry(const Entry* entry) const; private: Q_DISABLE_COPY(PasskeyUtils); diff --git a/src/gui/passkeys/PasskeyExporter.cpp b/src/gui/passkeys/PasskeyExporter.cpp index e1483930f..2585c76e0 100644 --- a/src/gui/passkeys/PasskeyExporter.cpp +++ b/src/gui/passkeys/PasskeyExporter.cpp @@ -19,6 +19,7 @@ #include "PasskeyExportDialog.h" #include "browser/BrowserPasskeys.h" +#include "browser/PasskeyUtils.h" #include "core/Entry.h" #include "core/Tools.h" #include "gui/MessageBox.h" @@ -90,8 +91,8 @@ void PasskeyExporter::exportSelectedEntry(const Entry* entry, const QString& fol QJsonObject passkeyObject; passkeyObject["relyingParty"] = entry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_RELYING_PARTY); passkeyObject["url"] = entry->url(); - passkeyObject["username"] = entry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_USERNAME); - passkeyObject["credentialId"] = entry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_CREDENTIAL_ID); + passkeyObject["username"] = passkeyUtils()->getUsernameFromEntry(entry); + passkeyObject["credentialId"] = passkeyUtils()->getCredentialIdFromEntry(entry); passkeyObject["userHandle"] = entry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_USER_HANDLE); passkeyObject["privateKey"] = entry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_PRIVATE_KEY_PEM);