From 5cb6ad6335cf8fd89910d2ff6b27a93695f6e3ad Mon Sep 17 00:00:00 2001 From: varjolintu Date: Thu, 22 May 2025 21:20:03 +0300 Subject: [PATCH] Passkeys: Fix ordering of clientDataJSON --- src/browser/BrowserPasskeys.cpp | 12 ++++++------ src/browser/PasskeyUtils.cpp | 20 ++++++++------------ src/browser/PasskeyUtils.h | 4 ++-- tests/TestPasskeys.cpp | 10 +++++----- 4 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/browser/BrowserPasskeys.cpp b/src/browser/BrowserPasskeys.cpp index 1f9804487..287b42cca 100644 --- a/src/browser/BrowserPasskeys.cpp +++ b/src/browser/BrowserPasskeys.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2024 KeePassXC Team + * Copyright (C) 2025 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -71,7 +71,7 @@ PublicKeyCredential BrowserPasskeys::buildRegisterPublicKeyCredential(const QJso } const auto authenticatorAttachment = credentialCreationOptions["authenticatorAttachment"]; - const auto clientDataJson = credentialCreationOptions["clientDataJSON"].toObject(); + const auto clientDataJson = credentialCreationOptions["clientDataJSON"].toString(); const auto extensions = credentialCreationOptions["extensions"].toString(); const auto credentialId = testingVariables.credentialId.isEmpty() ? browserMessageBuilder()->getRandomBytesAsBase64(ID_BYTES) @@ -98,7 +98,7 @@ PublicKeyCredential BrowserPasskeys::buildRegisterPublicKeyCredential(const QJso // Response QJsonObject responseObject; responseObject["attestationObject"] = browserMessageBuilder()->getBase64FromArray(attestationObject); - responseObject["clientDataJSON"] = browserMessageBuilder()->getBase64FromJson(clientDataJson); + responseObject["clientDataJSON"] = browserMessageBuilder()->getBase64FromArray(clientDataJson.toUtf8()); responseObject["clientExtensionResults"] = credentialCreationOptions["clientExtensionResults"]; // Additions for extension side functions @@ -130,8 +130,8 @@ QJsonObject BrowserPasskeys::buildGetPublicKeyCredential(const QJsonObject& asse const auto authenticatorData = buildAuthenticatorData(assertionOptions["rpId"].toString(), assertionOptions["extensions"].toString()); - const auto clientDataJson = assertionOptions["clientDataJson"].toObject(); - const auto clientDataArray = QJsonDocument(clientDataJson).toJson(QJsonDocument::Compact); + const auto clientDataJson = assertionOptions["clientDataJson"].toString(); + const auto clientDataArray = clientDataJson.toUtf8(); const auto signature = buildSignature(authenticatorData, clientDataArray, privateKeyPem); if (signature.isEmpty()) { @@ -140,7 +140,7 @@ QJsonObject BrowserPasskeys::buildGetPublicKeyCredential(const QJsonObject& asse QJsonObject responseObject; responseObject["authenticatorData"] = browserMessageBuilder()->getBase64FromArray(authenticatorData); - responseObject["clientDataJSON"] = browserMessageBuilder()->getBase64FromJson(clientDataJson); + responseObject["clientDataJSON"] = browserMessageBuilder()->getBase64FromArray(clientDataArray); responseObject["clientExtensionResults"] = assertionOptions["clientExtensionResults"]; responseObject["signature"] = browserMessageBuilder()->getBase64FromArray(signature); responseObject["userHandle"] = userHandle; diff --git a/src/browser/PasskeyUtils.cpp b/src/browser/PasskeyUtils.cpp index 41fc94221..b28c94dbb 100644 --- a/src/browser/PasskeyUtils.cpp +++ b/src/browser/PasskeyUtils.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2024 KeePassXC Team + * Copyright (C) 2025 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -53,8 +53,8 @@ bool PasskeyUtils::checkCredentialCreationOptions(const QJsonObject& credentialC { if (!credentialCreationOptions["attestation"].isString() || credentialCreationOptions["attestation"].toString().isEmpty() - || !credentialCreationOptions["clientDataJSON"].isObject() - || credentialCreationOptions["clientDataJSON"].toObject().isEmpty() + || !credentialCreationOptions["clientDataJSON"].isString() + || credentialCreationOptions["clientDataJSON"].toString().isEmpty() || !credentialCreationOptions["rp"].isObject() || credentialCreationOptions["rp"].toObject().isEmpty() || !credentialCreationOptions["user"].isObject() || credentialCreationOptions["user"].toObject().isEmpty() || !credentialCreationOptions["residentKey"].isBool() || credentialCreationOptions["residentKey"].isUndefined() @@ -75,7 +75,7 @@ bool PasskeyUtils::checkCredentialCreationOptions(const QJsonObject& credentialC // Basic check for the object that it contains necessary variables in a correct form bool PasskeyUtils::checkCredentialAssertionOptions(const QJsonObject& assertionOptions) const { - if (!assertionOptions["clientDataJson"].isObject() || assertionOptions["clientDataJson"].toObject().isEmpty() + if (!assertionOptions["clientDataJson"].isString() || assertionOptions["clientDataJson"].toString().isEmpty() || !assertionOptions["rpId"].isString() || assertionOptions["rpId"].toString().isEmpty() || !assertionOptions["userPresence"].isBool() || assertionOptions["userPresence"].isUndefined() || !assertionOptions["userVerification"].isBool() || assertionOptions["userVerification"].isUndefined()) { @@ -352,15 +352,11 @@ ExtensionResult PasskeyUtils::buildExtensionData(QJsonObject& extensionObject) c return {}; } -QJsonObject PasskeyUtils::buildClientDataJson(const QJsonObject& publicKey, const QString& origin, bool get) const +// Serialization order: https://w3c.github.io/webauthn/#clientdatajson-serialization +QString PasskeyUtils::buildClientDataJson(const QJsonObject& publicKey, const QString& origin, bool get) const { - QJsonObject clientData; - clientData["challenge"] = publicKey["challenge"]; - clientData["crossOrigin"] = false; - clientData["origin"] = origin; - clientData["type"] = get ? QString("webauthn.get") : QString("webauthn.create"); - - return clientData; + return QString("{\"type\":\"%1\",\"challenge\":\"%2\",\"origin\":\"%3\",\"crossOrigin\":false}") + .arg((get ? QString("webauthn.get") : QString("webauthn.create")), publicKey["challenge"].toString(), origin); } QStringList PasskeyUtils::getAllowedCredentialsFromAssertionOptions(const QJsonObject& assertionOptions) const diff --git a/src/browser/PasskeyUtils.h b/src/browser/PasskeyUtils.h index ec8e47668..b1e6a48ab 100644 --- a/src/browser/PasskeyUtils.h +++ b/src/browser/PasskeyUtils.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2024 KeePassXC Team + * Copyright (C) 2025 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -58,7 +58,7 @@ public: bool isUserVerificationRequired(const QJsonObject& authenticatorSelection) const; bool isOriginAllowedWithLocalhost(bool allowLocalhostWithPasskeys, const QString& origin) const; ExtensionResult buildExtensionData(QJsonObject& extensionObject) const; - QJsonObject buildClientDataJson(const QJsonObject& publicKey, const QString& origin, bool get) const; + QString 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; diff --git a/tests/TestPasskeys.cpp b/tests/TestPasskeys.cpp index 1618dce28..fd25c2baf 100644 --- a/tests/TestPasskeys.cpp +++ b/tests/TestPasskeys.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2024 KeePassXC Team + * Copyright (C) 2025 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -440,12 +440,12 @@ void TestPasskeys::testGet() auto response = publicKeyCredential["response"].toObject(); QCOMPARE(response["authenticatorData"].toString(), QString("dKbqkhPJnC90siSSsyDPQCYqlMGpUKA5fyklC2CEHvAFAAAAAA")); QCOMPARE(response["clientDataJSON"].toString(), - QString("eyJjaGFsbGVuZ2UiOiI5ejM2dlRmUVRMOTVMZjdXblpneXRlN29oR2VGLVhSaUx4a0wtTHVHVTF6b3BSbU1JVUExTFZ3ekdwe" - "UltMWZPQm4xUW5SYTBRSDI3QURBYUpHSHlzUSIsImNyb3NzT3JpZ2luIjpmYWxzZSwib3JpZ2luIjoiaHR0cHM6Ly93ZWJhdX" - "Robi5pbyIsInR5cGUiOiJ3ZWJhdXRobi5nZXQifQ")); + QString("eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiOXozNnZUZlFUTDk1TGY3V25aZ3l0ZTdvaEdlRi1YUmlMeGtML" + "Ux1R1Uxem9wUm1NSVVBMUxWd3pHcHlJbTFmT0JuMVFuUmEwUUgyN0FEQWFKR0h5c1EiLCJvcmlnaW4iOiJodHRwczovL3dlYm" + "F1dGhuLmlvIiwiY3Jvc3NPcmlnaW4iOmZhbHNlfQ")); QCOMPARE( response["signature"].toString(), - QString("MEUCIHFv0lOOGGloi_XoH5s3QDSs__8yAp9ZTMEjNiacMpOxAiEA04LAfO6TE7j12XNxd3zHQpn4kZN82jQFPntPiPBSD5c")); + QString("MEYCIQCpbDaYJ4b2ofqWBxfRNbH3XCpsyao7Iui5lVuJRU9HIQIhAPl5moNZgJu5zmurkKK_P900Ct6wd3ahVIqCEqTeeRdE")); auto clientDataJson = response["clientDataJSON"].toString(); auto clientDataByteArray = browserMessageBuilder()->getArrayFromBase64(clientDataJson);