From f396bd51846a85b8835857ca11b9ef0264512397 Mon Sep 17 00:00:00 2001 From: Michael Kaufmann Date: Mon, 10 Jul 2023 13:40:48 +0200 Subject: [PATCH] add otp security check to critical settings Signed-off-by: Michael Kaufmann --- actions/admin/settings/120.system.php | 3 +- actions/admin/settings/125.cronjob.php | 9 ++- actions/admin/settings/130.webserver.php | 6 +- actions/admin/settings/131.ssl.php | 3 +- actions/admin/settings/136.phpfpm.php | 12 ++-- actions/admin/settings/160.nameserver.php | 6 +- actions/admin/settings/180.dkim.php | 3 +- actions/admin/settings/210.security.php | 24 ++++--- actions/admin/settings/220.quota.php | 12 +++- admin_settings.php | 3 +- lib/Froxlor/CurrentUser.php | 64 +++++++++++++++++++ lib/Froxlor/Settings.php | 3 +- lib/Froxlor/UI/Form.php | 49 ++++++++++++++ lib/Froxlor/UI/HTML.php | 13 ++++ lib/config.example.inc.php | 5 ++ .../phpconfig/formfield.fpmconfig_add.php | 3 +- .../phpconfig/formfield.fpmconfig_edit.php | 3 +- .../phpconfig/formfield.phpconfig_add.php | 9 ++- .../phpconfig/formfield.phpconfig_edit.php | 9 ++- templates/Froxlor/form/otpquestion.html.twig | 31 +++++++++ 20 files changed, 235 insertions(+), 35 deletions(-) create mode 100644 templates/Froxlor/form/otpquestion.html.twig diff --git a/actions/admin/settings/120.system.php b/actions/admin/settings/120.system.php index 339ed9a0..09f3da7d 100644 --- a/actions/admin/settings/120.system.php +++ b/actions/admin/settings/120.system.php @@ -107,7 +107,8 @@ return [ 'varname' => 'enabled', 'type' => 'checkbox', 'default' => false, - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ], 'api_customer_default' => [ 'label' => lng('serversettings.api_customer_default'), diff --git a/actions/admin/settings/125.cronjob.php b/actions/admin/settings/125.cronjob.php index a04a8394..6c26bd11 100644 --- a/actions/admin/settings/125.cronjob.php +++ b/actions/admin/settings/125.cronjob.php @@ -46,7 +46,8 @@ return [ 'type' => 'text', 'string_regexp' => '/^[a-z0-9\/\._\- ]+$/i', 'default' => '/usr/bin/nice -n 5 /usr/bin/php -q', - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ], 'system_crondreload' => [ 'label' => lng('serversettings.system_crondreload'), @@ -55,7 +56,8 @@ return [ 'type' => 'text', 'string_regexp' => '/^[a-z0-9\/\._\- ]+$/i', 'default' => '/etc/init.d/cron reload', - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ], 'system_cron_allowautoupdate' => [ 'label' => lng('serversettings.system_cron_allowautoupdate'), @@ -63,7 +65,8 @@ return [ 'varname' => 'cron_allowautoupdate', 'type' => 'checkbox', 'default' => false, - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ] ] ] diff --git a/actions/admin/settings/130.webserver.php b/actions/admin/settings/130.webserver.php index e9e62dbb..84861fd9 100644 --- a/actions/admin/settings/130.webserver.php +++ b/actions/admin/settings/130.webserver.php @@ -308,7 +308,8 @@ return [ 'type' => 'text', 'string_regexp' => '/^[a-z0-9\/\._\- ]+$/i', 'default' => '/etc/init.d/apache2 reload', - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ], 'system_phpreload_command' => [ 'label' => lng('serversettings.phpreload_command'), @@ -320,7 +321,8 @@ return [ 'save_method' => 'storeSettingField', 'websrv_avail' => [ 'nginx' - ] + ], + 'required_otp' => true ], 'system_nginx_php_backend' => [ 'label' => lng('serversettings.nginx_php_backend'), diff --git a/actions/admin/settings/131.ssl.php b/actions/admin/settings/131.ssl.php index e2e00b0d..023f10a8 100644 --- a/actions/admin/settings/131.ssl.php +++ b/actions/admin/settings/131.ssl.php @@ -157,7 +157,8 @@ return [ 'string_type' => 'file', 'default' => '/root/.acme.sh/acme.sh', 'save_method' => 'storeSettingField', - 'advanced_mode' => true + 'advanced_mode' => true, + 'required_otp' => true ], 'system_letsencryptacmeconf' => [ 'label' => lng('serversettings.letsencryptacmeconf'), diff --git a/actions/admin/settings/136.phpfpm.php b/actions/admin/settings/136.phpfpm.php index a408c481..68e64517 100644 --- a/actions/admin/settings/136.phpfpm.php +++ b/actions/admin/settings/136.phpfpm.php @@ -126,7 +126,8 @@ return [ 'type' => 'textarea', 'default' => '', 'save_method' => 'storeSettingField', - 'advanced_mode' => true + 'advanced_mode' => true, + 'required_otp' => true ], 'phpfpm_ini_values' => [ 'label' => lng('phpfpm.ini_values'), @@ -135,7 +136,8 @@ return [ 'type' => 'textarea', 'default' => '', 'save_method' => 'storeSettingField', - 'advanced_mode' => true + 'advanced_mode' => true, + 'required_otp' => true ], 'phpfpm_ini_admin_flags' => [ 'label' => lng('phpfpm.ini_admin_flags'), @@ -144,7 +146,8 @@ return [ 'type' => 'textarea', 'default' => '', 'save_method' => 'storeSettingField', - 'advanced_mode' => true + 'advanced_mode' => true, + 'required_otp' => true ], 'phpfpm_ini_admin_values' => [ 'label' => lng('phpfpm.ini_admin_values'), @@ -153,7 +156,8 @@ return [ 'type' => 'textarea', 'default' => '', 'save_method' => 'storeSettingField', - 'advanced_mode' => true + 'advanced_mode' => true, + 'required_otp' => true ] ] ] diff --git a/actions/admin/settings/160.nameserver.php b/actions/admin/settings/160.nameserver.php index 5d6906b0..184d5991 100644 --- a/actions/admin/settings/160.nameserver.php +++ b/actions/admin/settings/160.nameserver.php @@ -80,7 +80,8 @@ return [ 'type' => 'text', 'string_regexp' => '/^[a-z0-9\/\._\- ]+$/i', 'default' => '/etc/init.d/bind9 reload', - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ], 'system_nameservers' => [ 'label' => lng('serversettings.nameservers'), @@ -111,7 +112,8 @@ return [ 'string_delimiter' => ',', 'string_emptyallowed' => true, 'default' => '', - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ], 'system_powerdns_mode' => [ 'label' => lng('serversettings.powerdns_mode'), diff --git a/actions/admin/settings/180.dkim.php b/actions/admin/settings/180.dkim.php index 7b2aada5..2feb67bc 100644 --- a/actions/admin/settings/180.dkim.php +++ b/actions/admin/settings/180.dkim.php @@ -137,7 +137,8 @@ return [ 'type' => 'text', 'string_regexp' => '/^[a-z0-9\/\._\- ]+$/i', 'default' => '/etc/init.d/dkim-filter restart', - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ] ] ] diff --git a/actions/admin/settings/210.security.php b/actions/admin/settings/210.security.php index cf0712a8..c45fa615 100644 --- a/actions/admin/settings/210.security.php +++ b/actions/admin/settings/210.security.php @@ -37,7 +37,8 @@ return [ 'varname' => 'unix_names', 'type' => 'checkbox', 'default' => true, - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ], 'system_mailpwcleartext' => [ 'label' => lng('serversettings.mailpwcleartext'), @@ -46,7 +47,8 @@ return [ 'type' => 'checkbox', 'default' => false, 'save_method' => 'storeSettingField', - 'advanced_mode' => true + 'advanced_mode' => true, + 'required_otp' => true ], 'system_passwordcryptfunc' => [ 'label' => lng('serversettings.passwordcryptfunc'), @@ -59,7 +61,8 @@ return [ 'getAvailablePasswordHashes' ], 'save_method' => 'storeSettingField', - 'advanced_mode' => true + 'advanced_mode' => true, + 'required_otp' => true ], 'system_allow_error_report_admin' => [ 'label' => lng('serversettings.allow_error_report_admin'), @@ -67,7 +70,8 @@ return [ 'varname' => 'allow_error_report_admin', 'type' => 'checkbox', 'default' => false, - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ], 'system_allow_error_report_customer' => [ 'label' => lng('serversettings.allow_error_report_customer'), @@ -75,7 +79,8 @@ return [ 'varname' => 'allow_error_report_customer', 'type' => 'checkbox', 'default' => false, - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ], 'system_allow_customer_shell' => [ 'label' => lng('serversettings.allow_allow_customer_shell'), @@ -84,7 +89,8 @@ return [ 'type' => 'checkbox', 'default' => false, 'save_method' => 'storeSettingField', - 'advanced_mode' => true + 'advanced_mode' => true, + 'required_otp' => true ], 'system_available_shells' => [ 'label' => lng('serversettings.available_shells'), @@ -94,7 +100,8 @@ return [ 'string_emptyallowed' => true, 'default' => '', 'save_method' => 'storeSettingField', - 'advanced_mode' => true + 'advanced_mode' => true, + 'required_otp' => true ], 'system_froxlorusergroup' => [ 'label' => lng('serversettings.froxlorusergroup'), @@ -108,7 +115,8 @@ return [ 'checkLocalGroup' ], 'visible' => Settings::Get('system.nssextrausers'), - 'advanced_mode' => true + 'advanced_mode' => true, + 'required_otp' => true ], ] ] diff --git a/actions/admin/settings/220.quota.php b/actions/admin/settings/220.quota.php index b7682bd1..8c0bbab3 100644 --- a/actions/admin/settings/220.quota.php +++ b/actions/admin/settings/220.quota.php @@ -44,24 +44,30 @@ return [ 'settinggroup' => 'system', 'varname' => 'diskquota_repquota_path', 'type' => 'text', + 'string_type' => 'file', 'default' => '/usr/sbin/repquota', - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ], 'system_diskquota_quotatool_path' => [ 'label' => lng('serversettings.diskquota_quotatool_path.description'), 'settinggroup' => 'system', 'varname' => 'diskquota_quotatool_path', 'type' => 'text', + 'string_type' => 'file', 'default' => '/usr/bin/quotatool', - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ], 'system_diskquota_customer_partition' => [ 'label' => lng('serversettings.diskquota_customer_partition.description'), 'settinggroup' => 'system', 'varname' => 'diskquota_customer_partition', 'type' => 'text', + 'string_type' => 'file', 'default' => '/dev/root', - 'save_method' => 'storeSettingField' + 'save_method' => 'storeSettingField', + 'required_otp' => true ] ] ] diff --git a/admin_settings.php b/admin_settings.php index b3c84e72..8d6fe497 100644 --- a/admin_settings.php +++ b/admin_settings.php @@ -77,7 +77,8 @@ if ($page == 'overview' && $userinfo['change_serversettings'] == '1') { if (Form::processForm($settings_data, $_POST, [ 'filename' => $filename, 'action' => $action, - 'page' => $page + 'page' => $page, + 'part' => $_part, ], $_part, $settings_all, $settings_part, $only_enabledisable)) { $log->logAction(FroxlorLogger::ADM_ACTION, LOG_INFO, "rebuild configfiles due to changed setting"); Cronjob::inserttask(TaskId::REBUILD_VHOST); diff --git a/lib/Froxlor/CurrentUser.php b/lib/Froxlor/CurrentUser.php index 45168176..a6796b3a 100644 --- a/lib/Froxlor/CurrentUser.php +++ b/lib/Froxlor/CurrentUser.php @@ -25,10 +25,13 @@ namespace Froxlor; +use Exception; use Froxlor\Api\Commands\Customers; use Froxlor\Api\Commands\SubDomains; use Froxlor\Database\Database; use Froxlor\UI\Collection; +use Froxlor\UI\Response; +use RobThree\Auth\TwoFactorAuthException; /** * Class to manage the current user / session @@ -169,4 +172,65 @@ class CurrentUser return ($_SESSION['userinfo'][$resource . '_used'] < $_SESSION['userinfo'][$resource] || $_SESSION['userinfo'][$resource] == '-1') && $addition; } + + /** + * @throws TwoFactorAuthException + */ + public static function sendOtpEmail() + { + global $mail; + + if (self::getField('type_2fa') == 1) { + // generate code + $tfa = new FroxlorTwoFactorAuth('Froxlor ' . Settings::Get('system.hostname')); + $code = $tfa->getCode($tfa->createSecret()); + // set code for user + $table = TABLE_PANEL_CUSTOMERS; + $uid = 'customerid'; + if (self::isAdmin()) { + $table = TABLE_PANEL_ADMINS; + $uid = 'adminid'; + } + $stmt = Database::prepare("UPDATE $table SET `data_2fa` = :d2fa WHERE `$uid` = :uid"); + Database::pexecute($stmt, [ + "d2fa" => $code, + "uid" => self::getField($uid) + ]); + // build up & send email + $_mailerror = false; + $mailerr_msg = ""; + $replace_arr = [ + 'CODE' => $code + ]; + $mail_body = html_entity_decode(PhpHelper::replaceVariables(lng('mails.2fa.mailbody'), $replace_arr)); + + try { + $mail->Subject = lng('mails.2fa.subject'); + $mail->AltBody = $mail_body; + $mail->MsgHTML(str_replace("\n", "
", $mail_body)); + $mail->AddAddress(self::getField('email'), User::getCorrectUserSalutation(self::getData())); + $mail->Send(); + } catch (\PHPMailer\PHPMailer\Exception $e) { + $mailerr_msg = $e->errorMessage(); + $_mailerror = true; + } catch (Exception $e) { + $mailerr_msg = $e->getMessage(); + $_mailerror = true; + } + + if ($_mailerror) { + $rstlog = FroxlorLogger::getInstanceOf([ + 'loginname' => '2fa code-sending' + ]); + $rstlog->logAction(FroxlorLogger::ADM_ACTION, LOG_ERR, "Error sending mail: " . $mailerr_msg); + Response::redirectTo('index.php', [ + 'showmessage' => '4', + 'customermail' => self::getField('email') + ]); + exit(); + } + + $mail->ClearAddresses(); + } + } } diff --git a/lib/Froxlor/Settings.php b/lib/Froxlor/Settings.php index d24a1c8a..dad81814 100644 --- a/lib/Froxlor/Settings.php +++ b/lib/Froxlor/Settings.php @@ -129,7 +129,8 @@ class Settings { // set defaults self::$conf = [ - 'enable_webupdate' => false + 'enable_webupdate' => false, + 'disable_otp_security_check' => false, ]; $configfile = Froxlor::getInstallDir() . '/lib/config.inc.php'; diff --git a/lib/Froxlor/UI/Form.php b/lib/Froxlor/UI/Form.php index acf0cdd8..17fafaa6 100644 --- a/lib/Froxlor/UI/Form.php +++ b/lib/Froxlor/UI/Form.php @@ -25,6 +25,8 @@ namespace Froxlor\UI; +use Froxlor\CurrentUser; +use Froxlor\FroxlorTwoFactorAuth; use Froxlor\Settings; use Froxlor\Validate\Check; @@ -183,6 +185,21 @@ class Form } } + // OTP security validation for sensitive settings + if (!Settings::Config('disable_otp_security_check') && isset($fielddata['required_otp']) && $do_show) { + $otp_enabled_system = (bool)Settings::Get('2fa.enabled'); + $otp_enabled_user = (int)CurrentUser::getField('type_2fa') != 0; + $do_show = !$fielddata['required_otp'] || ($otp_enabled_system && $otp_enabled_user); + if (!$do_show) { + $fielddata['note'] = lng('serversettings.option_required_otp'); + if (!$otp_enabled_system) { + $fielddata['note'] .= '
' . lng('2fa.2fa_not_activated'); + } elseif (!$otp_enabled_user) { + $fielddata['note'] .= '
' . lng('2fa.2fa_not_activated_for_user'); + } + } + } + if (!$do_show) { $fielddata['visible'] = false; } @@ -283,6 +300,38 @@ class Form } } } + if (!Settings::Config('disable_otp_security_check') && isset($fielddetails['required_otp']) && isset($changed_fields[$fieldname])) { + $otp_enabled_system = (bool)Settings::Get('2fa.enabled'); + $otp_enabled_user = (int)CurrentUser::getField('type_2fa') != 0; + $do_update = !$fielddetails['required_otp'] || ($otp_enabled_system && $otp_enabled_user); + if ($do_update) { + // setting that requires OTP verification + if (empty($input['otp_verification'])) { + // in case email 2fa is enabled, send it now + CurrentUser::sendOtpEmail(); + // build up form + if (is_array($url_params) && isset($url_params['filename'])) { + $filename = $url_params['filename']; + unset($url_params['filename']); + } else { + $filename = ''; + } + HTML::askOTP('please_enter_otp', $filename, array_merge($url_params, $submitted_fields)); + } else { + // validate given OTP code + $code = trim($input['otp_verification']); + $tfa = new FroxlorTwoFactorAuth('Froxlor ' . Settings::Get('system.hostname')); + $result = $tfa->verifyCode(CurrentUser::getField('data_2fa'), $code, 3); + if (!$result) { + Response::standardError('otpnotvalidated'); + } + } + } else { + // do not update this setting + unset($changed_fields[$fieldname]); + } + } + } } } diff --git a/lib/Froxlor/UI/HTML.php b/lib/Froxlor/UI/HTML.php index acf635a3..6fc398df 100644 --- a/lib/Froxlor/UI/HTML.php +++ b/lib/Froxlor/UI/HTML.php @@ -221,4 +221,17 @@ class HTML ]); exit(); } + + public static function askOTP(string $text, string $targetfile, array $params = [], string $replacer = '', array $back_link = []) + { + $text = lng('question.' . $text, [htmlspecialchars($replacer)]); + + Panel\UI::view('form/otpquestion.html.twig', [ + 'action' => $targetfile, + 'url_params' => $params, + 'question' => $text, + 'back_link' => $back_link + ]); + exit(); + } } diff --git a/lib/config.example.inc.php b/lib/config.example.inc.php index 7f34f735..cdec9462 100644 --- a/lib/config.example.inc.php +++ b/lib/config.example.inc.php @@ -11,4 +11,9 @@ return [ * updates the way the providers does it (e.g. automation, etc.) */ 'enable_webupdate' => false, + + /** + * @todo description + */ + 'disable_otp_security_check' => false, ]; diff --git a/lib/formfields/admin/phpconfig/formfield.fpmconfig_add.php b/lib/formfields/admin/phpconfig/formfield.fpmconfig_add.php index 0f8d005d..a2267655 100644 --- a/lib/formfields/admin/phpconfig/formfield.fpmconfig_add.php +++ b/lib/formfields/admin/phpconfig/formfield.fpmconfig_add.php @@ -44,7 +44,8 @@ return [ 'type' => 'text', 'maxlength' => 255, 'value' => 'service php7.4-fpm restart', - 'mandatory' => true + 'mandatory' => true, + 'required_otp' => true ], 'config_dir' => [ 'label' => lng('serversettings.phpfpm_settings.configdir'), diff --git a/lib/formfields/admin/phpconfig/formfield.fpmconfig_edit.php b/lib/formfields/admin/phpconfig/formfield.fpmconfig_edit.php index 49f5daf7..d52437f0 100644 --- a/lib/formfields/admin/phpconfig/formfield.fpmconfig_edit.php +++ b/lib/formfields/admin/phpconfig/formfield.fpmconfig_edit.php @@ -45,7 +45,8 @@ return [ 'type' => 'text', 'maxlength' => 255, 'value' => $result['reload_cmd'], - 'mandatory' => true + 'mandatory' => true, + 'required_otp' => true ], 'config_dir' => [ 'label' => lng('serversettings.phpfpm_settings.configdir'), diff --git a/lib/formfields/admin/phpconfig/formfield.phpconfig_add.php b/lib/formfields/admin/phpconfig/formfield.phpconfig_add.php index 1decbbd7..157171f0 100644 --- a/lib/formfields/admin/phpconfig/formfield.phpconfig_add.php +++ b/lib/formfields/admin/phpconfig/formfield.phpconfig_add.php @@ -46,7 +46,8 @@ return [ 'label' => lng('admin.phpsettings.binary'), 'type' => 'text', 'maxlength' => 255, - 'value' => '/usr/bin/php-cgi' + 'value' => '/usr/bin/php-cgi', + 'required_otp' => true ], 'fpmconfig' => [ 'visible' => Settings::Get('phpfpm.enabled') == 1, @@ -61,7 +62,8 @@ return [ 'desc' => lng('admin.phpsettings.file_extensions_note'), 'type' => 'text', 'maxlength' => 255, - 'value' => 'php' + 'value' => 'php', + 'required_otp' => true ], 'mod_fcgid_starter' => [ 'visible' => Settings::Get('system.mod_fcgid') == 1, @@ -181,7 +183,8 @@ return [ 'cols' => 80, 'rows' => 20, 'value' => $result['phpsettings'], - 'mandatory' => true + 'mandatory' => true, + 'required_otp' => true ], 'allow_all_customers' => [ 'label' => lng('serversettings.phpfpm_settings.allow_all_customers.title'), diff --git a/lib/formfields/admin/phpconfig/formfield.phpconfig_edit.php b/lib/formfields/admin/phpconfig/formfield.phpconfig_edit.php index 11d59401..8216c27b 100644 --- a/lib/formfields/admin/phpconfig/formfield.phpconfig_edit.php +++ b/lib/formfields/admin/phpconfig/formfield.phpconfig_edit.php @@ -47,7 +47,8 @@ return [ 'label' => lng('admin.phpsettings.binary'), 'type' => 'text', 'maxlength' => 255, - 'value' => $result['binary'] + 'value' => $result['binary'], + 'required_otp' => true ], 'fpmconfig' => [ 'visible' => Settings::Get('phpfpm.enabled') == 1, @@ -62,7 +63,8 @@ return [ 'desc' => lng('admin.phpsettings.file_extensions_note'), 'type' => 'text', 'maxlength' => 255, - 'value' => $result['file_extensions'] + 'value' => $result['file_extensions'], + 'required_otp' => true ], 'mod_fcgid_starter' => [ 'visible' => Settings::Get('system.mod_fcgid') == 1, @@ -185,7 +187,8 @@ return [ 'cols' => 80, 'rows' => 20, 'value' => $result['phpsettings'], - 'mandatory' => true + 'mandatory' => true, + 'required_otp' => true ], 'allow_all_customers' => [ 'label' => lng('serversettings.phpfpm_settings.allow_all_customers.title'), diff --git a/templates/Froxlor/form/otpquestion.html.twig b/templates/Froxlor/form/otpquestion.html.twig new file mode 100644 index 00000000..06e5bc9a --- /dev/null +++ b/templates/Froxlor/form/otpquestion.html.twig @@ -0,0 +1,31 @@ +{% extends "Froxlor/userarea.html.twig" %} + +{% block content %} + +
+ + + +
+ +{% endblock %}