From 555b4a0cbf92e17f0a57bd15d0049e15e6742168 Mon Sep 17 00:00:00 2001 From: MangosArentLiterature <58055358+MangosArentLiterature@users.noreply.github.com> Date: Wed, 9 Jun 2021 23:15:06 -0500 Subject: [PATCH 1/5] Implement remote banning - Use IPID for IP bans instead of remote IP. - Remove 2 extraneous DB queries by altering isIPBanned() - Allow banning unconnected clients --- core/include/db_manager.h | 10 ++++++---- core/src/commands/moderation.cpp | 8 ++++++-- core/src/db_manager.cpp | 19 ++++++++++--------- core/src/packets.cpp | 9 ++++++--- core/src/server.cpp | 8 +++++--- 5 files changed, 33 insertions(+), 21 deletions(-) diff --git a/core/include/db_manager.h b/core/include/db_manager.h index 02c56e5..1f340ee 100644 --- a/core/include/db_manager.h +++ b/core/include/db_manager.h @@ -56,13 +56,15 @@ public: ~DBManager(); /** - * @brief Checks if there is a record in the Bans table with the given IP address. + * @brief Checks if there is a record in the Bans table with the given IPID. * - * @param ip The IP address to check if it is banned. + * @param ipid The IPID to check if it is banned. * - * @return True if the query could return at least one such record. + * @return A pair of values: + * * First, a `bool` that is true if the query could return at least one such record. + * * Then, a `QString` that is the reason for the ban. */ - bool isIPBanned(QHostAddress ip); + std::pair isIPBanned(QString ipid); /** * @brief Checks if there is a record in the Bans table with the given hardware ID. diff --git a/core/src/commands/moderation.cpp b/core/src/commands/moderation.cpp index 43c7f89..ebad420 100644 --- a/core/src/commands/moderation.cpp +++ b/core/src/commands/moderation.cpp @@ -82,8 +82,12 @@ void AOClient::cmdBan(int argc, QStringList argv) if (kick_counter > 1) sendServerMessage("Kicked " + QString::number(kick_counter) + " clients with matching ipids."); - if (!ban_logged) - sendServerMessage("User with ipid not found!"); + + // We're banning someone not connected. + if (!ban_logged) { + server->db_manager->addBan(ban); + sendServerMessage("Banned " + ban.ipid + " for reason: " + ban.reason); + } } void AOClient::cmdKick(int argc, QStringList argv) diff --git a/core/src/db_manager.cpp b/core/src/db_manager.cpp index 7a3ad16..3cd4fd7 100644 --- a/core/src/db_manager.cpp +++ b/core/src/db_manager.cpp @@ -31,23 +31,24 @@ DBManager::DBManager() : updateDB(db_version); } -bool DBManager::isIPBanned(QHostAddress ip) +std::pair DBManager::isIPBanned(QString ipid) { QSqlQuery query; - query.prepare("SELECT TIME FROM BANS WHERE IP = ? ORDER BY TIME DESC"); - query.addBindValue(ip.toString()); + query.prepare("SELECT * FROM BANS WHERE IPID = ? ORDER BY TIME DESC"); + query.addBindValue(ipid); query.exec(); if (query.first()) { - long long duration = getBanDuration(ip); - long long ban_time = query.value(0).toLongLong(); + long long ban_time = query.value(4).toLongLong(); + QString reason = query.value(5).toString(); + long long duration = query.value(6).toLongLong(); if (duration == -2) - return true; + return {true, reason}; long long current_time = QDateTime::currentDateTime().toSecsSinceEpoch(); if (ban_time + duration > current_time) - return true; - else return false; + return {true, reason}; + else return {false, nullptr}; } - else return false; + else return {false, nullptr}; } bool DBManager::isHDIDBanned(QString hdid) diff --git a/core/src/packets.cpp b/core/src/packets.cpp index 4c4e0d0..0f62caa 100644 --- a/core/src/packets.cpp +++ b/core/src/packets.cpp @@ -315,9 +315,13 @@ void AOClient::pktWebSocketIp(AreaData* area, int argc, QStringList argv, AOPack { // Special packet to set remote IP from the webao proxy // Only valid if from a local ip + calculateIpid(); if (remote_ip.isLoopback()) { - if(server->db_manager->isIPBanned(QHostAddress(argv[0]))) { - sendPacket("BD", {server->db_manager->getBanReason(QHostAddress(argv[0]))}); + auto ban = server->db_manager->isIPBanned(ipid); + bool is_banned = ban.first; + if(is_banned) { + QString reason = ban.second; + sendPacket("BD", {reason}); socket->close(); return; } @@ -325,7 +329,6 @@ void AOClient::pktWebSocketIp(AreaData* area, int argc, QStringList argv, AOPack qDebug() << "ws ip set to" << argv[0]; #endif remote_ip = QHostAddress(argv[0]); - calculateIpid(); int multiclient_count = 0; for (AOClient* joined_client : server->clients) { diff --git a/core/src/server.cpp b/core/src/server.cpp index 5462609..87fe428 100644 --- a/core/src/server.cpp +++ b/core/src/server.cpp @@ -125,7 +125,9 @@ void Server::clientConnected() int multiclient_count = 1; bool is_at_multiclient_limit = false; - bool is_banned = db_manager->isIPBanned(socket->peerAddress()); + client->calculateIpid(); + auto ban = db_manager->isIPBanned(client->getIpid()); + bool is_banned = ban.first; for (AOClient* joined_client : clients) { if (client->remote_ip.isEqual(joined_client->remote_ip)) multiclient_count++; @@ -135,7 +137,8 @@ void Server::clientConnected() is_at_multiclient_limit = true; if (is_banned) { - AOPacket ban_reason("BD", {db_manager->getBanReason(socket->peerAddress())}); + QString reason = ban.second; + AOPacket ban_reason("BD", {reason}); socket->write(ban_reason.toUtf8()); } if (is_banned || is_at_multiclient_limit) { @@ -158,7 +161,6 @@ void Server::clientConnected() // tsuserver4. It should disable fantacrypt // completely in any client 2.4.3 or newer client->sendPacket(decryptor); - client->calculateIpid(); #ifdef NET_DEBUG qDebug() << client->remote_ip.toString() << "connected"; #endif From a561d3eb47eb3e56eac2314bce01b5ec7af8be85 Mon Sep 17 00:00:00 2001 From: MangosArentLiterature <58055358+MangosArentLiterature@users.noreply.github.com> Date: Wed, 9 Jun 2021 23:32:55 -0500 Subject: [PATCH 2/5] Mirror isIPBanned() changes to isHDIDBanned() this also removes getBanReason and getBanDuration as they are now no longer needed also minor clean up this websocket ip ban or whatever --- core/include/db_manager.h | 36 +++---------------- core/src/db_manager.cpp | 74 +++++---------------------------------- core/src/packets.cpp | 11 +++--- 3 files changed, 18 insertions(+), 103 deletions(-) diff --git a/core/include/db_manager.h b/core/include/db_manager.h index 1f340ee..1bc71e7 100644 --- a/core/include/db_manager.h +++ b/core/include/db_manager.h @@ -71,39 +71,11 @@ public: * * @param hdid The hardware ID to check if it is banned. * - * @return True if the query could return at least one such record. + * @return A pair of values: + * * First, a `bool` that is true if the query could return at least one such record. + * * Then, a `QString` that is the reason for the ban. */ - bool isHDIDBanned(QString hdid); - - /** - * @brief Returns the reason the given IP address was banned with. - * - * @param ip The IP address whose ban reason needs to be returned. - * - * @return The ban reason if the IP address is actually banned, - * or `"Ban reason not found."` if the IP address is not actually banned. - */ - QString getBanReason(QHostAddress ip); - - /** - * @overload - */ - QString getBanReason(QString hdid); - - /** - * @brief Returns the reason the given IP address was banned with. - * - * @param ip The IP address whose ban duration to get. - * - * @return The ban duration if the IP address is actually banned, - * or `-1` if the IP address is not actually banned. - */ - long long getBanDuration(QHostAddress ip); - - /** - * @overload - */ - long long getBanDuration(QString hdid); + std::pair isHDIDBanned(QString hdid); /** * @brief Gets the ID number of a given ban. diff --git a/core/src/db_manager.cpp b/core/src/db_manager.cpp index 3cd4fd7..0ce30aa 100644 --- a/core/src/db_manager.cpp +++ b/core/src/db_manager.cpp @@ -51,82 +51,26 @@ std::pair DBManager::isIPBanned(QString ipid) else return {false, nullptr}; } -bool DBManager::isHDIDBanned(QString hdid) +std::pair DBManager::isHDIDBanned(QString hdid) { QSqlQuery query; - query.prepare("SELECT TIME FROM BANS WHERE HDID = ? ORDER BY TIME DESC"); + query.prepare("SELECT * FROM BANS WHERE HDID = ? ORDER BY TIME DESC"); query.addBindValue(hdid); query.exec(); if (query.first()) { - long long duration = getBanDuration(hdid); - long long ban_time = query.value(0).toLongLong(); + long long ban_time = query.value(4).toLongLong(); + QString reason = query.value(5).toString(); + long long duration = query.value(6).toLongLong(); if (duration == -2) - return true; + return {true, reason}; long long current_time = QDateTime::currentDateTime().toSecsSinceEpoch(); if (ban_time + duration > current_time) - return true; - else return false; + return {true, reason}; + else return {false, nullptr}; } - else return false; + else return {false, nullptr}; } -QString DBManager::getBanReason(QHostAddress ip) -{ - QSqlQuery query; - query.prepare("SELECT REASON FROM BANS WHERE IP = ? ORDER BY TIME DESC"); - query.addBindValue(ip.toString()); - query.exec(); - if (query.first()) { - return query.value(0).toString(); - } - else { - return "Ban reason not found."; - } -} - -QString DBManager::getBanReason(QString hdid) -{ - QSqlQuery query; - query.prepare("SELECT REASON FROM BANS WHERE HDID = ? ORDER BY TIME DESC"); - query.addBindValue(hdid); - query.exec(); - if (query.first()) { - return query.value(0).toString(); - } - else { - return "Ban reason not found."; - } -} - -long long DBManager::getBanDuration(QString hdid) -{ - QSqlQuery query; - query.prepare("SELECT DURATION FROM BANS WHERE HDID = ? ORDER BY TIME DESC"); - query.addBindValue(hdid); - query.exec(); - if (query.first()) { - return query.value(0).toLongLong(); - } - else { - return -1; - } -} - -long long DBManager::getBanDuration(QHostAddress ip) -{ - QSqlQuery query; - query.prepare("SELECT DURATION FROM BANS WHERE IP = ? ORDER BY TIME DESC"); - query.addBindValue(ip.toString()); - query.exec(); - if (query.first()) { - return query.value(0).toLongLong(); - } - else { - return -1; - } -} - - int DBManager::getBanID(QString hdid) { QSqlQuery query; diff --git a/core/src/packets.cpp b/core/src/packets.cpp index 0f62caa..392b3dc 100644 --- a/core/src/packets.cpp +++ b/core/src/packets.cpp @@ -27,8 +27,9 @@ void AOClient::pktDefault(AreaData* area, int argc, QStringList argv, AOPacket p void AOClient::pktHardwareId(AreaData* area, int argc, QStringList argv, AOPacket packet) { hwid = argv[0]; - if(server->db_manager->isHDIDBanned(hwid)) { - sendPacket("BD", {server->db_manager->getBanReason(hwid) + "\nBan ID: " + QString::number(server->db_manager->getBanID(hwid))}); + auto ban = server->db_manager->isHDIDBanned(hwid); + if (ban.first) { + sendPacket("BD", {ban.second + "\nBan ID: " + QString::number(server->db_manager->getBanID(hwid))}); socket->close(); return; } @@ -318,10 +319,8 @@ void AOClient::pktWebSocketIp(AreaData* area, int argc, QStringList argv, AOPack calculateIpid(); if (remote_ip.isLoopback()) { auto ban = server->db_manager->isIPBanned(ipid); - bool is_banned = ban.first; - if(is_banned) { - QString reason = ban.second; - sendPacket("BD", {reason}); + if (ban.first) { + sendPacket("BD", {ban.second}); socket->close(); return; } From 9896feb1ab3e34ad4851160ee1a9be7b3a2171ff Mon Sep 17 00:00:00 2001 From: MangosArentLiterature <58055358+MangosArentLiterature@users.noreply.github.com> Date: Sun, 13 Jun 2021 21:07:38 -0500 Subject: [PATCH 3/5] Fix localhost being used for webAO IPIDs this was really dumb of me --- core/src/packets.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/src/packets.cpp b/core/src/packets.cpp index 392b3dc..f2512d1 100644 --- a/core/src/packets.cpp +++ b/core/src/packets.cpp @@ -316,18 +316,18 @@ void AOClient::pktWebSocketIp(AreaData* area, int argc, QStringList argv, AOPack { // Special packet to set remote IP from the webao proxy // Only valid if from a local ip - calculateIpid(); if (remote_ip.isLoopback()) { +#ifdef NET_DEBUG + qDebug() << "ws ip set to" << argv[0]; +#endif + remote_ip = QHostAddress(argv[0]); + calculateIpid(); auto ban = server->db_manager->isIPBanned(ipid); if (ban.first) { sendPacket("BD", {ban.second}); socket->close(); return; } -#ifdef NET_DEBUG - qDebug() << "ws ip set to" << argv[0]; -#endif - remote_ip = QHostAddress(argv[0]); int multiclient_count = 0; for (AOClient* joined_client : server->clients) { From ef3103a87ff60b7aeab4296b0197c80021c5d15d Mon Sep 17 00:00:00 2001 From: MangosArentLiterature <58055358+MangosArentLiterature@users.noreply.github.com> Date: Mon, 21 Jun 2021 22:14:37 -0500 Subject: [PATCH 4/5] Replace all std::pair with QPair --- core/include/area_data.h | 2 +- core/include/db_manager.h | 4 ++-- core/src/area_data.cpp | 2 +- core/src/db_manager.cpp | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/include/area_data.h b/core/include/area_data.h index ca436e5..94bac98 100644 --- a/core/include/area_data.h +++ b/core/include/area_data.h @@ -696,7 +696,7 @@ class AreaData : public QObject { * * First, a `QStringList` that is the packet of the statement that was advanced to. * * Then, a `TestimonyProgress` value that describes how the advancement happened. */ - std::pair jumpToStatement(int f_position); + QPair jumpToStatement(int f_position); /** * @brief Returns a copy of the judgelog in the area. diff --git a/core/include/db_manager.h b/core/include/db_manager.h index 1bc71e7..fcc5f4a 100644 --- a/core/include/db_manager.h +++ b/core/include/db_manager.h @@ -64,7 +64,7 @@ public: * * First, a `bool` that is true if the query could return at least one such record. * * Then, a `QString` that is the reason for the ban. */ - std::pair isIPBanned(QString ipid); + QPair isIPBanned(QString ipid); /** * @brief Checks if there is a record in the Bans table with the given hardware ID. @@ -75,7 +75,7 @@ public: * * First, a `bool` that is true if the query could return at least one such record. * * Then, a `QString` that is the reason for the ban. */ - std::pair isHDIDBanned(QString hdid); + QPair isHDIDBanned(QString hdid); /** * @brief Gets the ID number of a given ban. diff --git a/core/src/area_data.cpp b/core/src/area_data.cpp index 38daf76..fdd7b16 100644 --- a/core/src/area_data.cpp +++ b/core/src/area_data.cpp @@ -403,7 +403,7 @@ void AreaData::removeStatement(int f_position) --m_statement; } -std::pair AreaData::jumpToStatement(int f_position) +QPair AreaData::jumpToStatement(int f_position) { m_statement = f_position; diff --git a/core/src/db_manager.cpp b/core/src/db_manager.cpp index 0ce30aa..240f20c 100644 --- a/core/src/db_manager.cpp +++ b/core/src/db_manager.cpp @@ -31,7 +31,7 @@ DBManager::DBManager() : updateDB(db_version); } -std::pair DBManager::isIPBanned(QString ipid) +QPair DBManager::isIPBanned(QString ipid) { QSqlQuery query; query.prepare("SELECT * FROM BANS WHERE IPID = ? ORDER BY TIME DESC"); @@ -51,7 +51,7 @@ std::pair DBManager::isIPBanned(QString ipid) else return {false, nullptr}; } -std::pair DBManager::isHDIDBanned(QString hdid) +QPair DBManager::isHDIDBanned(QString hdid) { QSqlQuery query; query.prepare("SELECT * FROM BANS WHERE HDID = ? ORDER BY TIME DESC"); From 720669679a4f2482b78f55bbb811437ae6d9fe3f Mon Sep 17 00:00:00 2001 From: MangosArentLiterature <58055358+MangosArentLiterature@users.noreply.github.com> Date: Mon, 21 Jun 2021 22:22:02 -0500 Subject: [PATCH 5/5] Select only needed values from the DB --- core/src/db_manager.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/core/src/db_manager.cpp b/core/src/db_manager.cpp index 240f20c..d8e2ace 100644 --- a/core/src/db_manager.cpp +++ b/core/src/db_manager.cpp @@ -34,13 +34,13 @@ DBManager::DBManager() : QPair DBManager::isIPBanned(QString ipid) { QSqlQuery query; - query.prepare("SELECT * FROM BANS WHERE IPID = ? ORDER BY TIME DESC"); + query.prepare("SELECT TIME,REASON,DURATION FROM BANS WHERE IPID = ? ORDER BY TIME DESC"); query.addBindValue(ipid); query.exec(); if (query.first()) { - long long ban_time = query.value(4).toLongLong(); - QString reason = query.value(5).toString(); - long long duration = query.value(6).toLongLong(); + long long ban_time = query.value(0).toLongLong(); + QString reason = query.value(1).toString(); + long long duration = query.value(2).toLongLong(); if (duration == -2) return {true, reason}; long long current_time = QDateTime::currentDateTime().toSecsSinceEpoch(); @@ -54,13 +54,13 @@ QPair DBManager::isIPBanned(QString ipid) QPair DBManager::isHDIDBanned(QString hdid) { QSqlQuery query; - query.prepare("SELECT * FROM BANS WHERE HDID = ? ORDER BY TIME DESC"); + query.prepare("SELECT TIME,REASON,DURATION FROM BANS WHERE HDID = ? ORDER BY TIME DESC"); query.addBindValue(hdid); query.exec(); if (query.first()) { - long long ban_time = query.value(4).toLongLong(); - QString reason = query.value(5).toString(); - long long duration = query.value(6).toLongLong(); + long long ban_time = query.value(0).toLongLong(); + QString reason = query.value(1).toString(); + long long duration = query.value(2).toLongLong(); if (duration == -2) return {true, reason}; long long current_time = QDateTime::currentDateTime().toSecsSinceEpoch();