Skip to content

Commit d9f9085

Browse files
pitbulkpitbulk
committed
Add setting for strict destination matching
1 parent fdc3ed0 commit d9f9085

11 files changed

Lines changed: 100 additions & 28 deletions

‎CHANGELOG‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
CHANGELOG
22
=========
33
v.3.4.0
4-
* Support rejecting unsolicited SAMLResponses.
4+
* Support rejecting unsolicited SAMLResponses.
5+
* Support stric destination matching.
56
* Reject SAMLResponse if requestID was provided to the validotr but the InResponseTo attributeof the SAMLResponse is missing
67
* Check destination against the getSelfURLNoQuery as well on LogoutRequest and LogoutResponse as we do on Response
78
* Improve getSelfRoutedURLNoQuery method
@@ -41,6 +42,16 @@ v.3.0.0
4142
* Remove mcrypt dependency. Compatible with PHP 7.2
4243
* xmlseclibs now is not part of the toolkit and need to be installed from original source
4344

45+
v.2.18.0
46+
* Support rejecting unsolicited SAMLResponses.
47+
* Support stric destination matching.
48+
* Reject SAMLResponse if requestID was provided to the validotr but the InResponseTo attributeof the SAMLResponse is missing
49+
* Check destination against the getSelfURLNoQuery as well on LogoutRequest and LogoutResponse as we do on Response
50+
* Improve getSelfRoutedURLNoQuery method
51+
* Only add responseUrl to the settings if ResponseLocation present in the IdPMetadataParser
52+
* Remove use of $_GET on static method validateBinarySign
53+
* Fix error message when Assertion and NameId are both encrypted (not supported)
54+
4455
v.2.17.1
4556
* Update xmlseclibs to 3.0.4
4657
* Remove Comparison atribute from RequestedAuthnContext when setting has empty value

‎README.md‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ and supported by OneLogin Inc.
1010
Warning
1111
-------
1212

13-
Version 3.4.0 introduces the 'rejectUnsolicitedResponsesWithInResponseTo' setting parameter, by default disabled, that will allow invalidate unsolicited SAMLResponse. This version as well will reject SAMLResponse if requestId was provided to the validator but the SAMLResponse does not contain a InResponseTo attribute.
13+
Version 3.4.0 introduces the 'rejectUnsolicitedResponsesWithInResponseTo' setting parameter, by default disabled, that will allow invalidate unsolicited SAMLResponse. This version as well will reject SAMLResponse if requestId was provided to the validator but the SAMLResponse does not contain a InResponseTo attribute. And an additional setting parameter 'destinationStrictlyMatches', by default disabled, that will force that the Destination URL should strictly match to the address that process the SAMLResponse.
1414

1515
Version 3.3.1 updates xmlseclibs to 3.0.4 (CVE-2019-3465), but php-saml was not directly affected since it implements additional checks that prevent to exploit that vulnerability.
1616

@@ -482,6 +482,12 @@ $advancedSettings = array(
482482
// attribute will not be rejected for this fact.
483483
'relaxDestinationValidation' => false,
484484

485+
// If true, Destination URL should strictly match to the address to
486+
// which the response has been sent.
487+
// Notice that if 'relaxDestinationValidation' is true an empty Destintation
488+
// will be accepted.
489+
'destinationStrictlyMatches' => false,
490+
485491
// If true, SAMLResponses with an InResponseTo value will be rejectd if not
486492
// AuthNRequest ID provided to the validation method.
487493
'rejectUnsolicitedResponsesWithInResponseTo' => false,

‎advanced_settings_example.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,12 @@
8585
// attribute will not be rejected for this fact.
8686
'relaxDestinationValidation' => false,
8787

88+
// If true, Destination URL should strictly match to the address to
89+
// which the response has been sent.
90+
// Notice that if 'relaxDestinationValidation' is true an empty Destintation
91+
// will be accepted.
92+
'destinationStrictlyMatches' => false,
93+
8894
// If true, SAMLResponses with an InResponseTo value will be rejectd if not
8995
// AuthNRequest ID provided to the validation method.
9096
'rejectUnsolicitedResponsesWithInResponseTo' => false,

‎src/Saml2/LogoutRequest.php‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public function __construct(\OneLogin\Saml2\Settings $settings, $request = null,
104104
$nameIdFormat = Constants::NAMEID_ENTITY;
105105
}
106106

107-
/* From saml-core-2.0-os 8.3.6, when the entity Format is used:
107+
/* From saml-core-2.0-os 8.3.6, when the entity Format is used:
108108
"The NameQualifier, SPNameQualifier, and SPProvidedID attributes MUST be omitted.
109109
*/
110110
if (!empty($nameIdFormat) && $nameIdFormat == Constants::NAMEID_ENTITY) {
@@ -278,7 +278,7 @@ public static function getNameIdData($request, $key = null)
278278
* @param string|null $key The SP key
279279
*
280280
* @return string Name ID Value
281-
*
281+
*
282282
* @throws Error
283283
* @throws Exception
284284
* @throws ValidationError
@@ -295,7 +295,7 @@ public static function getNameId($request, $key = null)
295295
* @param string|DOMDocument $request Logout Request Message
296296
*
297297
* @return string|null $issuer The Issuer
298-
*
298+
*
299299
* @throws Exception
300300
*/
301301
public static function getIssuer($request)
@@ -324,7 +324,7 @@ public static function getIssuer($request)
324324
* @param string|DOMDocument $request Logout Request Message
325325
*
326326
* @return array The SessionIndex value
327-
*
327+
*
328328
* @throws Exception
329329
*/
330330
public static function getSessionIndexes($request)
@@ -350,7 +350,7 @@ public static function getSessionIndexes($request)
350350
* @param bool $retrieveParametersFromServer True if we want to use parameters from $_SERVER to validate the signature
351351
*
352352
* @return bool If the Logout Request is or not valid
353-
*
353+
*
354354
* @throws Exception
355355
* @throws ValidationError
356356
*/
@@ -393,15 +393,25 @@ public function isValid($retrieveParametersFromServer = false)
393393
// Check destination
394394
if ($dom->documentElement->hasAttribute('Destination')) {
395395
$destination = $dom->documentElement->getAttribute('Destination');
396-
if (!empty($destination) && strpos($destination, $currentURL) !== 0) {
397-
$currentURLNoRouted = Utils::getSelfURLNoQuery();
398-
399-
if (strpos($destination, $currentURLNoRouted) !== 0) {
396+
if (empty($destination)) {
397+
if (!$security['relaxDestinationValidation']) {
400398
throw new ValidationError(
401-
"The LogoutRequest was received at $currentURL instead of $destination",
402-
ValidationError::WRONG_DESTINATION
399+
"The LogoutRequest has an empty Destination value",
400+
ValidationError::EMPTY_DESTINATION
403401
);
404402
}
403+
} else {
404+
$urlComparisonLength = $security['destinationStrictlyMatches'] ? strlen($destination) : strlen($currentURL);
405+
if (strncmp($destination, $currentURL, $urlComparisonLength) !== 0) {
406+
$currentURLNoRouted = Utils::getSelfURLNoQuery();
407+
$urlComparisonLength = $security['destinationStrictlyMatches'] ? strlen($destination) : strlen($currentURLNoRouted);
408+
if (strncmp($destination, $currentURLNoRouted, $urlComparisonLength) !== 0) {
409+
throw new ValidationError(
410+
"The LogoutRequest was received at $currentURL instead of $destination",
411+
ValidationError::WRONG_DESTINATION
412+
);
413+
}
414+
}
405415
}
406416
}
407417

‎src/Saml2/LogoutResponse.php‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,10 @@ class LogoutResponse
6565
*
6666
* @param Settings $settings Settings.
6767
* @param string|null $response An UUEncoded SAML Logout response from the IdP.
68-
*
68+
*
6969
* @throws Error
7070
* @throws Exception
71-
*
71+
*
7272
*/
7373
public function __construct(\OneLogin\Saml2\Settings $settings, $response = null)
7474
{
@@ -140,7 +140,7 @@ public function getStatus()
140140
* @param bool $retrieveParametersFromServer True if we want to use parameters from $_SERVER to validate the signature
141141
*
142142
* @return bool Returns if the SAML LogoutResponse is or not valid
143-
*
143+
*
144144
* @throws ValidationError
145145
*/
146146
public function isValid($requestId = null, $retrieveParametersFromServer = false)
@@ -185,18 +185,27 @@ public function isValid($requestId = null, $retrieveParametersFromServer = false
185185

186186
$currentURL = Utils::getSelfRoutedURLNoQuery();
187187

188-
// Check destination
189188
if ($this->document->documentElement->hasAttribute('Destination')) {
190189
$destination = $this->document->documentElement->getAttribute('Destination');
191-
if (!empty($destination) && strpos($destination, $currentURL) !== 0) {
192-
$currentURLNoRouted = Utils::getSelfURLNoQuery();
193-
194-
if (strpos($destination, $currentURLNoRouted) !== 0) {
190+
if (empty($destination)) {
191+
if (!$security['relaxDestinationValidation']) {
195192
throw new ValidationError(
196-
"The LogoutResponse was received at $currentURL instead of $destination",
197-
ValidationError::WRONG_DESTINATION
193+
"The LogoutResponse has an empty Destination value",
194+
ValidationError::EMPTY_DESTINATION
198195
);
199196
}
197+
} else {
198+
$urlComparisonLength = $security['destinationStrictlyMatches'] ? strlen($destination) : strlen($currentURL);
199+
if (strncmp($destination, $currentURL, $urlComparisonLength) !== 0) {
200+
$currentURLNoRouted = Utils::getSelfURLNoQuery();
201+
$urlComparisonLength = $security['destinationStrictlyMatches'] ? strlen($destination) : strlen($currentURLNoRouted);
202+
if (strncmp($destination, $currentURLNoRouted, $urlComparisonLength) !== 0) {
203+
throw new ValidationError(
204+
"The LogoutResponse was received at $currentURL instead of $destination",
205+
ValidationError::WRONG_DESTINATION
206+
);
207+
}
208+
}
200209
}
201210
}
202211

‎src/Saml2/Response.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -278,10 +278,11 @@ public function isValid($requestId = null)
278278
);
279279
}
280280
} else {
281-
if (strpos($destination, $currentURL) !== 0) {
281+
$urlComparisonLength = $security['destinationStrictlyMatches'] ? strlen($destination) : strlen($currentURL);
282+
if (strncmp($destination, $currentURL, $urlComparisonLength) !== 0) {
282283
$currentURLNoRouted = Utils::getSelfURLNoQuery();
283-
284-
if (strpos($destination, $currentURLNoRouted) !== 0) {
284+
$urlComparisonLength = $security['destinationStrictlyMatches'] ? strlen($destination) : strlen($currentURLNoRouted);
285+
if (strncmp($destination, $currentURLNoRouted, $urlComparisonLength) !== 0) {
285286
throw new ValidationError(
286287
"The response was received at $currentURL instead of $destination",
287288
ValidationError::WRONG_DESTINATION
@@ -463,7 +464,7 @@ public function getId()
463464

464465
/**
465466
* @return string|null the ID of the assertion in the Response
466-
*
467+
*
467468
* @throws ValidationError
468469
*/
469470
public function getAssertionId()

‎src/Saml2/Settings.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,11 @@ private function _addDefaultValues()
378378
$this->_security['relaxDestinationValidation'] = false;
379379
}
380380

381+
// Strict Destination match validation
382+
if (!isset($this->_security['destinationStrictlyMatches'])) {
383+
$this->_security['destinationStrictlyMatches'] = false;
384+
}
385+
381386
// InResponseTo
382387
if (!isset($this->_security['rejectUnsolicitedResponsesWithInResponseTo'])) {
383388
$this->_security['rejectUnsolicitedResponsesWithInResponseTo'] = false;

0 commit comments

Comments
 (0)