Improve secure connection detection (#161)

The previous code broke the setup page when the iTop server is behind a proxy that handles SSL termination.
Now the detection also checks the `HTTP_X_FORWARDED_PROTO` and `HTTP_X_FORWARDED_PROTOCOL` HTTP headers. 
For any other page than the setup, the check is unchanged.

Many thanks @Hipska !
This commit is contained in:
Thomas Casteleyn
2020-12-16 15:37:48 +01:00
committed by GitHub
parent 1b115624a2
commit 42354ba794
3 changed files with 31 additions and 14 deletions

View File

@@ -851,11 +851,13 @@ class utils
/**
* Returns the absolute URL to the application root path
*
* @param bool $bTrustProxy
*
* @return string The absolute URL to the application root, without the first slash
*
* @throws \Exception
*/
public static function GetAbsoluteUrlAppRoot()
public static function GetAbsoluteUrlAppRoot($bTrustProxy=false)
{
static $sUrl = null;
if ($sUrl === null)
@@ -863,7 +865,7 @@ class utils
$sUrl = self::GetConfig()->Get('app_root_url');
if ($sUrl == '')
{
$sUrl = self::GetDefaultUrlAppRoot();
$sUrl = self::GetDefaultUrlAppRoot($bTrustProxy);
}
elseif (strpos($sUrl, SERVER_NAME_PLACEHOLDER) > -1)
{
@@ -887,15 +889,17 @@ class utils
* For most usages, when an root url is needed, use utils::GetAbsoluteUrlAppRoot() instead as uses this only as a fallback when the
* app_root_url conf parameter is not defined.
*
* @param bool $bTrustProxy
*
* @return string
*
* @throws \Exception
*/
public static function GetDefaultUrlAppRoot()
public static function GetDefaultUrlAppRoot($bTrustProxy=false)
{
// Build an absolute URL to this page on this server/port
$sServerName = isset($_SERVER['SERVER_NAME']) ? $_SERVER['SERVER_NAME'] : '';
$sProtocol = self::IsConnectionSecure() ? 'https' : 'http';
$sProtocol = self::IsConnectionSecure($bTrustProxy) ? 'https' : 'http';
$iPort = isset($_SERVER['SERVER_PORT']) ? $_SERVER['SERVER_PORT'] : 80;
if ($sProtocol == 'http')
{
@@ -1003,15 +1007,28 @@ class utils
* Though the official specs says 'a non empty string', some servers like IIS do set it to 'off' !
* nginx set it to an empty string
* Others might leave it unset (no array entry)
*
* @param bool $bTrustProxy
*
* @return bool
*/
public static function IsConnectionSecure()
public static function IsConnectionSecure($bTrustProxy=false)
{
$bSecured = false;
if (!empty($_SERVER['HTTPS']) && (strtolower($_SERVER['HTTPS']) != 'off'))
if (!empty($_SERVER['HTTP_X_FORWARDED_PROTO']) && $bTrustProxy)
{
$bSecured = true;
$bSecured = ($_SERVER['HTTP_X_FORWARDED_PROTO'] === 'https');
}
elseif (!empty($_SERVER['HTTP_X_FORWARDED_PROTOCOL']) && $bTrustProxy)
{
$bSecured = ($_SERVER['HTTP_X_FORWARDED_PROTOCOL'] === 'https');
}
elseif (isset($_SERVER['HTTPS']) && !empty($_SERVER['HTTPS']))
{
$bSecured = (strcasecmp($_SERVER['HTTPS'], 'off') !== 0);
}
return $bSecured;
}

View File

@@ -143,7 +143,7 @@ class SetupPage extends NiceWebPage
public function output()
{
$sLogo = utils::GetAbsoluteUrlAppRoot().'/images/itop-logo.png';
$sLogo = utils::GetAbsoluteUrlAppRoot(true).'/images/itop-logo.png';
$this->s_content = "<div id=\"header\"><h1><a href=\"http://www.combodo.com/itop\" target=\"_blank\"><img title=\"iTop by Combodo\" alt=\" \" src=\"{$sLogo}?t=".utils::GetCacheBusterTimestamp()."\"></a>&nbsp;".htmlentities($this->s_title,
ENT_QUOTES, self::PAGES_CHARSET)."</h1>\n</div><div id=\"setup\">{$this->s_content}\n</div>\n";

View File

@@ -969,7 +969,7 @@ class WizStepMiscParams extends WizardStep
public function Display(WebPage $oPage)
{
$sDefaultLanguage = $this->oWizard->GetParameter('default_language', $this->oWizard->GetParameter('admin_language'));
$sApplicationURL = $this->oWizard->GetParameter('application_url', utils::GetDefaultUrlAppRoot());
$sApplicationURL = $this->oWizard->GetParameter('application_url', utils::GetDefaultUrlAppRoot(true));
$sDefaultGraphvizPath = (strtolower(substr(PHP_OS, 0, 3)) === 'win') ? 'C:\\Program Files\\Graphviz\\bin\\dot.exe' : '/usr/bin/dot';
$sGraphvizPath = $this->oWizard->GetParameter('graphviz_path', $sDefaultGraphvizPath);
$sSampleData = $this->oWizard->GetParameter('sample_data', 'yes');
@@ -1126,7 +1126,7 @@ class WizStepUpgradeMiscParams extends WizardStep
public function Display(WebPage $oPage)
{
$sApplicationURL = $this->oWizard->GetParameter('application_url', utils::GetDefaultUrlAppRoot());
$sApplicationURL = $this->oWizard->GetParameter('application_url', utils::GetDefaultUrlAppRoot(true));
$sDefaultGraphvizPath = (strtolower(substr(PHP_OS, 0, 3)) === 'win') ? 'C:\\Program Files\\Graphviz\\bin\\dot.exe' : '/usr/bin/dot';
$sGraphvizPath = $this->oWizard->GetParameter('graphviz_path', $sDefaultGraphvizPath);
$oPage->add('<h2>Additional parameters</h2>');
@@ -1384,14 +1384,14 @@ class WizStepModulesChoice extends WizardStep
if (substr($sBannerPath, 0, 1) == '/')
{
// absolute path, means relative to APPROOT
$sBannerUrl = utils::GetDefaultUrlAppRoot().$sBannerPath;
$sBannerUrl = utils::GetDefaultUrlAppRoot(true).$sBannerPath;
}
else
{
// relative path: i.e. relative to the directory containing the XML file
$sFullPath = dirname($this->GetSourceFilePath()).'/'.$sBannerPath;
$sRealPath = realpath($sFullPath);
$sBannerUrl = utils::GetDefaultUrlAppRoot().str_replace(realpath(APPROOT), '', $sRealPath);
$sBannerUrl = utils::GetDefaultUrlAppRoot(true).str_replace(realpath(APPROOT), '', $sRealPath);
}
$oPage->add('<td><img src="'.$sBannerUrl.'"/><td>');
}
@@ -2502,7 +2502,7 @@ class WizStepDone extends WizardStep
$aManualSteps = array();
$aAvailableModules = SetupUtils::AnalyzeInstallation($this->oWizard);
$sRootUrl = utils::GetAbsoluteUrlAppRoot();
$sRootUrl = utils::GetAbsoluteUrlAppRoot(true);
$aSelectedModules = json_decode($this->oWizard->GetParameter('selected_modules'), true);
foreach($aSelectedModules as $sModuleId)
{
@@ -2534,7 +2534,7 @@ class WizStepDone extends WizardStep
{
// To mitigate security risks: pass only the filename without the extension, the download will add the extension itself
$oPage->p('Your backup is ready');
$oPage->p('<a style="background:transparent;" href="'.utils::GetAbsoluteUrlAppRoot().'setup/ajax.dataloader.php?operation=async_action&step_class=WizStepDone&params[backup]='.urlencode($sBackupDestination).'&authent='.$this->oWizard->GetParameter('authent','').'" target="_blank"><img src="../images/tar.png" style="border:0;vertical-align:middle;">&nbsp;Download '.basename($sBackupDestination).'</a>');
$oPage->p('<a style="background:transparent;" href="'.utils::GetAbsoluteUrlAppRoot(true).'setup/ajax.dataloader.php?operation=async_action&step_class=WizStepDone&params[backup]='.urlencode($sBackupDestination).'&authent='.$this->oWizard->GetParameter('authent','').'" target="_blank"><img src="../images/tar.png" style="border:0;vertical-align:middle;">&nbsp;Download '.basename($sBackupDestination).'</a>');
}
else
{