Skip to content

Commit

Permalink
Clearer error messages for KeePassXC-Browser (#2622)
Browse files Browse the repository at this point in the history
  • Loading branch information
@droidmonkey
Sami Vänttinen authored and droidmonkey committed Jan 25, 2019
1 parent 395a88a commit d662992
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 24 deletions.
46 changes: 23 additions & 23 deletions src/browser/BrowserAction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ BrowserAction::BrowserAction(BrowserService& browserService)
QJsonObject BrowserAction::readResponse(const QJsonObject& json)
{
if (json.isEmpty()) {
return QJsonObject();
return getErrorReply("", ERROR_KEEPASS_EMPTY_MESSAGE_RECEIVED);
}

bool triggerUnlock = false;
Expand All @@ -47,7 +47,7 @@ QJsonObject BrowserAction::readResponse(const QJsonObject& json)

const QString action = json.value("action").toString();
if (action.isEmpty()) {
return QJsonObject();
return getErrorReply(action, ERROR_KEEPASS_INCORRECT_ACTION);
}

QMutexLocker locker(&m_mutex);
Expand All @@ -68,9 +68,6 @@ QJsonObject BrowserAction::readResponse(const QJsonObject& json)
QJsonObject BrowserAction::handleAction(const QJsonObject& json)
{
QString action = json.value("action").toString();
if (action.isEmpty()) {
return getErrorReply(action, ERROR_KEEPASS_INCORRECT_ACTION);
}

if (action.compare("change-public-keys", Qt::CaseSensitive) == 0) {
return handleChangePublicKeys(json, action);
Expand Down Expand Up @@ -111,6 +108,10 @@ QJsonObject BrowserAction::handleChangePublicKeys(const QJsonObject& json, const

const QString publicKey = getBase64FromKey(pk, crypto_box_PUBLICKEYBYTES);
const QString secretKey = getBase64FromKey(sk, crypto_box_SECRETKEYBYTES);
if (publicKey.isEmpty() || secretKey.isEmpty()) {
return getErrorReply(action, ERROR_KEEPASS_ENCRYPTION_KEY_UNRECOGNIZED);
}

m_clientPublicKey = clientPublicKey;
m_publicKey = publicKey;
m_secretKey = secretKey;
Expand All @@ -127,7 +128,7 @@ QJsonObject BrowserAction::handleGetDatabaseHash(const QJsonObject& json, const
const QString hash = getDatabaseHash();
const QString nonce = json.value("nonce").toString();
const QString encrypted = json.value("message").toString();
const QJsonObject decrypted = decryptMessage(encrypted, nonce, action);
const QJsonObject decrypted = decryptMessage(encrypted, nonce);

if (decrypted.isEmpty()) {
return getErrorReply(action, ERROR_KEEPASS_CANNOT_DECRYPT_MESSAGE);
Expand All @@ -154,7 +155,7 @@ QJsonObject BrowserAction::handleAssociate(const QJsonObject& json, const QStrin
const QString hash = getDatabaseHash();
const QString nonce = json.value("nonce").toString();
const QString encrypted = json.value("message").toString();
const QJsonObject decrypted = decryptMessage(encrypted, nonce, action);
const QJsonObject decrypted = decryptMessage(encrypted, nonce);

if (decrypted.isEmpty()) {
return getErrorReply(action, ERROR_KEEPASS_CANNOT_DECRYPT_MESSAGE);
Expand Down Expand Up @@ -192,7 +193,7 @@ QJsonObject BrowserAction::handleTestAssociate(const QJsonObject& json, const QS
const QString hash = getDatabaseHash();
const QString nonce = json.value("nonce").toString();
const QString encrypted = json.value("message").toString();
const QJsonObject decrypted = decryptMessage(encrypted, nonce, action);
const QJsonObject decrypted = decryptMessage(encrypted, nonce);

if (decrypted.isEmpty()) {
return getErrorReply(action, ERROR_KEEPASS_CANNOT_DECRYPT_MESSAGE);
Expand Down Expand Up @@ -231,7 +232,7 @@ QJsonObject BrowserAction::handleGetLogins(const QJsonObject& json, const QStrin
return getErrorReply(action, ERROR_KEEPASS_ASSOCIATION_FAILED);
}

const QJsonObject decrypted = decryptMessage(encrypted, nonce, action);
const QJsonObject decrypted = decryptMessage(encrypted, nonce);
if (decrypted.isEmpty()) {
return getErrorReply(action, ERROR_KEEPASS_CANNOT_DECRYPT_MESSAGE);
}
Expand Down Expand Up @@ -304,7 +305,7 @@ QJsonObject BrowserAction::handleSetLogin(const QJsonObject& json, const QString
return getErrorReply(action, ERROR_KEEPASS_ASSOCIATION_FAILED);
}

const QJsonObject decrypted = decryptMessage(encrypted, nonce, action);
const QJsonObject decrypted = decryptMessage(encrypted, nonce);
if (decrypted.isEmpty()) {
return getErrorReply(action, ERROR_KEEPASS_CANNOT_DECRYPT_MESSAGE);
}
Expand Down Expand Up @@ -343,7 +344,7 @@ QJsonObject BrowserAction::handleLockDatabase(const QJsonObject& json, const QSt
const QString hash = getDatabaseHash();
const QString nonce = json.value("nonce").toString();
const QString encrypted = json.value("message").toString();
const QJsonObject decrypted = decryptMessage(encrypted, nonce, action);
const QJsonObject decrypted = decryptMessage(encrypted, nonce);

if (decrypted.isEmpty()) {
return getErrorReply(action, ERROR_KEEPASS_CANNOT_DECRYPT_MESSAGE);
Expand Down Expand Up @@ -388,8 +389,13 @@ QJsonObject BrowserAction::buildMessage(const QString& nonce) const
QJsonObject BrowserAction::buildResponse(const QString& action, const QJsonObject& message, const QString& nonce)
{
QJsonObject response;
QString encryptedMessage = encryptMessage(message, nonce);
if (encryptedMessage.isEmpty()) {
return getErrorReply(action, ERROR_KEEPASS_CANNOT_ENCRYPT_MESSAGE);
}

response["action"] = action;
response["message"] = encryptMessage(message, nonce);
response["message"] = encryptedMessage;
response["nonce"] = nonce;
return response;
}
Expand All @@ -405,20 +411,14 @@ QString BrowserAction::getErrorMessage(const int errorCode) const
return QObject::tr("Client public key not received");
case ERROR_KEEPASS_CANNOT_DECRYPT_MESSAGE:
return QObject::tr("Cannot decrypt message");
case ERROR_KEEPASS_TIMEOUT_OR_NOT_CONNECTED:
return QObject::tr("Timeout or cannot connect to KeePassXC");
case ERROR_KEEPASS_ACTION_CANCELLED_OR_DENIED:
return QObject::tr("Action cancelled or denied");
case ERROR_KEEPASS_CANNOT_ENCRYPT_MESSAGE:
return QObject::tr("Cannot encrypt message or public key not found. Is Native Messaging enabled in KeePassXC?");
return QObject::tr("Message encryption failed.");
case ERROR_KEEPASS_ASSOCIATION_FAILED:
return QObject::tr("KeePassXC association failed, try again");
case ERROR_KEEPASS_KEY_CHANGE_FAILED:
return QObject::tr("Key change was not successful");
case ERROR_KEEPASS_ENCRYPTION_KEY_UNRECOGNIZED:
return QObject::tr("Encryption key is not recognized");
case ERROR_KEEPASS_NO_SAVED_DATABASES_FOUND:
return QObject::tr("No saved databases found");
case ERROR_KEEPASS_INCORRECT_ACTION:
return QObject::tr("Incorrect action");
case ERROR_KEEPASS_EMPTY_MESSAGE_RECEIVED:
Expand Down Expand Up @@ -457,18 +457,18 @@ QString BrowserAction::encryptMessage(const QJsonObject& message, const QString&
return QString();
}

QJsonObject BrowserAction::decryptMessage(const QString& message, const QString& nonce, const QString& action)
QJsonObject BrowserAction::decryptMessage(const QString& message, const QString& nonce)
{
if (message.isEmpty() || nonce.isEmpty()) {
return QJsonObject();
}

QByteArray ba = decrypt(message, nonce);
if (!ba.isEmpty()) {
return getJsonObject(ba);
if (ba.isEmpty()) {
return QJsonObject();
}

return getErrorReply(action, ERROR_KEEPASS_CANNOT_DECRYPT_MESSAGE);
return getJsonObject(ba);
}

QString BrowserAction::encrypt(const QString& plaintext, const QString& nonce)
Expand Down
2 changes: 1 addition & 1 deletion src/browser/BrowserAction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class BrowserAction : public QObject
QString getDatabaseHash();

QString encryptMessage(const QJsonObject& message, const QString& nonce);
QJsonObject decryptMessage(const QString& message, const QString& nonce, const QString& action = QString());
QJsonObject decryptMessage(const QString& message, const QString& nonce);
QString encrypt(const QString& plaintext, const QString& nonce);
QByteArray decrypt(const QString& encrypted, const QString& nonce);

Expand Down

0 comments on commit d662992

Please sign in to comment.