From 3e1e7f7f9213dccea642615f4c670c1065383903 Mon Sep 17 00:00:00 2001 From: Mothblocks <35135081+Mothblocks@users.noreply.github.com> Date: Wed, 9 Jun 2021 17:07:36 -0700 Subject: [PATCH] 2FA for Admins (#59467) Adds a 2FA system for admins. When a config is enabled, a database storing your last CID and IP will be checked to see if you have connected through there before. If not, you will not be able to admin until you are verified. Verification is done by opening a website (decided by config), which is tasked with updating the database entry, which will then let you reverify. MSO wants to implement this as a forum page, but the DM-side is completely agnostic to whatever the implementation is, just that it updates the database. This means that it could potentially be some TOTP stuff, even. If the database is down, a backup file is checked for your most recent verified connection (IP + CID). If you are on this connection, you will be able to connect fine. If the database is down and you are on a new connection, someone with +PERMISSIONS can manually verify you through the Permissions Panel. We've had repeated attacks of admins getting their accounts stolen, it's time. --- SQL/database_changelog.txt | 20 +- SQL/tgstation_schema.sql | 14 ++ SQL/tgstation_schema_prefixed.sql | 14 ++ code/__DEFINES/subsystems.dm | 2 +- code/__HELPERS/roundend.dm | 28 ++- .../configuration/entries/general.dm | 3 + code/modules/admin/admin_verbs.dm | 7 + code/modules/admin/holder2.dm | 208 ++++++++++++++++-- code/modules/admin/permissionedit.dm | 21 +- code/modules/client/client_procs.dm | 7 +- config/admins.txt | 1 + config/config.txt | 5 + 12 files changed, 298 insertions(+), 32 deletions(-) diff --git a/SQL/database_changelog.txt b/SQL/database_changelog.txt index ae362c2eb76..80087fc2e7a 100644 --- a/SQL/database_changelog.txt +++ b/SQL/database_changelog.txt @@ -2,12 +2,28 @@ Any time you make a change to the schema files, remember to increment the databa The latest database version is 5.14; The query to update the schema revision table is: -INSERT INTO `schema_revision` (`major`, `minor`) VALUES (5, 14); +INSERT INTO `schema_revision` (`major`, `minor`) VALUES (5, 15); or -INSERT INTO `SS13_schema_revision` (`major`, `minor`) VALUES (5, 14); +INSERT INTO `SS13_schema_revision` (`major`, `minor`) VALUES (5, 15); In any query remember to add a prefix to the table names if you use one. +----------------------------------------------------- +Version 5.15, 2 June 2021, by Mothblocks +Added verified admin connection log used for 2FA + +``` +DROP TABLE IF EXISTS `admin_connections`; +CREATE TABLE `admin_connections` ( + `id` INT NOT NULL AUTO_INCREMENT, + `ckey` VARCHAR(32) NOT NULL, + `ip` INT(11) UNSIGNED NOT NULL, + `cid` VARCHAR(32) NOT NULL, + `verification_time` DATETIME NULL, + PRIMARY KEY (`id`), + UNIQUE INDEX `unique_constraints` (`ckey`, `ip`, `cid`)); +``` + ----------------------------------------------------- Version 5.14, xx May 2021, by Anturke diff --git a/SQL/tgstation_schema.sql b/SQL/tgstation_schema.sql index bbb759bb828..dae359cb0c4 100644 --- a/SQL/tgstation_schema.sql +++ b/SQL/tgstation_schema.sql @@ -630,6 +630,20 @@ CREATE TABLE `text_adventures` ( PRIMARY KEY (`id`) ) ENGINE=InnoDB; +-- +-- Table structure for table `admin_connections` +-- +DROP TABLE IF EXISTS `admin_connections`; +CREATE TABLE `admin_connections` ( + `id` INT NOT NULL AUTO_INCREMENT, + `ckey` VARCHAR(32) NOT NULL, + `ip` INT(11) UNSIGNED NOT NULL, + `cid` VARCHAR(32) NOT NULL, + `verification_time` DATETIME NULL, + PRIMARY KEY (`id`), + UNIQUE INDEX `unique_constraints` (`ckey`, `ip`, `cid`) +) ENGINE=InnoDB; + /*!40101 SET SQL_MODE=@OLD_SQL_MODE */; /*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */; /*!40014 SET UNIQUE_CHECKS=@OLD_UNIQUE_CHECKS */; diff --git a/SQL/tgstation_schema_prefixed.sql b/SQL/tgstation_schema_prefixed.sql index 564813ed446..c2686d31172 100644 --- a/SQL/tgstation_schema_prefixed.sql +++ b/SQL/tgstation_schema_prefixed.sql @@ -630,6 +630,20 @@ CREATE TABLE `SS13_text_adventures` ( PRIMARY KEY (`id`) ) ENGINE=InnoDB; +-- +-- Table structure for table `admin_connections` +-- +DROP TABLE IF EXISTS `SS13_admin_connections`; +CREATE TABLE `SS13_admin_connections` ( + `id` INT NOT NULL AUTO_INCREMENT, + `ckey` VARCHAR(32) NOT NULL, + `ip` INT(11) UNSIGNED NOT NULL, + `cid` VARCHAR(32) NOT NULL, + `verification_time` DATETIME NULL, + PRIMARY KEY (`id`), + UNIQUE INDEX `unique_constraints` (`ckey`, `ip`, `cid`) +) ENGINE=InnoDB; + /*!40101 SET SQL_MODE=@OLD_SQL_MODE */; /*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */; /*!40014 SET UNIQUE_CHECKS=@OLD_UNIQUE_CHECKS */; diff --git a/code/__DEFINES/subsystems.dm b/code/__DEFINES/subsystems.dm index cd927271428..69b6fa71ca8 100644 --- a/code/__DEFINES/subsystems.dm +++ b/code/__DEFINES/subsystems.dm @@ -20,7 +20,7 @@ * * make sure you add an update to the schema_version stable in the db changelog */ -#define DB_MINOR_VERSION 14 +#define DB_MINOR_VERSION 15 //! ## Timing subsystem diff --git a/code/__HELPERS/roundend.dm b/code/__HELPERS/roundend.dm index abc9b01ea5a..2be53ab799c 100644 --- a/code/__HELPERS/roundend.dm +++ b/code/__HELPERS/roundend.dm @@ -738,19 +738,33 @@ //json format backup file generation stored per server var/json_file = file("data/admins_backup.json") - var/list/file_data = list("ranks" = list(), "admins" = list()) + var/list/file_data = list( + "ranks" = list(), + "admins" = list(), + "connections" = list(), + ) for(var/datum/admin_rank/R in GLOB.admin_ranks) file_data["ranks"]["[R.name]"] = list() file_data["ranks"]["[R.name]"]["include rights"] = R.include_rights file_data["ranks"]["[R.name]"]["exclude rights"] = R.exclude_rights file_data["ranks"]["[R.name]"]["can edit rights"] = R.can_edit_rights - for(var/i in GLOB.admin_datums+GLOB.deadmins) - var/datum/admins/A = GLOB.admin_datums[i] - if(!A) - A = GLOB.deadmins[i] - if (!A) + + for(var/admin_ckey in GLOB.admin_datums + GLOB.deadmins) + var/datum/admins/admin = GLOB.admin_datums[admin_ckey] + + if(!admin) + admin = GLOB.deadmins[admin_ckey] + if (!admin) continue - file_data["admins"]["[i]"] = A.rank.name + + file_data["admins"][admin_ckey] = admin.rank.name + + if (admin.owner) + file_data["connections"][admin_ckey] = list( + "cid" = admin.owner.computer_id, + "ip" = admin.owner.address, + ) + fdel(json_file) WRITE_FILE(json_file, json_encode(file_data)) diff --git a/code/controllers/configuration/entries/general.dm b/code/controllers/configuration/entries/general.dm index c6d70cecc1e..f52a6cbd42c 100644 --- a/code/controllers/configuration/entries/general.dm +++ b/code/controllers/configuration/entries/general.dm @@ -509,3 +509,6 @@ /datum/config_entry/string/centcom_ban_db // URL for the CentCom Galactic Ban DB API /datum/config_entry/string/centcom_source_whitelist + +/// URL for admins to be redirected to for 2FA +/datum/config_entry/string/admin_2fa_url diff --git a/code/modules/admin/admin_verbs.dm b/code/modules/admin/admin_verbs.dm index 949e70ad6d5..6ca2c1f03de 100644 --- a/code/modules/admin/admin_verbs.dm +++ b/code/modules/admin/admin_verbs.dm @@ -803,3 +803,10 @@ GLOBAL_PROTECT(admin_verbs_hideable) set category = "Debug" src << output("", "statbrowser:create_debug") + +/client/proc/admin_2fa_verify() + set name = "Verify Admin" + set category = "Admin" + + var/datum/admins/admin = GLOB.admin_datums[ckey] + admin?.associate(src) diff --git a/code/modules/admin/holder2.dm b/code/modules/admin/holder2.dm index 43409363f0a..660fc225653 100644 --- a/code/modules/admin/holder2.dm +++ b/code/modules/admin/holder2.dm @@ -6,6 +6,9 @@ GLOBAL_PROTECT(protected_admins) GLOBAL_VAR_INIT(href_token, GenerateToken()) GLOBAL_PROTECT(href_token) +#define RESULT_2FA_VALID 1 +#define RESULT_2FA_ID 2 + /datum/admins var/datum/admin_rank/rank @@ -30,6 +33,12 @@ GLOBAL_PROTECT(href_token) var/datum/filter_editor/filteriffic + /// Whether or not the user tried to connect, but was blocked by 2FA + var/blocked_by_2fa = FALSE + + /// Whether or not this user can bypass 2FA + var/bypass_2fa = FALSE + /datum/admins/New(datum/admin_rank/R, ckey, force_active = FALSE, protected) if(IsAdminAdvancedProcCall()) var/msg = " has tried to elevate permissions!" @@ -95,27 +104,45 @@ GLOBAL_PROTECT(href_token) disassociate() add_verb(C, /client/proc/readmin) -/datum/admins/proc/associate(client/C) +/datum/admins/proc/associate(client/client) if(IsAdminAdvancedProcCall()) var/msg = " has tried to elevate permissions!" message_admins("[key_name_admin(usr)][msg]") log_admin("[key_name(usr)][msg]") return - if(istype(C)) - if(C.ckey != target) - var/msg = " has attempted to associate with [target]'s admin datum" - message_admins("[key_name_admin(C)][msg]") - log_admin("[key_name(C)][msg]") - return - if (deadmined) - activate() - owner = C - owner.holder = src - owner.add_admin_verbs() //TODO <--- todo what? the proc clearly exists and works since its the backbone to our entire admin system - remove_verb(owner, /client/proc/readmin) - owner.init_verbs() //re-initialize the verb list - GLOB.admins |= C + if(!istype(client)) + return + + if(client?.ckey != target) + var/msg = " has attempted to associate with [target]'s admin datum" + message_admins("[key_name_admin(client)][msg]") + log_admin("[key_name(client)][msg]") + return + + var/result_2fa = check_2fa(client) + if (!result_2fa[RESULT_2FA_VALID]) + blocked_by_2fa = TRUE + alert_2fa_necessary(client) + start_2fa_process(client, result_2fa[RESULT_2FA_ID]) + + return + else if (blocked_by_2fa) + sync_lastadminrank(client.ckey, client.key) + + blocked_by_2fa = FALSE + + if (deadmined) + activate() + + remove_verb(client, /client/proc/admin_2fa_verify) + + owner = client + owner.holder = src + owner.add_admin_verbs() + remove_verb(owner, /client/proc/readmin) + owner.init_verbs() //re-initialize the verb list + GLOB.admins |= client /datum/admins/proc/disassociate() if(IsAdminAdvancedProcCall()) @@ -148,6 +175,154 @@ GLOBAL_PROTECT(href_token) return TRUE //we have all the rights they have and more return FALSE +// TRUE for a vaild connection, null is the id (it is unnecessary) +#define VALID_2FA_CONNECTION list(TRUE, null) + +/// Returns whether or not the given client has a verified 2FA connection. +/// The output is in the form of a list with the first index being whether or not the +/// check was successful, the 2nd is the ID of the associated database entry +/// if its a false result and if one can be found. +/datum/admins/proc/check_2fa(client/client) + if (bypass_2fa) + return VALID_2FA_CONNECTION + + var/admin_2fa_url = CONFIG_GET(string/admin_2fa_url) + + // 2FA not being enabled == everyone passes + if (isnull(admin_2fa_url) || admin_2fa_url == "") + return VALID_2FA_CONNECTION + + // I believe this is only in the case of Dream Seeker. + if (isnull(client?.address)) + return VALID_2FA_CONNECTION + + if (!SSdbcore.Connect()) + if (verify_backup_data(client)) + return VALID_2FA_CONNECTION + else + return list(FALSE, null) + + var/datum/db_query/query = SSdbcore.NewQuery({" + SELECT id, verification_time FROM [format_table_name("admin_connections")] + WHERE ckey = :ckey + AND ip = INET_ATON(:ip) + AND cid = :cid + "}, list( + "ckey" = client.ckey, + "ip" = client.address, + "cid" = client.computer_id, + )) + + if (!query.Execute()) + qdel(query) + return list(FALSE, null) + + var/is_valid = FALSE + var/id = null + + if (query.NextRow()) + id = query.item[1] + is_valid = !isnull(query.item[2]) + + qdel(query) + return list(is_valid, id) + +#undef VALID_2FA_CONNECTION + +#define ERROR_2FA_REQUEST_PERMISSIONS "