From 7e7eaa0158b2074eb68bff6db4d32deaedad3cae Mon Sep 17 00:00:00 2001 From: varjolintu Date: Wed, 13 Mar 2024 17:20:39 +0200 Subject: [PATCH 1/3] Passkeys: Fix compatibility with StrongBox --- src/browser/BrowserPasskeys.cpp | 2 ++ src/browser/BrowserPasskeys.h | 1 + src/browser/BrowserService.cpp | 6 +++--- src/browser/PasskeyUtils.cpp | 12 ++++++++++++ src/browser/PasskeyUtils.h | 2 ++ src/gui/passkeys/PasskeyExporter.cpp | 3 ++- 6 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/browser/BrowserPasskeys.cpp b/src/browser/BrowserPasskeys.cpp index 3fe8e006f5..4f52d11897 100644 --- a/src/browser/BrowserPasskeys.cpp +++ b/src/browser/BrowserPasskeys.cpp @@ -57,6 +57,8 @@ 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::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 783d5ca689..cfb0f15d23 100644 --- a/src/browser/BrowserPasskeys.h +++ b/src/browser/BrowserPasskeys.h @@ -105,6 +105,7 @@ class BrowserPasskeys : public QObject 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 951f73d4ad..b1ec2171ea 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 = @@ -1363,7 +1363,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 +1385,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 1b4b59bf97..d081876ea5 100644 --- a/src/browser/PasskeyUtils.cpp +++ b/src/browser/PasskeyUtils.cpp @@ -350,3 +350,15 @@ 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); +} diff --git a/src/browser/PasskeyUtils.h b/src/browser/PasskeyUtils.h index 1a08e295ad..ec6fcc63b2 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,7 @@ class PasskeyUtils : public QObject 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; private: Q_DISABLE_COPY(PasskeyUtils); diff --git a/src/gui/passkeys/PasskeyExporter.cpp b/src/gui/passkeys/PasskeyExporter.cpp index e1483930fa..19296cdb09 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" @@ -91,7 +92,7 @@ void PasskeyExporter::exportSelectedEntry(const Entry* entry, const QString& fol 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["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); From aea4ac73f3256605e2f2c330b929a725444cf5ea Mon Sep 17 00:00:00 2001 From: varjolintu Date: Thu, 14 Mar 2024 15:48:53 +0200 Subject: [PATCH 2/3] Add same to username --- src/browser/BrowserPasskeys.cpp | 3 +++ src/browser/BrowserPasskeys.h | 1 + src/browser/BrowserService.cpp | 15 +++++++-------- src/browser/PasskeyUtils.cpp | 12 ++++++++++++ src/browser/PasskeyUtils.h | 1 + src/gui/passkeys/PasskeyExporter.cpp | 2 +- 6 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/browser/BrowserPasskeys.cpp b/src/browser/BrowserPasskeys.cpp index 4f52d11897..eebe10fd95 100644 --- a/src/browser/BrowserPasskeys.cpp +++ b/src/browser/BrowserPasskeys.cpp @@ -57,8 +57,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 cfb0f15d23..1edd123014 100644 --- a/src/browser/BrowserPasskeys.h +++ b/src/browser/BrowserPasskeys.h @@ -103,6 +103,7 @@ class BrowserPasskeys : public QObject 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; diff --git a/src/browser/BrowserService.cpp b/src/browser/BrowserService.cpp index b1ec2171ea..ef0e1bfaf9 100644 --- a/src/browser/BrowserService.cpp +++ b/src/browser/BrowserService.cpp @@ -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()); diff --git a/src/browser/PasskeyUtils.cpp b/src/browser/PasskeyUtils.cpp index d081876ea5..60b3f27c17 100644 --- a/src/browser/PasskeyUtils.cpp +++ b/src/browser/PasskeyUtils.cpp @@ -362,3 +362,15 @@ QString PasskeyUtils::getCredentialIdFromEntry(const Entry* entry) const ? 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); +} \ No newline at end of file diff --git a/src/browser/PasskeyUtils.h b/src/browser/PasskeyUtils.h index ec6fcc63b2..0ac689ae6b 100644 --- a/src/browser/PasskeyUtils.h +++ b/src/browser/PasskeyUtils.h @@ -55,6 +55,7 @@ class PasskeyUtils : public QObject 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 19296cdb09..2585c76e04 100644 --- a/src/gui/passkeys/PasskeyExporter.cpp +++ b/src/gui/passkeys/PasskeyExporter.cpp @@ -91,7 +91,7 @@ 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["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); From 28822fb21f2b0a9523a9c1b80dcbf4a184522011 Mon Sep 17 00:00:00 2001 From: varjolintu Date: Thu, 14 Mar 2024 17:08:27 +0200 Subject: [PATCH 3/3] Add missing EOL --- src/browser/PasskeyUtils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/browser/PasskeyUtils.cpp b/src/browser/PasskeyUtils.cpp index 60b3f27c17..fae8cfb5ca 100644 --- a/src/browser/PasskeyUtils.cpp +++ b/src/browser/PasskeyUtils.cpp @@ -373,4 +373,4 @@ QString PasskeyUtils::getUsernameFromEntry(const Entry* entry) const return entry->attributes()->hasKey(BrowserPasskeys::KPXC_PASSKEY_USERNAME) ? entry->attributes()->value(BrowserPasskeys::KPXC_PASSKEY_USERNAME) : entry->attributes()->value(BrowserPasskeys::KPEX_PASSKEY_USERNAME); -} \ No newline at end of file +}