From d47722a8e8ced58cad606b476e2c3c42bdc79784 Mon Sep 17 00:00:00 2001 From: Till Date: Tue, 22 Feb 2022 17:26:44 +0100 Subject: [PATCH 01/53] Start of implementing 2fa --- .../sql/incremental/upd_dev_collection.sql | 1 + install/sql/ispconfig3.sql | 5 + interface/web/login/index.php | 23 ++- interface/web/login/otp.php | 181 ++++++++++++++++++ 4 files changed, 208 insertions(+), 2 deletions(-) create mode 100644 interface/web/login/otp.php diff --git a/install/sql/incremental/upd_dev_collection.sql b/install/sql/incremental/upd_dev_collection.sql index e69de29bb2..9f73ec0647 100644 --- a/install/sql/incremental/upd_dev_collection.sql +++ b/install/sql/incremental/upd_dev_collection.sql @@ -0,0 +1 @@ +ALTER TABLE `sys_user` ADD `otp_enabled` SET('n', 'y','v') NOT NULL DEFAULT 'n' AFTER `lost_password_reqtime`, ADD `otp_type` SET('email') NOT NULL DEFAULT 'email' AFTER `otp_enabled`, ADD `otp_data` VARCHAR(255) NULL AFTER `otp_type`, ADD `otp_recovery` VARCHAR(64) NULL AFTER `otp_data`, ADD `otp_attempts` TINYINT NOT NULL DEFAULT '0' AFTER `otp_recovery`; \ No newline at end of file diff --git a/install/sql/ispconfig3.sql b/install/sql/ispconfig3.sql index 4d61c38d8f..60a90c2a5e 100644 --- a/install/sql/ispconfig3.sql +++ b/install/sql/ispconfig3.sql @@ -1820,6 +1820,11 @@ CREATE TABLE `sys_user` ( `lost_password_function` tinyint(1) NOT NULL default '1', `lost_password_hash` VARCHAR(50) NOT NULL default '', `lost_password_reqtime` DATETIME NULL default NULL, + `otp_enabled` set('n','y','v') NOT NULL DEFAULT 'n', + `otp_type` set('email') NOT NULL DEFAULT 'email', + `otp_data` varchar(255) DEFAULT NULL, + `otp_recovery` varchar(64) DEFAULT NULL, + `otp_attempts` tinyint(4) NOT NULL DEFAULT 0, PRIMARY KEY (`userid`) ) DEFAULT CHARSET=utf8 AUTO_INCREMENT=1 ; diff --git a/interface/web/login/index.php b/interface/web/login/index.php index 70c3dbe055..35f5c50294 100644 --- a/interface/web/login/index.php +++ b/interface/web/login/index.php @@ -279,8 +279,27 @@ if(count($_POST) > 0) { echo 'LOGIN_REDIRECT:'.$_SESSION['s']['module']['startpage']; exit; } else { - header('Location: ../index.php'); - die(); + + //* Do 2FA authentication + if($user['otp_enabled'] != 'n') { + + //* Save session in pending state and destroy original session + $_SESSION['s_pending'] = $_SESSION['s']; + unset($_SESSION['s']); + + //* Create OTP session + $_SESSION['otp']['session_attempts'] = 0; + $_SESSION['otp']['type'] = $user['otp_type']; + $_SESSION['otp']['data'] = $user['otp_data']; + $_SESSION['otp']['recovery'] = $user['otp_recovery']; + + //* Redirect to otp script + header('Location: otp.php'); + die(); + } else { + header('Location: ../index.php'); + die(); + } } } } else { diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php new file mode 100644 index 0000000000..4bcd79cf1a --- /dev/null +++ b/interface/web/login/otp.php @@ -0,0 +1,181 @@ += 1) { + $app->auth->csrf_token_check(); +} + +//* Handle recovery code +if(isset($_POST['code']) && strlen($_POST['code']) == 32 && $_SESSION['otp']['recovery'])) { + //* TODO Recovery code handling + + $user = $app->db->queryOneRecord('SELECT otp_attempts FROM sys_user WHERE userid = ?',$_SESSION['s_pending']['user']['userid']); + + //* We allow one more try to enter recovery code + if($user['otp_attempts'] > $max_global_code_retry + 1) { + + } + + + die('Handle recovery code'); +} + + +//* Begin 2fa via Email +if($_SESSION['otp']['type'] == 'email') { + + //* Email 2fa handler settings + $max_code_resend = 3; + $max_time = 600; // time in seconds until the code gets invalidated + $code_length = 6; + + if(isset($_POST['code']) && strlen($_POST['code']) == $code_length && isset($_SESSION['otp']['code'])) { + + if(strlen($_SESSION['otp']['code']) != $code_length) die(); // wrong code lenght, this should never happen + + $user = $app->db->queryOneRecord('SELECT otp_attempts FROM sys_user WHERE userid = ?',$_SESSION['s_pending']['user']['userid']); + + //* Check if we reached limits + if($_SESSION['otp']['sent'] > $max_code_resend + || $_SESSION['otp']['session_attempts'] > $max_session_code_retry + || $user['otp_attempts'] > $max_global_code_retry + || time() > $_SESSION['otp']['starttime'] + $max_time) { + unset($_SESSION['otp']); + unset($_SESSION['s_pending']); + $app->error('2FA failed','index.php'); + } + + //* 2fa success + if($_POST['code'] == $_SESSION['otp']['code']) { + $_SESSION['s'] = $_SESSION['s_pending']; + unset($_SESSION['s_pending']); + unset($_SESSION['otp']); + header('Location: ../index.php'); + die(); + } else { + //* 2fa wrong code + $_SESSION['otp']['session_attempts']++; + $app->db->query() + } + } + + //* set code + if(!isset($_SESSION['otp']['code']) || empty($_SESSION['otp']['code'])) { + // TODO Code generator + $_SESSION['otp']['code'] = 123456; + $_SESSION['otp']['starttime'] = time(); + } + + //* Send code via email + if(!isset($_SESSION['otp']['sent']) || $_GET['action'] == 'resend') { + + //* Ensure that code is not sent too often + if(isset($_SESSION['otp']['sent']) && $_SESSION['otp']['sent'] > $max_code_resend) { + $app->error('Code resend limit reached','index.php'); + } + + $app->uses('functions'); + + //* send email + $email_to = $_SESSION['otp']['data']; + $subject = 'ISPConfig Login authentication'; + $text = ''; + $from = 'root@localhost'; + + $app->functions->mail($email_to, $subject, $text, $from); + + //* increase sent counter + if(!isset($_SESSION['otp']['sent'])) { + $_SESSION['otp']['sent'] = 1; + } else { + $_SESSION['otp']['sent']++; + } + + } + + //* Show form to enter email code + + + +} else { + //* unsupported 2fa type + $app->error('Code resend limit reached','index.php'); +} + + + + + +//* Load templating system and lang file +$app->uses('tpl'); +$app->tpl->newTemplate('main_login.tpl.htm'); +$app->tpl->setInclude('content_tpl', 'templates/otp.htm'); + + +//* SET csrf token +$csrf_token = $app->auth->csrf_token_get('language_edit'); +$app->tpl->setVar('_csrf_id',$csrf_token['csrf_id']); +$app->tpl->setVar('_csrf_key',$csrf_token['csrf_key']); + + +$app->load_language_file('web/login/lib/lang/'.$conf["language"].'.lng'); + + + + + +$app->tpl_defaults(); +$app->tpl->pparse(); + + +?> \ No newline at end of file -- GitLab From 0ecb8206ce528790dae483b4886bbe7d9aef3c20 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Sat, 5 Mar 2022 22:44:02 +0100 Subject: [PATCH 02/53] Add translation strings --- interface/web/login/lib/lang/en.lng | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/interface/web/login/lib/lang/en.lng b/interface/web/login/lib/lang/en.lng index a99d0d3d09..1cc0f59f5c 100644 --- a/interface/web/login/lib/lang/en.lng +++ b/interface/web/login/lib/lang/en.lng @@ -32,4 +32,8 @@ $wb['lost_password_function_disabled_txt'] = 'The lost password function is not $wb['lost_password_function_wait_txt'] = 'You cannot request a new password yet. Please wait a few minutes.'; $wb['lost_password_function_expired_txt'] = 'This activation link has expired. Please request a new one.'; $wb['lost_password_function_denied_txt'] = 'This activation link is not valid.'; +$wb['otp_code_txt'] = '2-Factor Authentication'; +$wb['otp_code_desc_txt'] = 'Enter the code you got from your authenticator app or via email.'; +$wb['otp_code_placeholder_txt'] = 'OTP code'; +$wb['otp_code_reset_txt'] = 'Request new code'; ?> -- GitLab From d3c1500bac94e1707597bc258a24acfa259981ac Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Sat, 5 Mar 2022 22:46:34 +0100 Subject: [PATCH 03/53] More implementing 2fa --- interface/web/login/otp.php | 37 ++++++++++++++++++++------- interface/web/login/templates/otp.htm | 24 +++++++++++++++++ 2 files changed, 52 insertions(+), 9 deletions(-) create mode 100755 interface/web/login/templates/otp.htm diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 4bcd79cf1a..3e4abf8503 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -55,8 +55,13 @@ if(count($_POST) >= 1) { $app->auth->csrf_token_check(); } + +// FIXME What's the deal with otp_enabled=v ?? + + + //* Handle recovery code -if(isset($_POST['code']) && strlen($_POST['code']) == 32 && $_SESSION['otp']['recovery'])) { +if(isset($_POST['code']) && strlen($_POST['code']) == 32 && $_SESSION['otp']['recovery']) { //* TODO Recovery code handling $user = $app->db->queryOneRecord('SELECT otp_attempts FROM sys_user WHERE userid = ?',$_SESSION['s_pending']['user']['userid']); @@ -66,6 +71,7 @@ if(isset($_POST['code']) && strlen($_POST['code']) == 32 && $_SESSION['otp']['re } + // show reset form to create a new 2fa secret? die('Handle recovery code'); } @@ -104,8 +110,8 @@ if($_SESSION['otp']['type'] == 'email') { die(); } else { //* 2fa wrong code - $_SESSION['otp']['session_attempts']++; - $app->db->query() + $_SESSION['otp']['session_attempts']++; // FIXME can't we skip this and rely on the DB only? + $app->db->query('UPDATE `sys_user` SET otp_attempts=otp_attempts + 1 WHERE userid = ?', $_SESSION['s_pending']['user']['userid']); } } @@ -125,12 +131,16 @@ if($_SESSION['otp']['type'] == 'email') { } $app->uses('functions'); - + $app->uses('getconf'); + $system_config = $app->getconf->get_global_config(); + $from = $system_config['mail']['admin_mail']; + + //* send email $email_to = $_SESSION['otp']['data']; $subject = 'ISPConfig Login authentication'; - $text = ''; - $from = 'root@localhost'; + $text = 'Your One time login code is ' . $_SESSION['otp']['code'] . PHP_EOL + . 'This code is valid for 10 minutes' . PHP_EOL; $app->functions->mail($email_to, $subject, $text, $from); @@ -144,7 +154,7 @@ if($_SESSION['otp']['type'] == 'email') { } //* Show form to enter email code - + // ... below } else { @@ -153,7 +163,15 @@ if($_SESSION['otp']['type'] == 'email') { } +$logo = $app->db->queryOneRecord("SELECT * FROM sys_ini WHERE sysini_id = 1"); +if($logo['custom_logo'] != ''){ + $base64_logo_txt = $logo['custom_logo']; +} else { + $base64_logo_txt = $logo['default_logo']; +} +$app->tpl->setVar('base64_logo_txt', $base64_logo_txt); +$app->tpl->setVar('current_theme', isset($_SESSION['s']['theme']) ? $_SESSION['s']['theme'] : 'default', true); //* Load templating system and lang file @@ -168,7 +186,8 @@ $app->tpl->setVar('_csrf_id',$csrf_token['csrf_id']); $app->tpl->setVar('_csrf_key',$csrf_token['csrf_key']); -$app->load_language_file('web/login/lib/lang/'.$conf["language"].'.lng'); +require ISPC_ROOT_PATH.'/web/login/lib/lang/'.$app->functions->check_language($conf['language']).'.lng'; +$app->tpl->setVar($wb); @@ -178,4 +197,4 @@ $app->tpl_defaults(); $app->tpl->pparse(); -?> \ No newline at end of file +?> diff --git a/interface/web/login/templates/otp.htm b/interface/web/login/templates/otp.htm new file mode 100755 index 0000000000..d6208e6a64 --- /dev/null +++ b/interface/web/login/templates/otp.htm @@ -0,0 +1,24 @@ + + + + + + +

+

+
+
+
+ +
+
+ +
+ {tmpl_var name='otp_code_reset_txt'} + + + + + +
+
-- GitLab From 7a1aec566316cd8bcd5e5a4472a62706b7521b35 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Sun, 6 Mar 2022 21:23:14 +0100 Subject: [PATCH 04/53] Fix indent --- interface/web/login/otp.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 3e4abf8503..d4b5dc92e2 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -93,9 +93,10 @@ if($_SESSION['otp']['type'] == 'email') { //* Check if we reached limits if($_SESSION['otp']['sent'] > $max_code_resend - || $_SESSION['otp']['session_attempts'] > $max_session_code_retry - || $user['otp_attempts'] > $max_global_code_retry - || time() > $_SESSION['otp']['starttime'] + $max_time) { + || $_SESSION['otp']['session_attempts'] > $max_session_code_retry + || $user['otp_attempts'] > $max_global_code_retry + || time() > $_SESSION['otp']['starttime'] + $max_time + ) { unset($_SESSION['otp']); unset($_SESSION['s_pending']); $app->error('2FA failed','index.php'); @@ -107,7 +108,7 @@ if($_SESSION['otp']['type'] == 'email') { unset($_SESSION['s_pending']); unset($_SESSION['otp']); header('Location: ../index.php'); - die(); + die(); } else { //* 2fa wrong code $_SESSION['otp']['session_attempts']++; // FIXME can't we skip this and rely on the DB only? -- GitLab From 50352f2ea7abb132b6395e470932b10277141276 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Sun, 6 Mar 2022 21:23:48 +0100 Subject: [PATCH 05/53] Generate ramdom int --- interface/web/login/otp.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index d4b5dc92e2..ac92b10de6 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -118,8 +118,8 @@ if($_SESSION['otp']['type'] == 'email') { //* set code if(!isset($_SESSION['otp']['code']) || empty($_SESSION['otp']['code'])) { - // TODO Code generator - $_SESSION['otp']['code'] = 123456; + // Random int between 10^($code_length-1) and 10^$code_length + $_SESSION['otp']['code'] = rand(pow(10, $code_length - 1), pow(10, $code_length) - 1); $_SESSION['otp']['starttime'] = time(); } -- GitLab From 745b73c434ee1356afd18c3d5bc7b56da6186434 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Sun, 6 Mar 2022 21:38:25 +0100 Subject: [PATCH 06/53] Reset the attempt counter when successfully logged in --- interface/web/login/otp.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index ac92b10de6..60516d902e 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -105,6 +105,8 @@ if($_SESSION['otp']['type'] == 'email') { //* 2fa success if($_POST['code'] == $_SESSION['otp']['code']) { $_SESSION['s'] = $_SESSION['s_pending']; + // Reset the attempt counter. + $app->db->query('UPDATE `sys_user` SET otp_attempts=0 WHERE userid = ?', $_SESSION['s']['user']['userid']); unset($_SESSION['s_pending']); unset($_SESSION['otp']); header('Location: ../index.php'); @@ -185,6 +187,7 @@ $app->tpl->setInclude('content_tpl', 'templates/otp.htm'); $csrf_token = $app->auth->csrf_token_get('language_edit'); $app->tpl->setVar('_csrf_id',$csrf_token['csrf_id']); $app->tpl->setVar('_csrf_key',$csrf_token['csrf_key']); +#$app->tpl->setVar('msg', print_r($_SESSION['otp'], 1)); require ISPC_ROOT_PATH.'/web/login/lib/lang/'.$app->functions->check_language($conf['language']).'.lng'; -- GitLab From 08a42de9a7a6aa8505a4ec89dedff5f6e591aa74 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Mon, 7 Mar 2022 23:34:26 +0100 Subject: [PATCH 07/53] Add a UI element for OTP in the User Settings form --- interface/web/login/index.php | 2 +- .../web/tools/form/user_settings.tform.php | 23 +++++++++++++++++++ .../web/tools/lib/lang/en_usersettings.lng | 1 + .../web/tools/templates/user_settings.htm | 8 +++++++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/interface/web/login/index.php b/interface/web/login/index.php index 35f5c50294..3c03ca8faa 100644 --- a/interface/web/login/index.php +++ b/interface/web/login/index.php @@ -281,7 +281,7 @@ if(count($_POST) > 0) { } else { //* Do 2FA authentication - if($user['otp_enabled'] != 'n') { + if(!empty($user['otp_type']) && $user['otp_type'] != 'none') { //* Save session in pending state and destroy original session $_SESSION['s_pending'] = $_SESSION['s']; diff --git a/interface/web/tools/form/user_settings.tform.php b/interface/web/tools/form/user_settings.tform.php index f063634b0c..56252dff92 100644 --- a/interface/web/tools/form/user_settings.tform.php +++ b/interface/web/tools/form/user_settings.tform.php @@ -119,6 +119,10 @@ if($_SESSION["s"]["user"]["typ"] == 'admin') { } } +$otp_method_list = array( + 'none' => 'none', + 'email' => 'email', +); //* Load themes $themes_list = array(); $handle = @opendir(ISPC_THEMES_PATH); @@ -163,6 +167,25 @@ $form['tabs']['users'] = array ( 'rows' => '', 'cols' => '' ), + 'otp_type' => array( + 'datatype' => 'VARCHAR', + 'formtype' => 'SELECT', + 'validators' => array ( 0 => array ( 'type' => 'NOTEMPTY', + 'errmsg'=> 'oth_auth_empty'), + 1 => array ( 'type' => 'REGEX', + 'regex' => '/^[a-z0-9\_]{0,64}$/', + 'errmsg'=> 'otp_auth_regex'), + ), + 'regex' => '', + 'errmsg' => '', + 'default' => '', + 'value' => $otp_method_list, + 'separator' => '', + 'width' => '30', + 'maxlength' => '255', + 'rows' => '', + 'cols' => '' + ), 'language' => array ( 'datatype' => 'VARCHAR', 'formtype' => 'SELECT', diff --git a/interface/web/tools/lib/lang/en_usersettings.lng b/interface/web/tools/lib/lang/en_usersettings.lng index 421805e789..2f052a6e94 100644 --- a/interface/web/tools/lib/lang/en_usersettings.lng +++ b/interface/web/tools/lib/lang/en_usersettings.lng @@ -16,4 +16,5 @@ $wb['startmodule_empty'] = 'Startmodule empty.'; $wb['startmodule_regex'] = 'Invalid chars in Startmodule.'; $wb['app_theme_empty'] = 'App theme empty.'; $wb['app_theme_regex'] = 'Invalid chars in App theme.'; +$wb['otp_auth_txt'] = '2-Factor Authentication'; ?> diff --git a/interface/web/tools/templates/user_settings.htm b/interface/web/tools/templates/user_settings.htm index a620f419c5..f430b64195 100644 --- a/interface/web/tools/templates/user_settings.htm +++ b/interface/web/tools/templates/user_settings.htm @@ -26,6 +26,14 @@ +
+ +
+ +
+
+ {tmpl_var name='otp_type'} + +
+
+
-- GitLab From 5ac9724fd4fd2a7a514c0b13f853d1d8320ce67e Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 15:21:49 +0100 Subject: [PATCH 10/53] Deduplicate auth logging --- interface/lib/app.inc.php | 6 ++++++ interface/web/login/index.php | 11 +---------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/interface/lib/app.inc.php b/interface/lib/app.inc.php index ee4713cd98..acec963541 100755 --- a/interface/lib/app.inc.php +++ b/interface/lib/app.inc.php @@ -208,6 +208,12 @@ class app { } } + public function auth_log($msg) { + $authlog_handle = fopen($this->_conf['ispconfig_log_dir'].'/auth.log', 'a'); + fwrite($authlog_handle, $msg . PHP_EOL); + fclose($authlog_handle); + } + /** Priority values are: 0 = DEBUG, 1 = WARNING, 2 = ERROR */ public function error($msg, $next_link = '', $stop = true, $priority = 1) { //$this->uses("error"); diff --git a/interface/web/login/index.php b/interface/web/login/index.php index 3c03ca8faa..a9592c3460 100644 --- a/interface/web/login/index.php +++ b/interface/web/login/index.php @@ -264,11 +264,6 @@ if(count($_POST) > 0) { $app->plugin->raiseEvent('login', $username); - //* Save successfull login message to var - $authlog = 'Successful login for user \''. $username .'\' from '. $_SERVER['REMOTE_ADDR'] .' at '. date('Y-m-d H:i:s') . ' with session ID ' .session_id(); - $authlog_handle = fopen($conf['ispconfig_log_dir'].'/auth.log', 'a'); - fwrite($authlog_handle, $authlog ."\n"); - fclose($authlog_handle); /* * We need LOGIN_REDIRECT instead of HEADER_REDIRECT to load the @@ -321,11 +316,7 @@ if(count($_POST) > 0) { if($app->db->errorMessage != '') $error .= '
'.$app->db->errorMessage != ''; $app->plugin->raiseEvent('login_failed', $username); - //* Save failed login message to var - $authlog = 'Failed login for user \''. $username .'\' from '. $_SERVER['REMOTE_ADDR'] .' at '. date('Y-m-d H:i:s'); - $authlog_handle = fopen($conf['ispconfig_log_dir'].'/auth.log', 'a'); - fwrite($authlog_handle, $authlog ."\n"); - fclose($authlog_handle); + $app->auth_log('Failed login for user \''. $username .'\' from '. $_SERVER['REMOTE_ADDR'] .' at '. date('Y-m-d H:i:s')); } } } else { -- GitLab From bd02e907e542ab0709c0dfc2131dac802b7b753c Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 15:22:03 +0100 Subject: [PATCH 11/53] more logging --- interface/web/login/index.php | 1 + 1 file changed, 1 insertion(+) diff --git a/interface/web/login/index.php b/interface/web/login/index.php index a9592c3460..be3d3bd834 100644 --- a/interface/web/login/index.php +++ b/interface/web/login/index.php @@ -292,6 +292,7 @@ if(count($_POST) > 0) { header('Location: otp.php'); die(); } else { + $app->auth_log('Successful login for user \''. $username .'\' from '. $_SERVER['REMOTE_ADDR'] .' at '. date('Y-m-d H:i:s') . ' with session ID ' .session_id()); header('Location: ../index.php'); die(); } -- GitLab From 12deed60eb6d639737a5a0b98b78d72885305893 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 15:22:21 +0100 Subject: [PATCH 12/53] less magic numbers --- interface/web/login/otp.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 60516d902e..1d5b41d7ce 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -48,7 +48,7 @@ $error = ''; $msg = ''; $max_session_code_retry = 3; $max_global_code_retry = 10; - +$otp_recovery_code_length = 32; //* CSRF Check if we got POST data if(count($_POST) >= 1) { @@ -61,7 +61,7 @@ if(count($_POST) >= 1) { //* Handle recovery code -if(isset($_POST['code']) && strlen($_POST['code']) == 32 && $_SESSION['otp']['recovery']) { +if(isset($_POST['code']) && strlen($_POST['code']) == $otp_recovery_code_length && $_SESSION['otp']['recovery']) { //* TODO Recovery code handling $user = $app->db->queryOneRecord('SELECT otp_attempts FROM sys_user WHERE userid = ?',$_SESSION['s_pending']['user']['userid']); -- GitLab From d829bc1133f2e08b785a39c0f82315f14bba28bd Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 15:55:01 +0100 Subject: [PATCH 13/53] Re-use success code for recovery code flow --- interface/web/login/otp.php | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 1d5b41d7ce..ef85c7c4cc 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -59,6 +59,21 @@ if(count($_POST) >= 1) { // FIXME What's the deal with otp_enabled=v ?? +function finish_2fa_success($msg = '') { + global $app; + $_SESSION['s'] = $_SESSION['s_pending']; + unset($_SESSION['s_pending']); + unset($_SESSION['otp']); + $username = $_SESSION['s']['user']['username']; + if (!empty($msg)) { + $msg = ' ' . $msg; + } + $app->auth_log('Successful login for user \''. $username .'\' ' . $msg . ' from '. $_SERVER['REMOTE_ADDR'] .' at '. date('Y-m-d H:i:s') . ' with session ID ' .session_id()); + $app->db->query('UPDATE `sys_user` SET otp_attempts=0 WHERE userid = ?', $_SESSION['s']['user']['userid']); + session_write_close(); + header('Location: ../index.php'); + die(); +} //* Handle recovery code if(isset($_POST['code']) && strlen($_POST['code']) == $otp_recovery_code_length && $_SESSION['otp']['recovery']) { @@ -68,12 +83,12 @@ if(isset($_POST['code']) && strlen($_POST['code']) == $otp_recovery_code_length //* We allow one more try to enter recovery code if($user['otp_attempts'] > $max_global_code_retry + 1) { - + die("Sorry, contact your administrator."); } - // show reset form to create a new 2fa secret? - - die('Handle recovery code'); + if ($_SESSION['otp']['recovery'] == $_POST['code']) { + finish_2fa_success('via 2fa recovery code'); + } } @@ -104,13 +119,7 @@ if($_SESSION['otp']['type'] == 'email') { //* 2fa success if($_POST['code'] == $_SESSION['otp']['code']) { - $_SESSION['s'] = $_SESSION['s_pending']; - // Reset the attempt counter. - $app->db->query('UPDATE `sys_user` SET otp_attempts=0 WHERE userid = ?', $_SESSION['s']['user']['userid']); - unset($_SESSION['s_pending']); - unset($_SESSION['otp']); - header('Location: ../index.php'); - die(); + finish_2fa_success(); } else { //* 2fa wrong code $_SESSION['otp']['session_attempts']++; // FIXME can't we skip this and rely on the DB only? -- GitLab From fc3ef8450438719f05b53b7a3573722322311dd3 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 16:40:21 +0100 Subject: [PATCH 14/53] Move login event, for 2fa it is done in otp.php --- interface/web/login/index.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/interface/web/login/index.php b/interface/web/login/index.php index f7a24a206f..d0dbfc0ec5 100644 --- a/interface/web/login/index.php +++ b/interface/web/login/index.php @@ -126,13 +126,6 @@ function process_login_request(app $app, &$error, $conf, $module) } } - $app->plugin->raiseEvent('login', $username); - - //* Save successful login message to var - $authlog = 'Successful login for user \''.$username.'\' from '.$_SERVER['REMOTE_ADDR'].' at '.date('Y-m-d H:i:s').' with session ID '.session_id(); - $authlog_handle = fopen($conf['ispconfig_log_dir'].'/auth.log', 'a'); - fwrite($authlog_handle, $authlog."\n"); - fclose($authlog_handle); /* * We need LOGIN_REDIRECT instead of HEADER_REDIRECT to load the @@ -161,6 +154,8 @@ function process_login_request(app $app, &$error, $conf, $module) header('Location: otp.php'); die(); } else { + $app->plugin->raiseEvent('login', $username); + $app->auth_log('Successful login for user \''. $username .'\' ' . $msg . ' from '. $_SERVER['REMOTE_ADDR'] .' at '. date('Y-m-d H:i:s') . ' with session ID ' .session_id()); header('Location: ../index.php'); die(); } -- GitLab From d466b94295a14798d4defb36e3bc22cc0d25453a Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 16:40:41 +0100 Subject: [PATCH 15/53] More use of the auth_log fucction --- interface/web/login/index.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/interface/web/login/index.php b/interface/web/login/index.php index d0dbfc0ec5..5d0d5db9b8 100644 --- a/interface/web/login/index.php +++ b/interface/web/login/index.php @@ -175,11 +175,7 @@ function process_login_request(app $app, &$error, $conf, $module) if ($app->db->errorMessage != '') $error .= '
'.$app->db->errorMessage != ''; $app->plugin->raiseEvent('login_failed', $username); - //* Save failed login message to var - $authlog = 'Failed login for user \''.$username.'\' from '.$_SERVER['REMOTE_ADDR'].' at '.date('Y-m-d H:i:s'); - $authlog_handle = fopen($conf['ispconfig_log_dir'].'/auth.log', 'a'); - fwrite($authlog_handle, $authlog."\n"); - fclose($authlog_handle); + $app->auth_log('Failed login for user \''. $username .'\ from '. $_SERVER['REMOTE_ADDR'] .' at '. date('Y-m-d H:i:s'); } } -- GitLab From b9a4f8cad0a7e5df69ee8369cb3bcf9a98c15227 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 16:59:26 +0100 Subject: [PATCH 16/53] Whitespace cleanup --- interface/web/login/otp.php | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index ef85c7c4cc..d85eb7123e 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -78,14 +78,14 @@ function finish_2fa_success($msg = '') { //* Handle recovery code if(isset($_POST['code']) && strlen($_POST['code']) == $otp_recovery_code_length && $_SESSION['otp']['recovery']) { //* TODO Recovery code handling - + $user = $app->db->queryOneRecord('SELECT otp_attempts FROM sys_user WHERE userid = ?',$_SESSION['s_pending']['user']['userid']); - + //* We allow one more try to enter recovery code if($user['otp_attempts'] > $max_global_code_retry + 1) { die("Sorry, contact your administrator."); } - + if ($_SESSION['otp']['recovery'] == $_POST['code']) { finish_2fa_success('via 2fa recovery code'); } @@ -99,13 +99,13 @@ if($_SESSION['otp']['type'] == 'email') { $max_code_resend = 3; $max_time = 600; // time in seconds until the code gets invalidated $code_length = 6; - + if(isset($_POST['code']) && strlen($_POST['code']) == $code_length && isset($_SESSION['otp']['code'])) { - + if(strlen($_SESSION['otp']['code']) != $code_length) die(); // wrong code lenght, this should never happen $user = $app->db->queryOneRecord('SELECT otp_attempts FROM sys_user WHERE userid = ?',$_SESSION['s_pending']['user']['userid']); - + //* Check if we reached limits if($_SESSION['otp']['sent'] > $max_code_resend || $_SESSION['otp']['session_attempts'] > $max_session_code_retry @@ -116,7 +116,7 @@ if($_SESSION['otp']['type'] == 'email') { unset($_SESSION['s_pending']); $app->error('2FA failed','index.php'); } - + //* 2fa success if($_POST['code'] == $_SESSION['otp']['code']) { finish_2fa_success(); @@ -126,36 +126,35 @@ if($_SESSION['otp']['type'] == 'email') { $app->db->query('UPDATE `sys_user` SET otp_attempts=otp_attempts + 1 WHERE userid = ?', $_SESSION['s_pending']['user']['userid']); } } - + //* set code if(!isset($_SESSION['otp']['code']) || empty($_SESSION['otp']['code'])) { // Random int between 10^($code_length-1) and 10^$code_length $_SESSION['otp']['code'] = rand(pow(10, $code_length - 1), pow(10, $code_length) - 1); $_SESSION['otp']['starttime'] = time(); } - + //* Send code via email if(!isset($_SESSION['otp']['sent']) || $_GET['action'] == 'resend') { - + //* Ensure that code is not sent too often if(isset($_SESSION['otp']['sent']) && $_SESSION['otp']['sent'] > $max_code_resend) { $app->error('Code resend limit reached','index.php'); } - + $app->uses('functions'); $app->uses('getconf'); $system_config = $app->getconf->get_global_config(); $from = $system_config['mail']['admin_mail']; - //* send email $email_to = $_SESSION['otp']['data']; $subject = 'ISPConfig Login authentication'; $text = 'Your One time login code is ' . $_SESSION['otp']['code'] . PHP_EOL . 'This code is valid for 10 minutes' . PHP_EOL; - + $app->functions->mail($email_to, $subject, $text, $from); - + //* increase sent counter if(!isset($_SESSION['otp']['sent'])) { $_SESSION['otp']['sent'] = 1; @@ -164,10 +163,9 @@ if($_SESSION['otp']['type'] == 'email') { } } - + //* Show form to enter email code // ... below - } else { //* unsupported 2fa type @@ -191,21 +189,16 @@ $app->uses('tpl'); $app->tpl->newTemplate('main_login.tpl.htm'); $app->tpl->setInclude('content_tpl', 'templates/otp.htm'); - + //* SET csrf token $csrf_token = $app->auth->csrf_token_get('language_edit'); $app->tpl->setVar('_csrf_id',$csrf_token['csrf_id']); $app->tpl->setVar('_csrf_key',$csrf_token['csrf_key']); #$app->tpl->setVar('msg', print_r($_SESSION['otp'], 1)); - require ISPC_ROOT_PATH.'/web/login/lib/lang/'.$app->functions->check_language($conf['language']).'.lng'; $app->tpl->setVar($wb); - - - - $app->tpl_defaults(); $app->tpl->pparse(); -- GitLab From acea2cdbd33e16bb37f342c9a73c325a1419fdd2 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 17:14:50 +0100 Subject: [PATCH 17/53] syntax fix --- interface/web/login/index.php | 2 +- interface/web/login/otp.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/interface/web/login/index.php b/interface/web/login/index.php index 5d0d5db9b8..d72038d2f3 100644 --- a/interface/web/login/index.php +++ b/interface/web/login/index.php @@ -175,7 +175,7 @@ function process_login_request(app $app, &$error, $conf, $module) if ($app->db->errorMessage != '') $error .= '
'.$app->db->errorMessage != ''; $app->plugin->raiseEvent('login_failed', $username); - $app->auth_log('Failed login for user \''. $username .'\ from '. $_SERVER['REMOTE_ADDR'] .' at '. date('Y-m-d H:i:s'); + $app->auth_log('Failed login for user \''. $username .'\ from '. $_SERVER['REMOTE_ADDR'] .' at '. date('Y-m-d H:i:s')); } } diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index d85eb7123e..5d3e497a96 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -94,7 +94,7 @@ if(isset($_POST['code']) && strlen($_POST['code']) == $otp_recovery_code_length //* Begin 2fa via Email if($_SESSION['otp']['type'] == 'email') { - + //* Email 2fa handler settings $max_code_resend = 3; $max_time = 600; // time in seconds until the code gets invalidated @@ -103,7 +103,7 @@ if($_SESSION['otp']['type'] == 'email') { if(isset($_POST['code']) && strlen($_POST['code']) == $code_length && isset($_SESSION['otp']['code'])) { if(strlen($_SESSION['otp']['code']) != $code_length) die(); // wrong code lenght, this should never happen - + $user = $app->db->queryOneRecord('SELECT otp_attempts FROM sys_user WHERE userid = ?',$_SESSION['s_pending']['user']['userid']); //* Check if we reached limits -- GitLab From a53aba217f885edc480e76bb9edea87fb51beef8 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 20:45:55 +0100 Subject: [PATCH 18/53] typo --- interface/web/login/otp.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 5d3e497a96..903f5254ab 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -135,7 +135,7 @@ if($_SESSION['otp']['type'] == 'email') { } //* Send code via email - if(!isset($_SESSION['otp']['sent']) || $_GET['action'] == 'resend') { + if(!isset($_SESSION['otp']['sent']) || $_GET['action'] == 'resent') { //* Ensure that code is not sent too often if(isset($_SESSION['otp']['sent']) && $_SESSION['otp']['sent'] > $max_code_resend) { -- GitLab From 0a843842db059039da677810a0f926197f1b129b Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 20:56:14 +0100 Subject: [PATCH 19/53] resend with a d --- interface/web/login/lib/lang/en.lng | 2 +- interface/web/login/otp.php | 2 +- interface/web/login/templates/otp.htm | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/interface/web/login/lib/lang/en.lng b/interface/web/login/lib/lang/en.lng index 1cc0f59f5c..5b411688f2 100644 --- a/interface/web/login/lib/lang/en.lng +++ b/interface/web/login/lib/lang/en.lng @@ -35,5 +35,5 @@ $wb['lost_password_function_denied_txt'] = 'This activation link is not valid.'; $wb['otp_code_txt'] = '2-Factor Authentication'; $wb['otp_code_desc_txt'] = 'Enter the code you got from your authenticator app or via email.'; $wb['otp_code_placeholder_txt'] = 'OTP code'; -$wb['otp_code_reset_txt'] = 'Request new code'; +$wb['otp_code_resend_txt'] = 'Request new code'; ?> diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 903f5254ab..5d3e497a96 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -135,7 +135,7 @@ if($_SESSION['otp']['type'] == 'email') { } //* Send code via email - if(!isset($_SESSION['otp']['sent']) || $_GET['action'] == 'resent') { + if(!isset($_SESSION['otp']['sent']) || $_GET['action'] == 'resend') { //* Ensure that code is not sent too often if(isset($_SESSION['otp']['sent']) && $_SESSION['otp']['sent'] > $max_code_resend) { diff --git a/interface/web/login/templates/otp.htm b/interface/web/login/templates/otp.htm index d6208e6a64..716b1660cd 100755 --- a/interface/web/login/templates/otp.htm +++ b/interface/web/login/templates/otp.htm @@ -14,7 +14,7 @@
- {tmpl_var name='otp_code_reset_txt'} + {tmpl_var name='otp_code_resend_txt'} -- GitLab From c97ae7f42d9c998133153fe5b1f0e2537299e685 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 21:04:15 +0100 Subject: [PATCH 20/53] Explicit check, the verifying state should not trigger here. --- interface/web/login/index.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/web/login/index.php b/interface/web/login/index.php index d72038d2f3..b5b86be615 100644 --- a/interface/web/login/index.php +++ b/interface/web/login/index.php @@ -138,7 +138,7 @@ function process_login_request(app $app, &$error, $conf, $module) } else { //* Do 2FA authentication - if($user['otp_enabled'] != 'n') { + if($user['otp_enabled'] == 'y') { //* Save session in pending state and destroy original session $_SESSION['s_pending'] = $_SESSION['s']; -- GitLab From e748c8b156687093c7219051614a6a6e4029a59f Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 21:04:20 +0100 Subject: [PATCH 21/53] whitespace --- interface/web/tools/lib/lang/en.lng | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/web/tools/lib/lang/en.lng b/interface/web/tools/lib/lang/en.lng index 7794543497..eff8a13323 100644 --- a/interface/web/tools/lib/lang/en.lng +++ b/interface/web/tools/lib/lang/en.lng @@ -10,4 +10,4 @@ $wb['Resync'] = 'Resync'; $wb['Import'] = 'Import'; $wb['ISPConfig 3 mail'] = 'ISPConfig 3 mail'; $wb['PDNS Tupa'] = 'PowerDNS Tupa'; -?> \ No newline at end of file +?> -- GitLab From febd6ab12563e23f57eb4137f8ea61d866858313 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 21:58:24 +0100 Subject: [PATCH 22/53] Better use random_int --- interface/web/login/otp.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 5d3e497a96..091152bc0e 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -129,8 +129,7 @@ if($_SESSION['otp']['type'] == 'email') { //* set code if(!isset($_SESSION['otp']['code']) || empty($_SESSION['otp']['code'])) { - // Random int between 10^($code_length-1) and 10^$code_length - $_SESSION['otp']['code'] = rand(pow(10, $code_length - 1), pow(10, $code_length) - 1); + $_SESSION['otp']['code'] = random_int(100000, 999999); $_SESSION['otp']['starttime'] = time(); } -- GitLab From 1176d8480288079b8d37965ef7761074c96ad745 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 22:01:34 +0100 Subject: [PATCH 23/53] Better csrf_token name --- interface/web/login/otp.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 091152bc0e..db5e14cdae 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -190,7 +190,7 @@ $app->tpl->setInclude('content_tpl', 'templates/otp.htm'); //* SET csrf token -$csrf_token = $app->auth->csrf_token_get('language_edit'); +$csrf_token = $app->auth->csrf_token_get('otp'); $app->tpl->setVar('_csrf_id',$csrf_token['csrf_id']); $app->tpl->setVar('_csrf_key',$csrf_token['csrf_key']); #$app->tpl->setVar('msg', print_r($_SESSION['otp'], 1)); -- GitLab From 361a5efd77ab9e6935e33b9b6e168ab1bc1dd7ec Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 22:12:24 +0100 Subject: [PATCH 24/53] Increment otp_attempts also for otp_attempts --- interface/web/login/otp.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index db5e14cdae..24ef031b71 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -89,6 +89,9 @@ if(isset($_POST['code']) && strlen($_POST['code']) == $otp_recovery_code_length if ($_SESSION['otp']['recovery'] == $_POST['code']) { finish_2fa_success('via 2fa recovery code'); } + else { + $app->db->query('UPDATE `sys_user` SET otp_attempts=otp_attempts + 1 WHERE userid = ?', $_SESSION['s_pending']['user']['userid']); + } } -- GitLab From 4646aba11cae8fae310429ce400434749e3499c6 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 22:12:56 +0100 Subject: [PATCH 25/53] Session vs global/db-backed attempts --- interface/web/login/otp.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 24ef031b71..46da367804 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -125,7 +125,7 @@ if($_SESSION['otp']['type'] == 'email') { finish_2fa_success(); } else { //* 2fa wrong code - $_SESSION['otp']['session_attempts']++; // FIXME can't we skip this and rely on the DB only? + $_SESSION['otp']['session_attempts']++; $app->db->query('UPDATE `sys_user` SET otp_attempts=otp_attempts + 1 WHERE userid = ?', $_SESSION['s_pending']['user']['userid']); } } -- GitLab From 9740dd1e001b4d4e2f04641ba573fe7a74762b27 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 22:16:48 +0100 Subject: [PATCH 26/53] whitespace --- interface/web/login/otp.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 46da367804..7aba74530f 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -163,7 +163,7 @@ if($_SESSION['otp']['type'] == 'email') { } else { $_SESSION['otp']['sent']++; } - + } //* Show form to enter email code -- GitLab From fd4a1849e704325f4b245c4999047200a7e4168f Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 22:24:30 +0100 Subject: [PATCH 27/53] Hash the otp code --- interface/web/login/otp.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 7aba74530f..35c3046d20 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -121,7 +121,7 @@ if($_SESSION['otp']['type'] == 'email') { } //* 2fa success - if($_POST['code'] == $_SESSION['otp']['code']) { + if(password_verify($_POST['code'], $_SESSION['otp']['code_hash'])) { finish_2fa_success(); } else { //* 2fa wrong code @@ -132,7 +132,8 @@ if($_SESSION['otp']['type'] == 'email') { //* set code if(!isset($_SESSION['otp']['code']) || empty($_SESSION['otp']['code'])) { - $_SESSION['otp']['code'] = random_int(100000, 999999); + $new_otp_code = random_int(100000, 999999); + $_SESSION['otp']['code_hash'] = password_hash($new_otp_code, PASSWORD_DEFAULT); $_SESSION['otp']['starttime'] = time(); } @@ -152,7 +153,7 @@ if($_SESSION['otp']['type'] == 'email') { //* send email $email_to = $_SESSION['otp']['data']; $subject = 'ISPConfig Login authentication'; - $text = 'Your One time login code is ' . $_SESSION['otp']['code'] . PHP_EOL + $text = 'Your One time login code is ' . $new_otp_code . PHP_EOL . 'This code is valid for 10 minutes' . PHP_EOL; $app->functions->mail($email_to, $subject, $text, $from); -- GitLab From d1d5bfe10d15bfa95284c62bbd60ed7d0144fbf2 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 22:43:33 +0100 Subject: [PATCH 28/53] Expect otp_recovery to be hashed --- interface/web/login/otp.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 35c3046d20..44f3d2fed2 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -86,7 +86,7 @@ if(isset($_POST['code']) && strlen($_POST['code']) == $otp_recovery_code_length die("Sorry, contact your administrator."); } - if ($_SESSION['otp']['recovery'] == $_POST['code']) { + if (password_verify($_POST['code'], $user['otp_recovery'])) { finish_2fa_success('via 2fa recovery code'); } else { -- GitLab From ee4ab7694b25907ee82ca4c41a44b0770a9b24d2 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 11 Mar 2022 22:50:41 +0100 Subject: [PATCH 29/53] Keep otp_recovery out of the session --- interface/web/login/index.php | 1 - interface/web/login/otp.php | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/interface/web/login/index.php b/interface/web/login/index.php index b5b86be615..7be4e2c365 100644 --- a/interface/web/login/index.php +++ b/interface/web/login/index.php @@ -148,7 +148,6 @@ function process_login_request(app $app, &$error, $conf, $module) $_SESSION['otp']['session_attempts'] = 0; $_SESSION['otp']['type'] = $user['otp_type']; $_SESSION['otp']['data'] = $user['otp_data']; - $_SESSION['otp']['recovery'] = $user['otp_recovery']; //* Redirect to otp script header('Location: otp.php'); diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 44f3d2fed2..8312d6dc53 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -76,10 +76,10 @@ function finish_2fa_success($msg = '') { } //* Handle recovery code -if(isset($_POST['code']) && strlen($_POST['code']) == $otp_recovery_code_length && $_SESSION['otp']['recovery']) { +if(isset($_POST['code']) && strlen($_POST['code']) == $otp_recovery_code_length) { //* TODO Recovery code handling - $user = $app->db->queryOneRecord('SELECT otp_attempts FROM sys_user WHERE userid = ?',$_SESSION['s_pending']['user']['userid']); + $user = $app->db->queryOneRecord('SELECT otp_attempts, otp_recovery FROM sys_user WHERE userid = ?',$_SESSION['s_pending']['user']['userid']); //* We allow one more try to enter recovery code if($user['otp_attempts'] > $max_global_code_retry + 1) { -- GitLab From 413ec5c672296a20cd3ad7ea394cd2de389bf0c8 Mon Sep 17 00:00:00 2001 From: Thom Date: Fri, 18 Mar 2022 18:43:51 +0000 Subject: [PATCH 30/53] Apply 1 suggestion(s) to 1 file(s) --- interface/web/tools/lib/lang/en_usersettings.lng | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/web/tools/lib/lang/en_usersettings.lng b/interface/web/tools/lib/lang/en_usersettings.lng index 2f052a6e94..458eef7606 100644 --- a/interface/web/tools/lib/lang/en_usersettings.lng +++ b/interface/web/tools/lib/lang/en_usersettings.lng @@ -16,5 +16,5 @@ $wb['startmodule_empty'] = 'Startmodule empty.'; $wb['startmodule_regex'] = 'Invalid chars in Startmodule.'; $wb['app_theme_empty'] = 'App theme empty.'; $wb['app_theme_regex'] = 'Invalid chars in App theme.'; -$wb['otp_auth_txt'] = '2-Factor Authentication'; +$wb['otp_auth_txt'] = 'Two Factor Authentication'; ?> -- GitLab From fdeb086aabd851b5a08103df911a9ae6e0767df7 Mon Sep 17 00:00:00 2001 From: Thom Date: Fri, 18 Mar 2022 18:43:59 +0000 Subject: [PATCH 31/53] Apply 1 suggestion(s) to 1 file(s) --- interface/web/login/lib/lang/en.lng | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/web/login/lib/lang/en.lng b/interface/web/login/lib/lang/en.lng index 5b411688f2..e781498b7a 100644 --- a/interface/web/login/lib/lang/en.lng +++ b/interface/web/login/lib/lang/en.lng @@ -32,7 +32,7 @@ $wb['lost_password_function_disabled_txt'] = 'The lost password function is not $wb['lost_password_function_wait_txt'] = 'You cannot request a new password yet. Please wait a few minutes.'; $wb['lost_password_function_expired_txt'] = 'This activation link has expired. Please request a new one.'; $wb['lost_password_function_denied_txt'] = 'This activation link is not valid.'; -$wb['otp_code_txt'] = '2-Factor Authentication'; +$wb['otp_code_txt'] = 'Two Factor Authentication'; $wb['otp_code_desc_txt'] = 'Enter the code you got from your authenticator app or via email.'; $wb['otp_code_placeholder_txt'] = 'OTP code'; $wb['otp_code_resend_txt'] = 'Request new code'; -- GitLab From 67591236fab8ee579d42ef7045bc3eda76a8502e Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Sat, 12 Mar 2022 15:51:42 +0100 Subject: [PATCH 32/53] Add commented Debug code --- interface/web/login/index.php | 1 + interface/web/login/otp.php | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/interface/web/login/index.php b/interface/web/login/index.php index 7be4e2c365..ad08c92a79 100644 --- a/interface/web/login/index.php +++ b/interface/web/login/index.php @@ -148,6 +148,7 @@ function process_login_request(app $app, &$error, $conf, $module) $_SESSION['otp']['session_attempts'] = 0; $_SESSION['otp']['type'] = $user['otp_type']; $_SESSION['otp']['data'] = $user['otp_data']; + //$_SESSION['otp']['recovery_debug'] = $user['otp_recovery']; // For DEBUG only. //* Redirect to otp script header('Location: otp.php'); diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 8312d6dc53..894a870336 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -134,6 +134,7 @@ if($_SESSION['otp']['type'] == 'email') { if(!isset($_SESSION['otp']['code']) || empty($_SESSION['otp']['code'])) { $new_otp_code = random_int(100000, 999999); $_SESSION['otp']['code_hash'] = password_hash($new_otp_code, PASSWORD_DEFAULT); + //$_SESSION['otp']['code_debug'] = $new_otp_code; # for DEBUG only. $_SESSION['otp']['starttime'] = time(); } @@ -197,7 +198,7 @@ $app->tpl->setInclude('content_tpl', 'templates/otp.htm'); $csrf_token = $app->auth->csrf_token_get('otp'); $app->tpl->setVar('_csrf_id',$csrf_token['csrf_id']); $app->tpl->setVar('_csrf_key',$csrf_token['csrf_key']); -#$app->tpl->setVar('msg', print_r($_SESSION['otp'], 1)); +//$app->tpl->setVar('msg', print_r($_SESSION['otp'], 1)); // For DEBUG only. require ISPC_ROOT_PATH.'/web/login/lib/lang/'.$app->functions->check_language($conf['language']).'.lng'; $app->tpl->setVar($wb); -- GitLab From c2c042779e4ae770562041281c3f7c43f4db57d5 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Fri, 18 Mar 2022 22:28:15 +0100 Subject: [PATCH 33/53] Document otp_enabled=v in a comment --- install/sql/incremental/upd_dev_collection.sql | 2 +- interface/web/login/otp.php | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/install/sql/incremental/upd_dev_collection.sql b/install/sql/incremental/upd_dev_collection.sql index f0066d5180..c499579f64 100644 --- a/install/sql/incremental/upd_dev_collection.sql +++ b/install/sql/incremental/upd_dev_collection.sql @@ -18,4 +18,4 @@ INSERT INTO `mail_relay_domain` SELECT NULL, `sys_userid`, `sys_groupid`, `sys_p ALTER TABLE `dns_soa` ADD `rendered_zone` MEDIUMTEXT NULL AFTER `dnssec_info`; -ALTER TABLE `sys_user` ADD `otp_enabled` SET('n', 'y','v') NOT NULL DEFAULT 'n' AFTER `lost_password_reqtime`, ADD `otp_type` SET('email') NOT NULL DEFAULT 'email' AFTER `otp_enabled`, ADD `otp_data` VARCHAR(255) NULL AFTER `otp_type`, ADD `otp_recovery` VARCHAR(64) NULL AFTER `otp_data`, ADD `otp_attempts` TINYINT NOT NULL DEFAULT '0' AFTER `otp_recovery`; +ALTER TABLE `sys_user` ADD `otp_enabled` SET('n', 'y','v') NOT NULL DEFAULT 'n' COMMENT 'v=waiting for validation of the chosen otp method' AFTER `lost_password_reqtime`, ADD `otp_type` SET('email') NOT NULL DEFAULT 'email' AFTER `otp_enabled`, ADD `otp_data` VARCHAR(255) NULL AFTER `otp_type`, ADD `otp_recovery` VARCHAR(64) NULL AFTER `otp_data`, ADD `otp_attempts` TINYINT NOT NULL DEFAULT '0' AFTER `otp_recovery`; diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 894a870336..7acc8b6f12 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -56,9 +56,6 @@ if(count($_POST) >= 1) { } -// FIXME What's the deal with otp_enabled=v ?? - - function finish_2fa_success($msg = '') { global $app; $_SESSION['s'] = $_SESSION['s_pending']; -- GitLab From 29ef4521c086b8668f3932d7d0b6862d2ef0fffd Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Sun, 27 Mar 2022 13:27:37 +0200 Subject: [PATCH 34/53] Print that we sent an email --- interface/web/login/lib/lang/en.lng | 1 + interface/web/login/otp.php | 7 +++++-- interface/web/login/templates/otp.htm | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/interface/web/login/lib/lang/en.lng b/interface/web/login/lib/lang/en.lng index e781498b7a..af01fb2a2c 100644 --- a/interface/web/login/lib/lang/en.lng +++ b/interface/web/login/lib/lang/en.lng @@ -35,5 +35,6 @@ $wb['lost_password_function_denied_txt'] = 'This activation link is not valid.'; $wb['otp_code_txt'] = 'Two Factor Authentication'; $wb['otp_code_desc_txt'] = 'Enter the code you got from your authenticator app or via email.'; $wb['otp_code_placeholder_txt'] = 'OTP code'; +$wb['otp_code_email_sent_txt'] = 'An email was sent to'; $wb['otp_code_resend_txt'] = 'Request new code'; ?> diff --git a/interface/web/login/otp.php b/interface/web/login/otp.php index 7acc8b6f12..d15538305c 100644 --- a/interface/web/login/otp.php +++ b/interface/web/login/otp.php @@ -55,6 +55,7 @@ if(count($_POST) >= 1) { $app->auth->csrf_token_check(); } +require ISPC_ROOT_PATH.'/web/login/lib/lang/'.$app->functions->check_language($conf['language']).'.lng'; function finish_2fa_success($msg = '') { global $app; @@ -162,6 +163,7 @@ if($_SESSION['otp']['type'] == 'email') { } else { $_SESSION['otp']['sent']++; } + $token_sent_message = $wb['otp_code_email_sent_txt'] . ' ' . $email_to; } @@ -183,7 +185,9 @@ if($logo['custom_logo'] != ''){ $app->tpl->setVar('base64_logo_txt', $base64_logo_txt); $app->tpl->setVar('current_theme', isset($_SESSION['s']['theme']) ? $_SESSION['s']['theme'] : 'default', true); - +if (!empty($token_sent_message)) { + $app->tpl->setVar('token_sent_message', $token_sent_message); +} //* Load templating system and lang file $app->uses('tpl'); @@ -197,7 +201,6 @@ $app->tpl->setVar('_csrf_id',$csrf_token['csrf_id']); $app->tpl->setVar('_csrf_key',$csrf_token['csrf_key']); //$app->tpl->setVar('msg', print_r($_SESSION['otp'], 1)); // For DEBUG only. -require ISPC_ROOT_PATH.'/web/login/lib/lang/'.$app->functions->check_language($conf['language']).'.lng'; $app->tpl->setVar($wb); $app->tpl_defaults(); diff --git a/interface/web/login/templates/otp.htm b/interface/web/login/templates/otp.htm index 716b1660cd..cf49ba8108 100755 --- a/interface/web/login/templates/otp.htm +++ b/interface/web/login/templates/otp.htm @@ -14,6 +14,7 @@
+ {tmpl_var name='token_sent_message'}
 {tmpl_var name='otp_code_resend_txt'} -- GitLab From 9038c4344baedbc99d9de478dc355426529eba19 Mon Sep 17 00:00:00 2001 From: Herman van Rink Date: Sun, 27 Mar 2022 13:38:55 +0200 Subject: [PATCH 35/53] fix label name --- interface/web/admin/templates/users_user_edit.htm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/web/admin/templates/users_user_edit.htm b/interface/web/admin/templates/users_user_edit.htm index 9451b4b100..4e8e3da496 100644 --- a/interface/web/admin/templates/users_user_edit.htm +++ b/interface/web/admin/templates/users_user_edit.htm @@ -29,7 +29,7 @@
- +