From 62f1a9d811b480adf8bd2e3fa9f75a4ac5fa55f9 Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Mon, 17 Jan 2022 17:14:02 +0100 Subject: [PATCH] Refactor web application security measures, closes https://github.com/opnsense/core/issues/5481 --- src/etc/inc/auth.inc | 119 ---------------------------------------- src/etc/inc/authgui.inc | 85 +++++++++++++++++++++++++--- src/etc/inc/config.inc | 27 --------- src/etc/inc/system.inc | 2 - src/www/diag_backup.php | 2 - src/www/wizard.php | 3 - 6 files changed, 78 insertions(+), 160 deletions(-) diff --git a/src/etc/inc/auth.inc b/src/etc/inc/auth.inc index bdedec31d..510901efe 100644 --- a/src/etc/inc/auth.inc +++ b/src/etc/inc/auth.inc @@ -34,128 +34,9 @@ require_once("interfaces.inc"); require_once("util.inc"); -// Will be changed to false if security checks fail -$security_passed = true; - -/* If this function doesn't exist, we're being called from Captive Portal or - another internal subsystem which does not include authgui.inc */ -if (function_exists("display_error_form") && !isset($config['system']['webgui']['nodnsrebindcheck'])) { - /* DNS ReBinding attack prevention */ - $found_host = false; - - /* either an IPv6 address with or without an alternate port */ - if (strstr($_SERVER['HTTP_HOST'], "]")) { - $http_host_port = explode("]", $_SERVER['HTTP_HOST']); - /* v6 address has more parts, drop the last part */ - if (count($http_host_port) > 1) { - array_pop($http_host_port); - $http_host = str_replace(array("[", "]"), "", implode(":", $http_host_port)); - } else { - $http_host = str_replace(array("[", "]"), "", implode(":", $http_host_port)); - } - } else { - $http_host = explode(":", $_SERVER['HTTP_HOST']); - $http_host = $http_host[0]; - } - if ( - is_ipaddr($http_host) || $_SERVER['SERVER_ADDR'] == "127.0.0.1" || - strcasecmp($http_host, "localhost") == 0 or $_SERVER['SERVER_ADDR'] == "::1" - ) { - $found_host = true; - } - if ( - strcasecmp($http_host, $config['system']['hostname'] . "." . $config['system']['domain']) == 0 || - strcasecmp($http_host, $config['system']['hostname']) == 0 - ) { - $found_host = true; - } - - if (!empty($config['system']['webgui']['althostnames']) && !$found_host) { - $althosts = explode(" ", $config['system']['webgui']['althostnames']); - foreach ($althosts as $ah) { - if (strcasecmp($ah, $http_host) == 0 or strcasecmp($ah, $_SERVER['SERVER_ADDR']) == 0) { - $found_host = true; - break; - } - } - } - - if ($found_host == false) { - if (!security_checks_disabled()) { - display_error_form(sprintf(gettext("A potential %sDNS Rebind attack%s has been detected.%sTry to access the router by IP address instead of by hostname."), '', '', '
')); - exit; - } - $security_passed = false; - } -} - -// If the HTTP_REFERER is something other than ourselves then disallow. -if (function_exists("display_error_form") && !isset($config['system']['webgui']['nohttpreferercheck'])) { - if (isset($_SERVER['HTTP_REFERER'])) { - if (file_exists('/tmp/setupwizard_lastreferrer')) { - if ($_SERVER['HTTP_REFERER'] == file_get_contents('/tmp/setupwizard_lastreferrer')) { - unlink('/tmp/setupwizard_lastreferrer'); - header("Refresh: 1; url=index.php"); - echo ""; - echo "" . gettext("Redirecting...") . "" . gettext("Redirecting to the dashboard...") . ""; - exit; - } - } - $found_host = false; - $referrer_host = parse_url($_SERVER['HTTP_REFERER'], PHP_URL_HOST); - $referrer_host = str_replace(array("[", "]"), "", $referrer_host); - if ($referrer_host) { - if ( - strcasecmp($referrer_host, $config['system']['hostname'] . "." . $config['system']['domain']) == 0 || - strcasecmp($referrer_host, $config['system']['hostname']) == 0 - ) { - $found_host = true; - } - - - if (!empty($config['system']['webgui']['althostnames']) && !$found_host) { - $althosts = explode(" ", $config['system']['webgui']['althostnames']); - foreach ($althosts as $ah) { - if (strcasecmp($referrer_host, $ah) == 0) { - $found_host = true; - break; - } - } - } - - if (!$found_host) { - $found_host = isAuthLocalIP($referrer_host); - if ($referrer_host == "127.0.0.1" || $referrer_host == "localhost") { - // allow SSH port forwarded connections and links from localhost - $found_host = true; - } - } - } - if ($found_host == false) { - if (!security_checks_disabled()) { - display_error_form(sprintf( - gettext('The HTTP_REFERER "%s" does not match the predefined settings. You can disable this check if needed under System: Settings: Administration.'), - html_safe($_SERVER['HTTP_REFERER']) - )); - exit; - } - $security_passed = false; - } - } else { - $security_passed = false; - } -} - -if (function_exists("display_error_form") && $security_passed) { - /* Security checks passed, so it should be OK to turn them back on */ - restore_security_checks(); -} -unset($security_passed); - $groupindex = index_groups(); $userindex = index_users(); - /** * check if $http_host is a local configured ip address */ diff --git a/src/etc/inc/authgui.inc b/src/etc/inc/authgui.inc index d5fbc3211..bf9f498cf 100644 --- a/src/etc/inc/authgui.inc +++ b/src/etc/inc/authgui.inc @@ -61,7 +61,69 @@ function set_language() bind_textdomain_codeset($textdomain, $lang_encoding); } -function session_auth(&$Login_Error) +/* DNS ReBinding attack prevention, return true when rebind detected*/ +function check_security_dns_rebind() +{ + global $config; + if (!isset($config['system']['webgui']['nodnsrebindcheck'])) { + /* either an IPv6 address with or without an alternate port */ + if (strstr($_SERVER['HTTP_HOST'], "]")) { + $http_host_port = explode("]", $_SERVER['HTTP_HOST']); + /* v6 address has more parts, drop the last part */ + if (count($http_host_port) > 1) { + array_pop($hfttp_host_port); + $http_host = str_replace(["[", "]"], "", implode(":", $http_host_port)); + } else { + $http_host = str_replace(["[", "]"], "", implode(":", $http_host_port)); + } + } else { + $http_host = explode(":", $_SERVER['HTTP_HOST']); + $http_host = $http_host[0]; + } + $this_host = [ + $config['system']['hostname'] . "." . $config['system']['domain'], + $config['system']['hostname'], + "localhost" + ]; + if (!empty($config['system']['webgui']['althostnames'])) { + $this_host = array_merge($this_host, explode(" ", $config['system']['webgui']['althostnames'])); + } + if (is_ipaddr($http_host) || in_array($_SERVER['SERVER_ADDR'], ["127.0.0.1", "::1"])) { + return false; + } elseif (in_array($http_host, $this_host)) { + return false; + } + return true; + } + return false; +} + +/* HTTP referer detection, return true when being forwarded from an unknown referer*/ +function check_security_http_referer_enforement() +{ + global $config; + if (!isset($config['system']['webgui']['nohttpreferercheck']) && isset($_SERVER['HTTP_REFERER'])) { + $referrer_host = str_replace(["[", "]"], "", parse_url($_SERVER['HTTP_REFERER'], PHP_URL_HOST)); + $this_host = [$config['system']['hostname'] . "." . $config['system']['domain'], $config['system']['hostname']]; + if (!empty($config['system']['webgui']['althostnames'])) { + $this_host = array_merge($this_host, explode(" ", $config['system']['webgui']['althostnames'])); + } + if ($referrer_host) { + if (in_array($referrer_host, $this_host)) { + return false; + } elseif (isAuthLocalIP($referrer_host)) { + return false; + } elseif ($referrer_host == "127.0.0.1" || $referrer_host == "localhost") { + // allow SSH port forwarded connections and links from localhost + return false; + } + } + return true; + } + return false; +} + +function session_auth() { global $config; @@ -90,6 +152,19 @@ function session_auth(&$Login_Error) session_write_close(); return false; } + // check additional security measures + if (empty($_SESSION['Username'])) { + if (check_security_dns_rebind()) { + display_error_form(sprintf(gettext("A potential %sDNS Rebind attack%s has been detected.%sTry to access the router by IP address instead of by hostname."), '', '', '
')); + exit; + } elseif (check_security_http_referer_enforement()) { + display_error_form(sprintf( + gettext('The HTTP_REFERER "%s" does not match the predefined settings. You can disable this check if needed under System: Settings: Administration.'), + html_safe($_SERVER['HTTP_REFERER']) + )); + exit; + } + } /* Validate incoming login request */ if (isset($_POST['login']) && !empty($_POST['usernamefld']) && !empty($_POST['passwordfld'])) { @@ -134,7 +209,6 @@ function session_auth(&$Login_Error) exit; } else { auth_log("Web GUI authentication error for '{$_POST['usernamefld']}' from {$_SERVER['REMOTE_ADDR']}"); - $Login_Error = true; } } @@ -189,13 +263,10 @@ function session_auth(&$Login_Error) return true; } -/* XXX spagehtti code :( */ -$Login_Error = false; - /* Authenticate user - exit if failed */ -if (!session_auth($Login_Error)) { +if (!session_auth()) { set_language(); - display_login_form($Login_Error ? gettext('Wrong username or password.') : null); + display_login_form(!empty($_POST['usernamefld']) ? gettext('Wrong username or password.') : null); exit; } diff --git a/src/etc/inc/config.inc b/src/etc/inc/config.inc index 35f833be8..c12bee072 100644 --- a/src/etc/inc/config.inc +++ b/src/etc/inc/config.inc @@ -173,8 +173,6 @@ function config_restore($conffile) $cnf->backup(); $cnf->restoreBackup($conffile); - disable_security_checks(); - $config = parse_config(); write_config(sprintf('Reverted to %s', array_pop(explode('/', $conffile))), false); @@ -182,31 +180,6 @@ function config_restore($conffile) return 0; } -/* - * Disable security checks for DNS rebind and HTTP referrer until next time - * they pass (or reboot), to aid in preventing accidental lockout when - * restoring settings like hostname, domain, IP addresses, and settings - * related to the DNS rebind and HTTP referrer checks. - * Intended for use when restoring a configuration or directly - * modifying config.xml without an unconditional reboot. - */ -function disable_security_checks() -{ - touch('/tmp/disable_security_checks'); -} - -/* Restores security checks. Should be called after all succeed. */ -function restore_security_checks() -{ - @unlink('/tmp/disable_security_checks'); -} - -/* Returns status of security check temporary disable. */ -function security_checks_disabled() -{ - return file_exists('/tmp/disable_security_checks'); -} - function &config_read_array() { global $config; diff --git a/src/etc/inc/system.inc b/src/etc/inc/system.inc index c3daf58f5..e939c94d5 100644 --- a/src/etc/inc/system.inc +++ b/src/etc/inc/system.inc @@ -1150,8 +1150,6 @@ function system_login_configure($verbose = false) function reset_factory_defaults($sync = true) { mwexec('/bin/rm -fr /conf/* /var/log/* /root/.history'); - disable_security_checks(); - mwexec('/usr/local/sbin/opnsense-beep stop'); /* as we go through a special case directly shut down */ diff --git a/src/www/diag_backup.php b/src/www/diag_backup.php index 1898cdf2f..e74a21bec 100644 --- a/src/www/diag_backup.php +++ b/src/www/diag_backup.php @@ -63,8 +63,6 @@ function restore_config_section($section_name, $new_contents) write_config(sprintf('Restored section %s of config file', $section_name)); convert_config(); - disable_security_checks(); - return true; } diff --git a/src/www/wizard.php b/src/www/wizard.php index 774d44185..a6b9b9c8a 100644 --- a/src/www/wizard.php +++ b/src/www/wizard.php @@ -247,9 +247,6 @@ function redirect_url() $urlhost = $config['wizardtemp']['system']['hostname'] . '.' . $config['wizardtemp']['system']['domain']; } } - if ($urlhost != $http_host) { - file_put_contents('/tmp/setupwizard_lastreferrer', $proto . '://' . $http_host . $urlport . $_SERVER['REQUEST_URI']); - } return $proto . '://' . $urlhost . $urlport; }