Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passkeys improvements #10318

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 40 additions & 4 deletions share/translations/keepassxc_en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,14 @@ Do you want to overwrite the Passkey in %1 - %2?</source>
<source>&lt;b&gt;Error:&lt;/b&gt; The installed proxy executable is missing from the expected location: %1&lt;br/&gt;Please set a custom proxy location in the advanced settings or reinstall the application.</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Allows using insecure http://localhost with Passkeys for testing purposes.</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Allow using localhost with Passkeys</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>CloneDialog</name>
Expand Down Expand Up @@ -8419,10 +8427,6 @@ Kernel: %3 %4</source>
<source>Invalid URL provided</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Resident Keys are not supported</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Passkeys</source>
<translation type="unfinished"></translation>
Expand Down Expand Up @@ -8483,6 +8487,38 @@ Kernel: %3 %4</source>
<source>Failed to decrypt key data.</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Origin is empty or not allowed</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Effective domain is not a valid domain</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Origin and RP ID do not match</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>No supported algorithms were provided</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Wait for timer to expire</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Unknown Passkeys error</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Challenge is shorter than required minimum length</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>user.id does not match the required length</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>QtIOCompressor</name>
Expand Down
9 changes: 5 additions & 4 deletions src/browser/BrowserAction.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2023 KeePassXC Team <[email protected]>
* Copyright (C) 2024 KeePassXC Team <[email protected]>
varjolintu marked this conversation as resolved.
Show resolved Hide resolved
*
* 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
Expand All @@ -19,6 +19,7 @@
#include "BrowserMessageBuilder.h"
#ifdef WITH_XC_BROWSER_PASSKEYS
#include "BrowserPasskeys.h"
#include "PasskeyUtils.h"
#endif
#include "BrowserSettings.h"
#include "core/Global.h"
Expand Down Expand Up @@ -541,7 +542,7 @@ QJsonObject BrowserAction::handlePasskeysGet(const QJsonObject& json, const QStr
}

const auto origin = browserRequest.getString("origin");
if (!origin.startsWith("https://")) {
if (!passkeyUtils()->isOriginAllowedWithLocalhost(browserSettings()->allowLocalhostWithPasskeys(), origin)) {
return getErrorReply(action, ERROR_PASSKEYS_INVALID_URL_PROVIDED);
}

Expand Down Expand Up @@ -574,8 +575,8 @@ QJsonObject BrowserAction::handlePasskeysRegister(const QJsonObject& json, const
}

const auto origin = browserRequest.getString("origin");
if (!origin.startsWith("https://")) {
return getErrorReply(action, ERROR_KEEPASS_ACTION_CANCELLED_OR_DENIED);
if (!passkeyUtils()->isOriginAllowedWithLocalhost(browserSettings()->allowLocalhostWithPasskeys(), origin)) {
return getErrorReply(action, ERROR_PASSKEYS_INVALID_URL_PROVIDED);
}

const auto keyList = getConnectionKeys(browserRequest);
Expand Down
5 changes: 5 additions & 0 deletions src/browser/BrowserAction.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ struct BrowserRequest
return decrypted.value(param).toArray();
}

inline bool getBool(const QString& param) const
{
return decrypted.value(param).toBool();
}

inline QJsonObject getObject(const QString& param) const
{
return decrypted.value(param).toObject();
Expand Down
40 changes: 27 additions & 13 deletions src/browser/BrowserCbor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ QByteArray BrowserCbor::cborEncodeAttestation(const QByteArray& authData) const
// https://w3c.github.io/webauthn/#authdata-attestedcredentialdata-credentialpublickey
QByteArray BrowserCbor::cborEncodePublicKey(int alg, const QByteArray& first, const QByteArray& second) const
{
const auto keyType = getCoseKeyType(alg);
if (keyType == 0) {
return {};
}

const auto curveParameter = getCurveParameter(alg);
if ((alg == WebAuthnAlgorithms::ES256 || alg == WebAuthnAlgorithms::EDDSA) && curveParameter == 0) {
return {};
}

QByteArray result;
QCborStreamWriter writer(&result);

Expand All @@ -56,15 +66,15 @@ QByteArray BrowserCbor::cborEncodePublicKey(int alg, const QByteArray& first, co

// Key type
writer.append(1);
writer.append(getCoseKeyType(alg));
writer.append(keyType);

// Signature algorithm
writer.append(3);
writer.append(alg);

// Curve parameter
writer.append(-1);
writer.append(getCurveParameter(alg));
writer.append(curveParameter);

// Key x-coordinate
writer.append(-2);
Expand All @@ -80,7 +90,7 @@ QByteArray BrowserCbor::cborEncodePublicKey(int alg, const QByteArray& first, co

// Key type
writer.append(1);
writer.append(getCoseKeyType(alg));
writer.append(keyType);

// Signature algorithm
writer.append(3);
Expand All @@ -96,21 +106,24 @@ QByteArray BrowserCbor::cborEncodePublicKey(int alg, const QByteArray& first, co

writer.endMap();
} else if (alg == WebAuthnAlgorithms::EDDSA) {
// https://www.rfc-editor.org/rfc/rfc8152#section-13.2
writer.startMap(3);
writer.startMap(4);

// Key type
writer.append(1);
writer.append(keyType);

// Algorithm
writer.append(3);
writer.append(alg);

// Curve parameter
writer.append(-1);
writer.append(getCurveParameter(alg));
writer.append(curveParameter);

// Public key
writer.append(-2);
writer.append(first);

// Private key
writer.append(-4);
writer.append(second);

writer.endMap();
}

Expand Down Expand Up @@ -230,7 +243,7 @@ unsigned int BrowserCbor::getCurveParameter(int alg) const
case WebAuthnAlgorithms::EDDSA:
return WebAuthnCurveKey::ED25519;
default:
return WebAuthnCurveKey::P256;
return 0;
varjolintu marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -240,14 +253,15 @@ unsigned int BrowserCbor::getCoseKeyType(int alg) const
{
switch (alg) {
case WebAuthnAlgorithms::ES256:
return WebAuthnCoseKeyType::EC2;
case WebAuthnAlgorithms::ES384:
case WebAuthnAlgorithms::ES512:
return WebAuthnCoseKeyType::EC2;
return 0;
varjolintu marked this conversation as resolved.
Show resolved Hide resolved
case WebAuthnAlgorithms::EDDSA:
return WebAuthnCoseKeyType::OKP;
case WebAuthnAlgorithms::RS256:
return WebAuthnCoseKeyType::RSA;
default:
return WebAuthnCoseKeyType::EC2;
return 0;
varjolintu marked this conversation as resolved.
Show resolved Hide resolved
}
}
18 changes: 16 additions & 2 deletions src/browser/BrowserMessageBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,22 @@ QString BrowserMessageBuilder::getErrorMessage(const int errorCode) const
return QObject::tr("Empty public key");
case ERROR_PASSKEYS_INVALID_URL_PROVIDED:
return QObject::tr("Invalid URL provided");
case ERROR_PASSKEYS_RESIDENT_KEYS_NOT_SUPPORTED:
return QObject::tr("Resident Keys are not supported");
case ERROR_PASSKEYS_ORIGIN_NOT_ALLOWED:
return QObject::tr("Origin is empty or not allowed");
case ERROR_PASSKEYS_DOMAIN_IS_NOT_VALID:
return QObject::tr("Effective domain is not a valid domain");
case ERROR_PASSKEYS_DOMAIN_RPID_MISMATCH:
return QObject::tr("Origin and RP ID do not match");
case ERROR_PASSKEYS_NO_SUPPORTED_ALGORITHMS:
return QObject::tr("No supported algorithms were provided");
case ERROR_PASSKEYS_WAIT_FOR_LIFETIMER:
return QObject::tr("Wait for timer to expire");
case ERROR_PASSKEYS_UNKNOWN_ERROR:
return QObject::tr("Unknown Passkeys error");
case ERROR_PASSKEYS_INVALID_CHALLENGE:
return QObject::tr("Challenge is shorter than required minimum length");
case ERROR_PASSKEYS_INVALID_USER_ID:
return QObject::tr("user.id does not match the required length");
default:
return QObject::tr("Unknown error");
}
Expand Down
11 changes: 9 additions & 2 deletions src/browser/BrowserMessageBuilder.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2023 KeePassXC Team <[email protected]>
* Copyright (C) 2024 KeePassXC Team <[email protected]>
*
* 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
Expand Down Expand Up @@ -55,7 +55,14 @@ namespace
ERROR_PASSKEYS_INVALID_USER_VERIFICATION = 23,
ERROR_PASSKEYS_EMPTY_PUBLIC_KEY = 24,
ERROR_PASSKEYS_INVALID_URL_PROVIDED = 25,
ERROR_PASSKEYS_RESIDENT_KEYS_NOT_SUPPORTED = 26,
ERROR_PASSKEYS_ORIGIN_NOT_ALLOWED = 26,
ERROR_PASSKEYS_DOMAIN_IS_NOT_VALID = 27,
ERROR_PASSKEYS_DOMAIN_RPID_MISMATCH = 28,
ERROR_PASSKEYS_NO_SUPPORTED_ALGORITHMS = 29,
ERROR_PASSKEYS_WAIT_FOR_LIFETIMER = 30,
ERROR_PASSKEYS_UNKNOWN_ERROR = 31,
ERROR_PASSKEYS_INVALID_CHALLENGE = 32,
ERROR_PASSKEYS_INVALID_USER_ID = 33,
};
}

Expand Down
Loading
Loading