Mamanyonyo/akashi-esquizolandia
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add more secure password hashing support (#293)

Browse Source 
* Separate crypto operations into new class

* Implement PBKDF2 backend

* Add auto-update to stronger hashes

* clang-format pass

* documentation and cleanup

* Add unit tests for CryptoHelper

Co-authored-by: scatterflower <marisa@scatterflower.online>
This commit is contained in:
scatterflower 2022-06-30 15:13:22 -05:00 committed by GitHub
parent 0234588007
commit aece875362
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 236 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
core/include/aoclient.h
View File

@ -25,9 +25,6 @@
#include <QRegularExpression>
#include <QTimer>
#include <QtGlobal>
#if QT_VERSION > QT_VERSION_CHECK(5, 10, 0)
#include <QRandomGenerator>
#endif
#include "include/acl_roles_handler.h"
#include "include/network/aopacket.h"

160
core/include/crypto_helper.h Normal file
View File

@ -0,0 +1,160 @@
//////////////////////////////////////////////////////////////////////////////////////
// akashi - a server for Attorney Online 2 //
// Copyright (C) 2022 scatterflower //
// //
// This program is free software: you can redistribute it and/or modify //
// it under the terms of the GNU Affero General Public License as //
// published by the Free Software Foundation, either version 3 of the //
// License, or (at your option) any later version. //
// //
// This program is distributed in the hope that it will be useful, //
// but WITHOUT ANY WARRANTY; without even the implied warranty of //
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the //
// GNU Affero General Public License for more details. //
// //
// You should have received a copy of the GNU Affero General Public License //
// along with this program. If not, see <https://www.gnu.org/licenses/>. //
//////////////////////////////////////////////////////////////////////////////////////
#ifndef CRYPTO_HELPER_H
#define CRYPTO_HELPER_H
#include <QMessageAuthenticationCode>
#include <QString>
#if QT_VERSION > QT_VERSION_CHECK(5, 10, 0)
#include <QRandomGenerator>
#endif
/**
* @brief Simple header library for basic cryptographic functionality
*/
class CryptoHelper
{
private:
/**
* @brief Length of the output of PBKDF2
*/
static constexpr qint32 pbkdf2_output_len = 32; // 32 bytes (SHA-256)
/**
* @brief Configurable cost parameter of PBKDF2
*/
static constexpr quint32 pbkdf2_cost = 100000;
/**
* @brief Compute the SHA-256 HMAC of given data
*
* @param salt HMAC key
* @param password HMAC data
* @return QByteArray HMAC result
*/
static QByteArray hmac(QByteArray salt, QByteArray password)
{
QMessageAuthenticationCode hmac(QCryptographicHash::Sha256);
hmac.setKey(salt);
hmac.addData(password);
return hmac.result();
}
/**
* @brief Compute the SHA-256 HMAC of strings
*
* @param salt Salt value
* @param password Password value
* @return QString HMAC result, hex encoded
*/
static QString password_hmac(QString salt, QString password)
{
return hmac(salt.toUtf8(), password.toUtf8()).toHex();
}
/**
* @brief Perform the PBKDF2 key-derivation function
*
* @details PBKDF2 is a very configurable algorithm, but most of its functionality
* does not apply here. Instead, we fix the output to the size of the underlying
* hash function, which greatly simplifies the algorithm.
*
* @param salt Salt value
* @param password Password value
* @return QString PBKDF2 result, hex encoded
*/
static QString pbkdf2(QByteArray salt, QString password)
{
QByteArray bigendian_one("\x00\x00\x00\x01", 4);
QByteArray last_block = salt;
last_block.append(bigendian_one);
QByteArray result(pbkdf2_output_len, '\0');
for (unsigned int i = 0; i < pbkdf2_cost; i++) {
last_block = hmac(password.toUtf8(), last_block);
for (unsigned int n = 0; n < pbkdf2_output_len; n++)
result[n] = result[n] ^ last_block[n];
}
return result.toHex();
}
public:
/**
* @brief The length of the salt value used for PBKDF2, in bytes
*/
static constexpr qint32 pbkdf2_salt_len = 16;
/**
* @brief Compute the hash of a password to store, given a salt
*
* @details This function selects one of two hashing algorithm backends to use,
* dependent on the length of the salt. Older versions of this program used an
* 8-byte salt, but newer versions use a 16-byte salt. This allows us to version
* the algorithm being used without the need to change the schema of the database.
*
* @param salt Salt value
* @param password Password value
* @return QString Hashed password, hex encoded
*/
static QString hash_password(QByteArray salt, QString password)
{
// Select the correct hash backend based on the salt length
if (salt.length() < pbkdf2_salt_len) {
// Due to an implementation oversight, the old HMAC algorithm
// does not correctly handle salts. Instead of treating the hex string
// as binary data, it is used directly. We have to handle this case.
return password_hmac(salt.toHex(), password);
}
return pbkdf2(salt, password);
}
/**
* @brief Generate a random octet
*
* @return quint8 A random 8-bit value
*/
static quint8 rand8()
{
#if QT_VERSION < QT_VERSION_CHECK(5, 10, 0)
qsrand(QDateTime::currentMSecsSinceEpoch());
quint32 l_rand = qrand();
#else
quint32 l_rand = QRandomGenerator::system()->generate();
#endif
return (quint8)(l_rand & 0xFF);
}
/**
* @brief Generate an arbitrary amount of random data
*
* @param n Number of bytes to generate
* @return QByteArray Random bytes
*/
static QByteArray randbytes(int n)
{
QByteArray output;
for (int i = 0; i < n; i++) {
output.append(CryptoHelper::rand8());
}
return output;
}
};
#endif

6
core/include/db_manager.h
View File

@ -23,13 +23,13 @@
#include <QDateTime>
#include <QFileInfo>
#include <QHostAddress>
#include <QMessageAuthenticationCode>
#include <QSqlDatabase>
#include <QSqlDriver>
#include <QSqlError>
#include <QSqlQuery>
#include "include/acl_roles_handler.h"
#include "include/crypto_helper.h"
/**
* @brief A class used to handle database interaction.
@ -147,7 +147,7 @@ class DBManager : public QObject
* @see AOClient#cmdLogin and AOClient#cmdLogout for the username and password's contexts.
* @see ACLRolesHandler for details regarding ACL roles and ACL role identifiers.
*/
bool createUser(QString username, QString salt, QString password, QString acl);
bool createUser(QString username, QByteArray salt, QString password, QString acl);
/**
* @brief Deletes an authorised user from the database.
@ -274,4 +274,4 @@ class DBManager : public QObject
void updateDB(int current_version);
};
#endif // BAN_MANAGER_H
#endif // BAN_MANAGER_H

22
core/src/commands/authentication.cpp
View File

@ -18,6 +18,7 @@
#include "include/aoclient.h"
#include "include/config_manager.h"
#include "include/crypto_helper.h"
#include "include/db_manager.h"
#include "include/server.h"
@ -80,15 +81,7 @@ void AOClient::cmdSetRootPass(int argc, QStringList argv)
m_authenticated = false;
ConfigManager::setAuthType(DataTypes::AuthType::ADVANCED);
#if QT_VERSION < QT_VERSION_CHECK(5, 10, 0)
qsrand(QDateTime::currentMSecsSinceEpoch());
quint32 l_upper_salt = qrand();
quint32 l_lower_salt = qrand();
quint64 l_salt_number = (upper_salt << 32) | lower_salt;
#else
quint64 l_salt_number = QRandomGenerator::system()->generate64();
#endif
QString l_salt = QStringLiteral("%1").arg(l_salt_number, 16, 16, QLatin1Char('0'));
QByteArray l_salt = CryptoHelper::randbytes(16);
server->getDatabaseManager()->createUser("root", l_salt, argv[0], ACLRolesHandler::SUPER_ID);
}
@ -101,15 +94,8 @@ void AOClient::cmdAddUser(int argc, QStringList argv)
sendServerMessage("Password does not meet server requirements.");
return;
}
#if QT_VERSION < QT_VERSION_CHECK(5, 10, 0)
qsrand(QDateTime::currentMSecsSinceEpoch());
quint32 l_upper_salt = qrand();
quint32 l_lower_salt = qrand();
quint64 l_salt_number = (upper_salt << 32) | lower_salt;
#else
quint64 l_salt_number = QRandomGenerator::system()->generate64();
#endif
QString l_salt = QStringLiteral("%1").arg(l_salt_number, 16, 16, QLatin1Char('0'));
QByteArray l_salt = CryptoHelper::randbytes(16);
if (server->getDatabaseManager()->createUser(argv[0], l_salt, argv[1], ACLRolesHandler::NONE_ID))
sendServerMessage("Created user " + argv[0] + ".\nUse /setperms to modify their permissions.");

47
core/src/db_manager.cpp
View File

@ -171,7 +171,7 @@ bool DBManager::invalidateBan(int id)
return true;
}
bool DBManager::createUser(QString f_username, QString f_salt, QString f_password, QString f_acl)
bool DBManager::createUser(QString f_username, QByteArray f_salt, QString f_password, QString f_acl)
{
QSqlQuery username_exists;
username_exists.prepare("SELECT ACL FROM users WHERE USERNAME = ?");
@ -183,15 +183,11 @@ bool DBManager::createUser(QString f_username, QString f_salt, QString f_passwor
QSqlQuery query;
QString salted_password;
QMessageAuthenticationCode hmac(QCryptographicHash::Sha256);
hmac.setKey(f_salt.toUtf8());
hmac.addData(f_password.toUtf8());
salted_password = hmac.result().toHex();
QString salted_password = CryptoHelper::hash_password(f_salt, f_password);
query.prepare("INSERT INTO users(USERNAME, SALT, PASSWORD, ACL) VALUES(?, ?, ?, ?)");
query.addBindValue(f_username);
query.addBindValue(f_salt);
query.addBindValue(f_salt.toHex());
query.addBindValue(salted_password);
query.addBindValue(f_acl);
query.exec();
@ -247,19 +243,20 @@ bool DBManager::authenticate(QString username, QString password)
return false;
QString salt = query_salt.value(0).toString();
QString salted_password;
QMessageAuthenticationCode hmac(QCryptographicHash::Sha256);
hmac.setKey(salt.toUtf8());
hmac.addData(password.toUtf8());
salted_password = hmac.result().toHex();
QString salted_password = CryptoHelper::hash_password(QByteArray::fromHex(salt.toUtf8()), password);
QSqlQuery query_pass("SELECT PASSWORD FROM users WHERE SALT = ?");
query_pass.addBindValue(salt);
QSqlQuery query_pass("SELECT PASSWORD FROM users WHERE USERNAME = ?");
query_pass.addBindValue(username);
query_pass.exec();
if (!query_pass.first())
return false;
QString stored_pass = query_pass.value(0).toString();
// Update old-style hashes to new ones on the fly
if (QByteArray::fromHex(salt.toUtf8()).length() < CryptoHelper::pbkdf2_salt_len && salted_password == stored_pass) {
updatePassword(username, password);
}
return salted_password == stored_pass;
}
@ -353,27 +350,13 @@ bool DBManager::updateBan(int ban_id, QString field, QVariant updated_info)
bool DBManager::updatePassword(QString username, QString password)
{
QString salt;
QSqlQuery salt_check;
salt_check.prepare("SELECT SALT FROM users WHERE USERNAME = ?");
salt_check.addBindValue(username);
salt_check.exec();
if (!salt_check.first())
return false;
else
salt = salt_check.value(0).toString();
QByteArray salt = CryptoHelper::randbytes(16);
QString salted_password = CryptoHelper::hash_password(salt, password);
QSqlQuery query;
QString salted_password;
QMessageAuthenticationCode hmac(QCryptographicHash::Sha256);
hmac.setKey(salt.toUtf8());
hmac.addData(password.toUtf8());
salted_password = hmac.result().toHex();
query.prepare("UPDATE users SET PASSWORD = ? WHERE USERNAME = ?");
query.prepare("UPDATE users SET PASSWORD = ?, SALT = ? WHERE USERNAME = ?");
query.addBindValue(salted_password);
query.addBindValue(salt.toHex());
query.addBindValue(username);
query.exec();
return true;

3
tests/tests.pro
View File

@ -5,4 +5,5 @@ SUBDIRS += \
unittest_music_manager \
unittest_acl_roles_handler \
unittest_command_extension \
unittest_aopacket
unittest_aopacket \
unittest_crypto

47
tests/unittest_crypto/tst_unittest_crypto.cpp Normal file
View File

@ -0,0 +1,47 @@
#include <QTest>
#include <include/crypto_helper.h>
namespace tests {
namespace unittests {
class tst_Crypto : public QObject
{
Q_OBJECT
public:
private slots:
void checkHash();
void checkHash_data();
};
void tst_Crypto::checkHash_data()
{
QTest::addColumn<QString>("password");
QTest::addColumn<QString>("salt_hex");
QTest::addColumn<QString>("expected_hash");
QTest::newRow("HMAC only (old algorithm)") << "password"
<< "73616c7473616c74"
<< "4128058d074264779ec23ee1ffed65d7c4e16c93003315cd5cb85170770b254a";
QTest::newRow("PBKDF2 (new algorithm)") << "password"
<< "73616c7473616c7473616c7473616c74"
<< "4fbf2d122fe6afc61a81e9f2fe393ab39f906a78ddddc797763c0e784857e9b4";
}
void tst_Crypto::checkHash()
{
QFETCH(QString, password);
QFETCH(QString, salt_hex);
QFETCH(QString, expected_hash);
QCOMPARE(CryptoHelper::hash_password(QByteArray::fromHex(salt_hex.toUtf8()), password), expected_hash);
}
}
}
QTEST_APPLESS_MAIN(tests::unittests::tst_Crypto)
#include "tst_unittest_crypto.moc"

5
tests/unittest_crypto/unittest_crypto.pro Normal file
View File

@ -0,0 +1,5 @@
QT -= gui
include(../tests_common.pri)
SOURCES += tst_unittest_crypto.cpp