Commit 70d1b712 authored by haemmer's avatar haemmer
Browse files

Fixed bug that prevented CLI execution of readMetadata.php

Corrected behaviour of return URL checks as discussed in #351
parent 23e7e423
......@@ -342,6 +342,11 @@ For category entries, only Type, (local) Name and Index are relevant.
-------------------------------------------------------------------------------
Version History:
1.14.1 - Corrected behaviour of $enableDSReturnParamCheck and
$useACURLsForReturnParamCheck. There won't be an error anymore if an SP
has no <idpdisc:DiscoveryResponse> extension defined. In such a case
there will only be a check if $useACURLsForReturnParamCheck is enabled.
- Fixed a bug in readMetadata.php that prevented CLI execution
1.14 - Added the configuration option wayf_force_remember_for_session to
the Embedded WAYF on request of Wolgang Lierz from ETH Zurich. This
option allows setting the remember for session checkbox to true
......
......@@ -4,7 +4,7 @@
******************************************************************************
SWITCH PHP WAYF,
Copyright 2009 SWITCH - Serving Swiss Universities
Version: 1.14
Version: 1.14.1
Contact: aai@switch.ch
Web site: http://www.switch.ch/aai/wayf
******************************************************************************
......@@ -90,8 +90,9 @@ if(isValidDSRequest()){
exit;
}
// Check that return URL in DS request is verified
if(!isVerifiedReturnURL($_GET['entityID'], $returnURL)){
// Check return URL in DS request if checks are enabled
$returnURLOK = verifyReturnURL($_GET['entityID'], $returnURL);
if(!$returnURLOK){
// Show error
$message = sprintf(getLocalString('unverified_return_url'), htmlentities($returnURL), htmlentities($_GET['entityID']));
printError($message);
......
......@@ -63,17 +63,21 @@ $includeLocalConfEntries = true;
// Whether the return parameter is checked against SAML2 metadata or not
// The Discovery Service specification says the DS SHOULD check this in order
// to mitigate phising problems
// This check only is active if $useSAML2Metadata = true
// to mitigate phising problems.
// You must have $useSAML2Metadata = true in order to activate this check.
// The return parameter will only be checked if the Service Provider's metadata
// contains an <idpdisc:DiscoveryResponse> or if
// $useACURLsForReturnParamCheck = true
$enableDSReturnParamCheck = true;
// If true, not only the the URLs defined in the metadata extension
// <idpdisc:DiscoveryResponse> are used for the check but also the hostnames
// of the assertion consumer URLs. The hostnames are compared against the
// hostname used in the return parameter
// This feature is especially useful in case metadata doesn't contain the
// <idpdisc:DiscoveryResponse> extension. However, enabling this feature also
// reduces the security of the check.
// If true, the return parameter is checked also for Service Providers that
// don't have and <idpdisc:DiscoveryResponse> extension set. Instead of this
// extension the hostnames of the assertion consumer URLs are used to check
// the return paraemter against.
// This feature is useful in case the Service Provider's metadata doesn't contain
// a <idpdisc:DiscoveryResponse> extension. Enabling this feature increases
// security for Service Provider's that don't have an <idpdisc:DiscoveryResponse>
// extensions.
// This feature only is active if $enableDSReturnParamCheck = true
// and if $useSAML2Metadata = true
$useACURLsForReturnParamCheck = false;
......@@ -158,4 +162,5 @@ $WAYFLogFile = '/var/log/apache2/wayf.log';
// If the development mode is activated, PHP errors and warnings will be displayed
$developmentMode = false;
?>
......@@ -396,36 +396,43 @@ function getIPAdressHint() {
return '-';
}
/******************************************************************************/
// Returns true if URL could be verified, false otherwise
function isVerifiedReturnURL($entityID, $returnURL) {
// Returns true if URL could be verified or if no check is necessary, false otherwise
function verifyReturnURL($entityID, $returnURL) {
global $SProviders, $enableDSReturnParamCheck, $useACURLsForReturnParamCheck;
// Is check necessary
if (!isset($enableDSReturnParamCheck) || !$enableDSReturnParamCheck){
// Skip check if is is deactivated
if (
!isset($enableDSReturnParamCheck)
|| !$enableDSReturnParamCheck
){
return true;
}
// SP unknown, therefore return false
// SP is unknown, therefore return false
if (!isset($SProviders[$entityID])){
return false;
}
// Check using DiscoveryResponse extension
if (isset($SProviders[$entityID]['DSURL']) && in_array($returnURL, $SProviders[$entityID]['DSURL'])){
return true;
// If SP has a <idpdisc:DiscoveryResponse>, check return param
if (isset($SProviders[$entityID]['DSURL'])){
return in_array($returnURL, $SProviders[$entityID]['DSURL']);
}
if ($useACURLsForReturnParamCheck && isset($SProviders[$entityID]['ACURL'])){
// If fall back check is enabled, check return param
if ($useACURLsForReturnParamCheck){
$returnURLHostName = getHostNameFromURI($returnURL);
foreach($SProviders[$entityID]['ACURL'] as $ACURL){
if (getHostNameFromURI($ACURL) == $returnURLHostName){
return true;
}
}
// We haven't found a matchin assertion consumer url so we return false
return false;
}
// Default return value
return false;
// SP has no <idpdisc:DiscoveryResponse> and $useACURLsForReturnParamCheck
// is disabled, so we don't check anything
return true;
}
/******************************************************************************/
......
......@@ -5,13 +5,6 @@
* Configuration parameters are specified in config.php.
*/
// Check configuration
if (!isset($metadataSPFile)){
$errorMsg = 'Please first define a file $metadataSPFile = \'SProvider.metadata.conf.php\'; in config.php before running this script.';
syslog(LOG_ERR, $errorMsg);
die($errorMsg);
}
// Make sure this script is not accessed directly
if(isRunViaCLI()){
// Run in cli mode.
......@@ -27,6 +20,13 @@ if(isRunViaCLI()){
exit ("Exiting: File ".$metadataFile." is empty or does not exist\n");
}
// Check configuration
if (!isset($metadataSPFile)){
$errorMsg = 'Please first define a file $metadataSPFile = \'SProvider.metadata.conf.php\'; in config.php before running this script.';
syslog(LOG_ERR, $errorMsg);
die($errorMsg);
}
echo 'Parsing metadata file '.$metadataFile."\n";
list($metadataIDProviders, $metadataSProviders) = parseMetadata($metadataFile, $defaultLanguage);
......@@ -61,6 +61,14 @@ if(isRunViaCLI()){
} elseif (isRunViaInclude()) {
// Check configuration
if (!isset($metadataSPFile)){
$errorMsg = 'Please first define a file $metadataSPFile = \'SProvider.metadata.conf.php\'; in config.php before running this script.';
syslog(LOG_ERR, $errorMsg);
die($errorMsg);
}
// Run as included file
if(!file_exists($metadataIDPFile) or filemtime($metadataFile) > filemtime($metadataIDPFile)){
// Regenerate $metadataIDPFile.
......@@ -97,7 +105,6 @@ if(isRunViaCLI()){
$SProviders = &$metadataSProviders;
}
} else {
exit('No direct script access allowed');
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment