From 1fbb174357433391b07fc42c5fecf7bd739ab29a Mon Sep 17 00:00:00 2001 From: "Rob Swindell (on Windows 11)" <rob@synchro.net> Date: Thu, 10 Apr 2025 23:08:10 -0700 Subject: [PATCH] Move password quality check logic to new userdat.c function: check_pass() sbbs_t::chkpass() now becomes just a thin wrapper around check_pass() and it prints the reason for the failure or calls sbbs_t::trashcan(). I also refactored the code quite a bit: no more copying and uppercasing (we have strcasestr() now!) and supports calling with a NULL user_t* as I expect will be a use case. This will allow us to expose the password quality checking algorithm to other servers (e.g. the web server) that can be used to create new user accounts with passwords that meet our quality bar. e.g. via a newly created JS method: system.check_password() Yes, we have bbs.good_password() already (and that still works fine), but can't be used by non-terminal server code. --- src/sbbs3/str.cpp | 84 +++++--------------------------------------- src/sbbs3/userdat.c | 85 +++++++++++++++++++++++++++++++++++++++++++++ src/sbbs3/userdat.h | 1 + 3 files changed, 94 insertions(+), 76 deletions(-) diff --git a/src/sbbs3/str.cpp b/src/sbbs3/str.cpp index fff7567b47..d7124dd3ab 100644 --- a/src/sbbs3/str.cpp +++ b/src/sbbs3/str.cpp @@ -843,86 +843,18 @@ bool sbbs_t::inputnstime(time_t *dt) return true; } -/*****************************************************************************/ -/* Checks a password for uniqueness and validity */ -/*****************************************************************************/ +/****************************************************************************/ +/* Check a password for validity and print reason upon failure */ +/****************************************************************************/ bool sbbs_t::chkpass(char *passwd, user_t* user, bool unique) { - char first[128], last[128], sysop[41], sysname[41], *p; - char alias[LEN_ALIAS + 1], name[LEN_NAME + 1], handle[LEN_HANDLE + 1]; - char pass[LEN_PASS + 1]; - - SAFECOPY(pass, passwd); - strupr(pass); - - int len = strlen(pass); - if (len < cfg.min_pwlen || len < MIN_PASS_LEN) { - bputs(text[PasswordTooShort]); - return false; - } - if (unique && strcmp(pass, user->pass) == 0) { - bputs(text[PasswordNotChanged]); - return false; - } - int i; - int run = 0; - for (i = 0; i < (len - 1); ++i) { - if (abs(toupper(pass[i]) - toupper(pass[i + 1])) > 1) { - if (++run >= cfg.min_pwlen / 2) - break; - } else - run = 0; - } - if (i >= (len - 1)) { - bputs(text[PasswordInvalid]); - return false; - } - SAFECOPY(name, user->name); - strupr(name); - SAFECOPY(alias, user->alias); - strupr(alias); - SAFECOPY(first, alias); - p = strchr(first, ' '); - if (p) { - *p = 0; - SAFECOPY(last, p + 1); - } - else - last[0] = 0; - SAFECOPY(handle, user->handle); - strupr(handle); - SAFECOPY(sysop, cfg.sys_op); - strupr(sysop); - SAFECOPY(sysname, cfg.sys_name); - strupr(sysname); - if ((unique && user->pass[0] - && (strstr(pass, user->pass) || strstr(user->pass, pass))) - || (name[0] - && (strstr(pass, name) || strstr(name, pass))) - || strstr(pass, alias) || strstr(alias, pass) - || strstr(pass, first) || strstr(first, pass) - || (last[0] - && (strstr(pass, last) || strstr(last, pass))) - || strstr(pass, handle) || strstr(handle, pass) - || (user->zipcode[0] - && (strstr(pass, user->zipcode) || strstr(user->zipcode, pass))) - || (sysname[0] - && (strstr(pass, sysname) || strstr(sysname, pass))) - || (sysop[0] - && (strstr(pass, sysop) || strstr(sysop, pass))) - || (cfg.sys_id[0] - && (strstr(pass, cfg.sys_id) || strstr(cfg.sys_id, pass))) - || (cfg.node_phone[0] && strstr(pass, cfg.node_phone)) - || (user->phone[0] && strstr(user->phone, pass)) - || !strncmp(pass, "QWER", 4) - || !strncmp(pass, "ASDF", 4) - || !strncmp(pass, "!@#$", 4) - ) - { - bputs(text[PasswordObvious]); + int reason = -1; + if (!check_pass(&cfg, passwd, user, unique, &reason)) { + if (reason >= 0) + bputs(text[reason]); return false; } - return !trashcan(pass, "password"); + return !trashcan(passwd, "password"); } /****************************************************************************/ diff --git a/src/sbbs3/userdat.c b/src/sbbs3/userdat.c index 662ac7634b..9b2b0e0955 100644 --- a/src/sbbs3/userdat.c +++ b/src/sbbs3/userdat.c @@ -4588,3 +4588,88 @@ enum parsed_vpath parse_vpath(scfg_t* cfg, const char* vpath, int* lib, int* dir return *filename == NULL ? PARSED_VPATH_DIR : PARSED_VPATH_FULL; } + +/****************************************************************************/ +/* Check a password for uniqueness and validity */ +/* Does *not* check the password.can file! */ +/****************************************************************************/ +bool check_pass(scfg_t* cfg, const char *pass, user_t* user, bool unique, int* reason) +{ + int reason_; + + if (reason == NULL) + reason = &reason_; + + int len = strlen(pass); + if (len < cfg->min_pwlen || len < MIN_PASS_LEN) { + *reason = PasswordTooShort; + return false; + } + if (unique) { + if (user == NULL) + return false; + if (stricmp(pass, user->pass) == 0) { + *reason = PasswordNotChanged; + return false; + } + } + + // Require a minimum sequence of unique (non-repeating/increment/decrementing) characters + int i; + int run = 0; + for (i = 0; i < (len - 1); ++i) { + if (abs(toupper(pass[i]) - toupper(pass[i + 1])) > 1) { + if (++run >= cfg->min_pwlen / 2) + break; + } else + run = 0; + } + if (i >= (len - 1)) { + *reason = PasswordInvalid; + return false; + } + + // Compare proposed password against user properties + if (user != NULL) { + char first[128], last[128], *p; + + SAFECOPY(first, user->alias); + p = strchr(first, ' '); + if (p) { + *p = 0; + SAFECOPY(last, p + 1); + } + else + last[0] = 0; + if ((unique && user->pass[0] + && (strcasestr(pass, user->pass) || strcasestr(user->pass, pass))) + || (user->name[0] + && (strcasestr(pass, user->name) || strcasestr(user->name, pass))) + || strcasestr(pass, user->alias) || strcasestr(user->alias, pass) + || strcasestr(pass, first) || strcasestr(first, pass) + || (last[0] + && (strcasestr(pass, last) || strcasestr(last, pass))) + || strcasestr(pass, user->handle) || strcasestr(user->handle, pass) + || (user->zipcode[0] + && (strcasestr(pass, user->zipcode) || strcasestr(user->zipcode, pass))) + || (user->phone[0] && strcasestr(user->phone, pass)) + ) { + *reason = PasswordObvious; + return false; + } + } + + // Compare proposed password against system properties + if ((cfg->sys_name[0] + && (strcasestr(pass, cfg->sys_name) || strcasestr(cfg->sys_name, pass))) + || (cfg->sys_op[0] + && (strcasestr(pass, cfg->sys_op) || strcasestr(cfg->sys_op, pass))) + || (cfg->sys_id[0] + && (strcasestr(pass, cfg->sys_id) || strcasestr(cfg->sys_id, pass))) + || (cfg->node_phone[0] && strcasestr(pass, cfg->node_phone)) + ) { + *reason = PasswordObvious; + return false; + } + return true; +} diff --git a/src/sbbs3/userdat.h b/src/sbbs3/userdat.h index a04744540e..6d07f7915b 100644 --- a/src/sbbs3/userdat.h +++ b/src/sbbs3/userdat.h @@ -202,6 +202,7 @@ DLLEXPORT bool user_adjust_minutes(scfg_t*, user_t*, long amount); DLLEXPORT time_t gettimeleft(scfg_t*, user_t*, time_t starttime); +DLLEXPORT bool check_pass(scfg_t*, const char *passwd, user_t* user, bool unique, int* reason); DLLEXPORT bool check_name(scfg_t*, const char* name); DLLEXPORT bool check_realname(scfg_t*, const char* name); DLLEXPORT bool sysop_available(scfg_t*); -- GitLab