From b26dcd0575f22111a0c0cf9c4df5ca24dddeaea2 Mon Sep 17 00:00:00 2001 From: Rajko Albrecht Date: Thu, 7 Feb 2019 15:48:31 +0100 Subject: [PATCH 1/4] Ticket #5236 First workflow for allow reverse proxy with different site names for ispconfig panel. TODO: Allow only for sites known to the system itself instead of all or none. --- interface/lib/app.inc.php | 69 +++++++++++++++++++++------------- security/security_settings.ini | 4 +- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/interface/lib/app.inc.php b/interface/lib/app.inc.php index 7af764f92b..e546521224 100755 --- a/interface/lib/app.inc.php +++ b/interface/lib/app.inc.php @@ -68,20 +68,31 @@ class app { $this->db = false; } } + $this->uses('functions'); // we need this before all others! + $this->uses('auth,plugin,ini_parser,getconf'); + + } + public function __get($prop) { + if(property_exists($this, $prop)) return $this->{$prop}; + + $this->uses($prop); + if(property_exists($this, $prop)) return $this->{$prop}; + else return null; + } + + public function __destruct() { + session_write_close(); + } + + public function initialize_session() { //* Start the session if($this->_conf['start_session'] == true) { - $this->uses('session'); $sess_timeout = $this->conf('interface', 'session_timeout'); - $cookie_domain = (isset($_SERVER['SERVER_NAME']) ? $_SERVER['SERVER_NAME'] : $_SERVER['HTTP_HOST']); - - // Workaround for Nginx servers - if($cookie_domain == '_') { - $tmp = explode(':',$_SERVER["HTTP_HOST"]); - $cookie_domain = $tmp[0]; - unset($tmp); - } + $cookie_domain = $this->get_cookie_domain(); + $this->log("cookie_domain is ".$cookie_domain,0); + $cookie_domain = ''; $cookie_secure = ($_SERVER["HTTPS"] == 'on')?true:false; if($sess_timeout) { /* check if user wants to stay logged in */ @@ -122,23 +133,8 @@ class app { if(empty($_SESSION['s']['language'])) $_SESSION['s']['language'] = $conf['language']; } - $this->uses('functions'); // we need this before all others! - $this->uses('auth,plugin,ini_parser,getconf'); - - } - - public function __get($prop) { - if(property_exists($this, $prop)) return $this->{$prop}; - - $this->uses($prop); - if(property_exists($this, $prop)) return $this->{$prop}; - else return null; } - public function __destruct() { - session_write_close(); - } - public function uses($classes) { $cl = explode(',', $classes); if(is_array($cl)) { @@ -192,7 +188,7 @@ class app { $tstamp = time(); $msg = '[INTERFACE]: '.$msg; $this->db->query("INSERT INTO sys_log (server_id,datalog_id,loglevel,tstamp,message) VALUES (?, 0, ?, ?, ?)", $server_id, $priority,$tstamp,$msg); - /* + if (is_writable($this->_conf['log_file'])) { if (!$fp = fopen ($this->_conf['log_file'], 'a')) { $this->error('Unable to open logfile: ' . $this->_conf['log_file']); @@ -204,7 +200,7 @@ class app { } else { $this->error('Unable to write to logfile: ' . $this->_conf['log_file']); } - */ + } } @@ -336,12 +332,33 @@ class app { $this->tpl->setVar('globalsearch_noresults_limit_txt', $this->lng('globalsearch_noresults_limit_txt')); $this->tpl->setVar('globalsearch_searchfield_watermark_txt', $this->lng('globalsearch_searchfield_watermark_txt')); } + + private function get_cookie_domain() { + $proxy_panel_allowed = $this->getconf->get_security_config('permissions')['reverse_proxy_panel_allowed']; + $cookie_domain = (isset($_SERVER['SERVER_NAME']) ? $_SERVER['SERVER_NAME'] : $_SERVER['HTTP_HOST']); + // Workaround for Nginx servers + if($cookie_domain == '_') { + $tmp = explode(':',$_SERVER["HTTP_HOST"]); + $cookie_domain = $tmp[0]; + unset($tmp); + } + $this->log("Server: ".print_r($_SERVER,true)); + if ($proxy_panel_allowed == 'all') { + return ''; + } + return $cookie_domain; + } } // end class //** Initialize application (app) object //* possible future = new app($conf); $app = new app(); +/* split session creation out of constructor is IMHO better. + otherwise we have some circular references to global $app like in + getconfig property of App - RA +*/ +$app->initialize_session(); // load and enable PHP Intrusion Detection System (PHPIDS) $ids_security_config = $app->getconf->get_security_config('ids'); diff --git a/security/security_settings.ini b/security/security_settings.ini index 24f4e38d20..c135652e17 100644 --- a/security/security_settings.ini +++ b/security/security_settings.ini @@ -17,6 +17,7 @@ admin_allow_software_repo=superadmin remote_api_allowed=yes password_reset_allowed=yes session_regenerate_id=yes +reverse_proxy_panel_allowed=none [ids] ids_anon_enabled=yes @@ -42,4 +43,5 @@ security_admin_email_subject=Security alert from server warn_new_admin=yes warn_passwd_change=no warn_shadow_change=no -warn_group_change=no \ No newline at end of file +warn_group_change=no + -- GitLab From 0344bc5218a6057c15c93530de5a645f1756c147 Mon Sep 17 00:00:00 2001 From: Rajko Albrecht Date: Fri, 8 Feb 2019 12:47:12 +0100 Subject: [PATCH 2/4] Ticket #5236 Start checks against local sites list for allowed reverse proxy forwards --- interface/lib/app.inc.php | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/interface/lib/app.inc.php b/interface/lib/app.inc.php index e546521224..dddbb20613 100755 --- a/interface/lib/app.inc.php +++ b/interface/lib/app.inc.php @@ -335,6 +335,9 @@ class app { private function get_cookie_domain() { $proxy_panel_allowed = $this->getconf->get_security_config('permissions')['reverse_proxy_panel_allowed']; + if ($proxy_panel_allowed == 'all') { + return ''; + } $cookie_domain = (isset($_SERVER['SERVER_NAME']) ? $_SERVER['SERVER_NAME'] : $_SERVER['HTTP_HOST']); // Workaround for Nginx servers if($cookie_domain == '_') { @@ -342,10 +345,18 @@ class app { $cookie_domain = $tmp[0]; unset($tmp); } - $this->log("Server: ".print_r($_SERVER,true)); - if ($proxy_panel_allowed == 'all') { - return ''; + if($proxy_panel_allowed == 'sites') { + $forwarded_host = (isset($_SERVER['HTTP_X_FORWARDED_HOST']) ? $_SERVER['HTTP_X_FORWARDED_HOST'] : null ); + if($forwarded_host !== null && $forwarded_host !== $cookie_domain) { + $sql = "SELECT domain_id from web_domain where domain = '$forwarded_host'"; + $recs = $this->db->queryOneRecord($sql); + if($recs !== null) { + $cookie_domain = $forwarded_host; + } + unset($forwarded_host); + } } + return $cookie_domain; } -- GitLab From e42fd8321dd041cf7a692dc955e6fde3a569e03a Mon Sep 17 00:00:00 2001 From: Rajko Albrecht Date: Mon, 11 Feb 2019 12:44:00 +0100 Subject: [PATCH 3/4] Comments --- interface/lib/app.inc.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/interface/lib/app.inc.php b/interface/lib/app.inc.php index dddbb20613..79be4c9abb 100755 --- a/interface/lib/app.inc.php +++ b/interface/lib/app.inc.php @@ -338,6 +338,11 @@ class app { if ($proxy_panel_allowed == 'all') { return ''; } + /* + * See ticket #5238: It should be ensured, that _SERVER_NAME is always set. + * Otherwise the security improvement doesn't work with nginx. If this is done, + * the check for HTTP_HOST and workaround for nginx is obsolete. + */ $cookie_domain = (isset($_SERVER['SERVER_NAME']) ? $_SERVER['SERVER_NAME'] : $_SERVER['HTTP_HOST']); // Workaround for Nginx servers if($cookie_domain == '_') { @@ -348,6 +353,7 @@ class app { if($proxy_panel_allowed == 'sites') { $forwarded_host = (isset($_SERVER['HTTP_X_FORWARDED_HOST']) ? $_SERVER['HTTP_X_FORWARDED_HOST'] : null ); if($forwarded_host !== null && $forwarded_host !== $cookie_domain) { + // Just check for complete domain name and not auto subdomains $sql = "SELECT domain_id from web_domain where domain = '$forwarded_host'"; $recs = $this->db->queryOneRecord($sql); if($recs !== null) { @@ -365,7 +371,8 @@ class app { //** Initialize application (app) object //* possible future = new app($conf); $app = new app(); -/* split session creation out of constructor is IMHO better. +/* + split session creation out of constructor is IMHO better. otherwise we have some circular references to global $app like in getconfig property of App - RA */ -- GitLab From bb4bf2800c846e44b7c342322ebe9739e2bd69ec Mon Sep 17 00:00:00 2001 From: Rajko Albrecht Date: Mon, 11 Feb 2019 13:24:12 +0100 Subject: [PATCH 4/4] Re-disable log to logfile as before. --- interface/lib/app.inc.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/lib/app.inc.php b/interface/lib/app.inc.php index 79be4c9abb..f8b59317d0 100755 --- a/interface/lib/app.inc.php +++ b/interface/lib/app.inc.php @@ -188,7 +188,7 @@ class app { $tstamp = time(); $msg = '[INTERFACE]: '.$msg; $this->db->query("INSERT INTO sys_log (server_id,datalog_id,loglevel,tstamp,message) VALUES (?, 0, ?, ?, ?)", $server_id, $priority,$tstamp,$msg); - + /* if (is_writable($this->_conf['log_file'])) { if (!$fp = fopen ($this->_conf['log_file'], 'a')) { $this->error('Unable to open logfile: ' . $this->_conf['log_file']); @@ -200,7 +200,7 @@ class app { } else { $this->error('Unable to write to logfile: ' . $this->_conf['log_file']); } - + */ } } -- GitLab