Skip to content

Commit

Permalink
Passkeys: Fix compatibility with StrongBox (#10420)
Browse files Browse the repository at this point in the history
  • Loading branch information
varjolintu authored and droidmonkey committed Apr 28, 2024
1 parent a5c3bf6 commit 305fd24
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 13 deletions.
5 changes: 5 additions & 0 deletions src/browser/BrowserPasskeys.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
2 changes: 2 additions & 0 deletions src/browser/BrowserPasskeys.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,10 @@ 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;
static const QString KPEX_PASSKEY_PRIVATE_KEY_PEM;
static const QString KPEX_PASSKEY_RELYING_PARTY;
static const QString KPEX_PASSKEY_USER_HANDLE;
Expand Down
21 changes: 10 additions & 11 deletions src/browser/BrowserService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,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 =
Expand Down Expand Up @@ -739,13 +739,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;
}
Expand Down Expand Up @@ -1136,7 +1135,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());
Expand Down Expand Up @@ -1370,7 +1369,7 @@ QList<Entry*> 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;
Expand All @@ -1392,7 +1391,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));
});
}

Expand Down
24 changes: 24 additions & 0 deletions src/browser/PasskeyUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
3 changes: 3 additions & 0 deletions src/browser/PasskeyUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <QStringList>

#include "BrowserCbor.h"
#include "core/Entry.h"

#define DEFAULT_TIMEOUT 300000
#define DEFAULT_DISCOURAGED_TIMEOUT 120000
Expand Down Expand Up @@ -53,6 +54,8 @@ 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;
QString getUsernameFromEntry(const Entry* entry) const;

private:
Q_DISABLE_COPY(PasskeyUtils);
Expand Down
5 changes: 3 additions & 2 deletions src/gui/passkeys/PasskeyExporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit 305fd24

Please sign in to comment.