Skip to content

Commit

Permalink
Fix SAML-Toolkits#386. Support rejecting unsolicited SAMLResponses. R…
Browse files Browse the repository at this point in the history
…eject SAMLResponse if requestID was provided to the validotr but the InResponseTo attributeof the SAMLResponse is missing
  • Loading branch information
pitbulk committed Nov 10, 2019
1 parent 4ff22ef commit 0c557b4
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 4 deletions.
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ and supported by OneLogin Inc.
Warning
-------

Version 2.18.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.

Version 2.17.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.

Version 2.17.0 sets strict mode active by default
Expand Down Expand Up @@ -511,6 +513,10 @@ $advancedSettings = array (
// attribute will not be rejected for this fact.
'relaxDestinationValidation' => false,

// If true, SAMLResponses with an InResponseTo value will be rejectd if not
// AuthNRequest ID provided to the validation method.
'rejectUnsolicitedResponsesWithInResponseTo' => false,

// Algorithm that the toolkit will use on signing process. Options:
// 'http://www.w3.org/2000/09/xmldsig#rsa-sha1'
// 'http://www.w3.org/2000/09/xmldsig#dsa-sha1'
Expand Down
4 changes: 4 additions & 0 deletions advanced_settings_example.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@
// attribute will not be rejected for this fact.
'relaxDestinationValidation' => false,

// If true, SAMLResponses with an InResponseTo value will be rejectd if not
// AuthNRequest ID provided to the validation method.
'rejectUnsolicitedResponsesWithInResponseTo' => false,

// Algorithm that the toolkit will use on signing process. Options:
// 'http://www.w3.org/2000/09/xmldsig#rsa-sha1'
// 'http://www.w3.org/2000/09/xmldsig#dsa-sha1'
Expand Down
21 changes: 18 additions & 3 deletions lib/Saml2/Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -166,18 +166,33 @@ public function isValid($requestId = null)

$currentURL = OneLogin_Saml2_Utils::getSelfRoutedURLNoQuery();

$responseInResponseTo = null;
if ($this->document->documentElement->hasAttribute('InResponseTo')) {
$responseInResponseTo = $this->document->documentElement->getAttribute('InResponseTo');
}

// Check if the InResponseTo of the Response matchs the ID of the AuthNRequest (requestId) if provided
if (isset($requestId) && isset($responseInResponseTo) && $requestId != $responseInResponseTo) {
if (!isset($requestId) && isset($responseInResponseTo) && $security['rejectUnsolicitedResponsesWithInResponseTo']) {
throw new OneLogin_Saml2_ValidationError(
"The InResponseTo of the Response: $responseInResponseTo, does not match the ID of the AuthNRequest sent by the SP: $requestId",
"The Response has an InResponseTo attribute: " . $responseInResponseTo . " while no InResponseTo was expected",
OneLogin_Saml2_ValidationError::WRONG_INRESPONSETO
);
}

// Check if the InResponseTo of the Response matchs the ID of the AuthNRequest (requestId) if provided
if (isset($requestId) && $requestId != $responseInResponseTo) {
if ($responseInResponseTo == null) {
throw new OneLogin_Saml2_ValidationError(
"No InResponseTo at the Response, but it was provided the requestId related to the AuthNRequest sent by the SP: $requestId",
OneLogin_Saml2_ValidationError::WRONG_INRESPONSETO
);
} else {
throw new OneLogin_Saml2_ValidationError(
"The InResponseTo of the Response: $responseInResponseTo, does not match the ID of the AuthNRequest sent by the SP: $requestId",
OneLogin_Saml2_ValidationError::WRONG_INRESPONSETO
);
}
}

if (!$this->encrypted && $security['wantAssertionsEncrypted']) {
throw new OneLogin_Saml2_ValidationError(
"The assertion of the Response is not encrypted and the SP requires it",
Expand Down
5 changes: 5 additions & 0 deletions lib/Saml2/Settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,11 @@ private function _addDefaultValues()
$this->_security['relaxDestinationValidation'] = false;
}

// InResponseTo
if (!isset($this->_security['rejectUnsolicitedResponsesWithInResponseTo'])) {
$this->_security['rejectUnsolicitedResponsesWithInResponseTo'] = false;
}

// encrypt expected
if (!isset($this->_security['wantAssertionsEncrypted'])) {
$this->_security['wantAssertionsEncrypted'] = false;
Expand Down
2 changes: 1 addition & 1 deletion lib/Saml2/version.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"php-saml": {
"version": "2.17.1",
"version": "2.18.0",
"released": "06/11/2019"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
PD94bWwgdmVyc2lvbj0iMS4wIj8+DQo8c2FtbHA6UmVzcG9uc2UgeG1sbnM6c2FtbHA9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpwcm90b2NvbCIgeG1sbnM6c2FtbD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmFzc2VydGlvbiIgSUQ9InBmeGMzMmFlZDY3LTgyMGYtNDI5Ni0wYzIwLTIwNWExMGRkNTc4NyIgVmVyc2lvbj0iMi4wIiBJc3N1ZUluc3RhbnQ9IjIwMTEtMDYtMTdUMTQ6NTQ6MTRaIiBEZXN0aW5hdGlvbj0iaHR0cDovL3N0dWZmLmNvbS9lbmRwb2ludHMvZW5kcG9pbnRzL2Fjcy5waHAiPg0KICA8c2FtbDpJc3N1ZXI+aHR0cDovL2lkcC5leGFtcGxlLmNvbS88L3NhbWw6SXNzdWVyPg0KICA8c2FtbHA6U3RhdHVzPg0KICAgIDxzYW1scDpTdGF0dXNDb2RlIFZhbHVlPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6c3RhdHVzOlN1Y2Nlc3MiLz4NCiAgPC9zYW1scDpTdGF0dXM+DQogIDxzYW1sOkFzc2VydGlvbiB4bWxuczp4c2k9Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvWE1MU2NoZW1hLWluc3RhbmNlIiB4bWxuczp4cz0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEiIElEPSJwZng3ODQxOTkxYy1jNzNmLTQwMzUtZTJlZS1jMTcwYzBlMWQzZTQiIFZlcnNpb249IjIuMCIgSXNzdWVJbnN0YW50PSIyMDExLTA2LTE3VDE0OjU0OjE0WiI+DQogICAgPHNhbWw6SXNzdWVyPmh0dHA6Ly9pZHAuZXhhbXBsZS5jb20vPC9zYW1sOklzc3Vlcj4gICAgDQogICAgPHNhbWw6U3ViamVjdD4NCiAgICAgIDxzYW1sOk5hbWVJRCBTUE5hbWVRdWFsaWZpZXI9ImhlbGxvLmNvbSIgRm9ybWF0PSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoxLjE6bmFtZWlkLWZvcm1hdDplbWFpbEFkZHJlc3MiPnNvbWVvbmVAZXhhbXBsZS5jb208L3NhbWw6TmFtZUlEPg0KICAgICAgPHNhbWw6U3ViamVjdENvbmZpcm1hdGlvbiBNZXRob2Q9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpjbTpiZWFyZXIiPg0KICAgICAgICA8c2FtbDpTdWJqZWN0Q29uZmlybWF0aW9uRGF0YSBOb3RPbk9yQWZ0ZXI9IjIwMjAtMDYtMTdUMTQ6NTk6MTRaIiBSZWNpcGllbnQ9Imh0dHA6Ly9zdHVmZi5jb20vZW5kcG9pbnRzL2VuZHBvaW50cy9hY3MucGhwIiBJblJlc3BvbnNlVG89ImludmFsaWRfaW5yZXNwb25zZSIvPg0KICAgICAgPC9zYW1sOlN1YmplY3RDb25maXJtYXRpb24+DQogICAgPC9zYW1sOlN1YmplY3Q+DQogICAgPHNhbWw6Q29uZGl0aW9ucyBOb3RCZWZvcmU9IjIwMTAtMDYtMTdUMTQ6NTM6NDRaIiBOb3RPbk9yQWZ0ZXI9IjIwMjEtMDYtMTdUMTQ6NTk6MTRaIj4NCiAgICAgIDxzYW1sOkF1ZGllbmNlUmVzdHJpY3Rpb24+DQogICAgICAgIDxzYW1sOkF1ZGllbmNlPmh0dHA6Ly9zdHVmZi5jb20vZW5kcG9pbnRzL21ldGFkYXRhLnBocDwvc2FtbDpBdWRpZW5jZT4NCiAgICAgIDwvc2FtbDpBdWRpZW5jZVJlc3RyaWN0aW9uPg0KICAgIDwvc2FtbDpDb25kaXRpb25zPg0KICAgIDxzYW1sOkF1dGhuU3RhdGVtZW50IEF1dGhuSW5zdGFudD0iMjAxMS0wNi0xN1QxNDo1NDowN1oiIFNlc3Npb25Ob3RPbk9yQWZ0ZXI9IjIwMjEtMDYtMTdUMjI6NTQ6MTRaIiBTZXNzaW9uSW5kZXg9Il81MWJlMzc5NjVmZWI1NTc5ZDgwMzE0MTA3NjkzNmRjMmU5ZDFkOThlYmYiPg0KICAgICAgPHNhbWw6QXV0aG5Db250ZXh0Pg0KICAgICAgICA8c2FtbDpBdXRobkNvbnRleHRDbGFzc1JlZj51cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YWM6Y2xhc3NlczpQYXNzd29yZDwvc2FtbDpBdXRobkNvbnRleHRDbGFzc1JlZj4NCiAgICAgIDwvc2FtbDpBdXRobkNvbnRleHQ+DQogICAgPC9zYW1sOkF1dGhuU3RhdGVtZW50Pg0KICAgIDxzYW1sOkF0dHJpYnV0ZVN0YXRlbWVudD4NCiAgICAgIDxzYW1sOkF0dHJpYnV0ZSBOYW1lPSJtYWlsIiBOYW1lRm9ybWF0PSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXR0cm5hbWUtZm9ybWF0OmJhc2ljIj4NCiAgICAgICAgPHNhbWw6QXR0cmlidXRlVmFsdWUgeHNpOnR5cGU9InhzOnN0cmluZyI+c29tZW9uZUBleGFtcGxlLmNvbTwvc2FtbDpBdHRyaWJ1dGVWYWx1ZT4NCiAgICAgIDwvc2FtbDpBdHRyaWJ1dGU+DQogICAgPC9zYW1sOkF0dHJpYnV0ZVN0YXRlbWVudD4NCiAgPC9zYW1sOkFzc2VydGlvbj4NCjwvc2FtbHA6UmVzcG9uc2U+
63 changes: 63 additions & 0 deletions tests/src/OneLogin/Saml2/ResponseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1280,6 +1280,69 @@ public function testIsInValidRequestId()
$this->assertContains('No Signature found. SAML Response rejected', $response2->getError());
}

/**
* Tests the isValid method of the OneLogin_Saml2_Response class
* Case Invalid Response, Unexpected InResponseTo
*
* @covers OneLogin_Saml2_Response::isValid
*/
public function testIsInValidUnexpectedInResponseTo()
{
$xml = file_get_contents(TEST_ROOT . '/data/responses/unsigned_response.xml.base64');

$plainMessage = base64_decode($xml);
$currentURL = OneLogin_Saml2_Utils::getSelfURLNoQuery();
$plainMessage = str_replace('http://stuff.com/endpoints/endpoints/acs.php', $currentURL, $plainMessage);
$message = base64_encode($plainMessage);

$settingsDir = TEST_ROOT .'/settings/';
include $settingsDir.'settings1.php';
$settingsInfo['security']['rejectUnsolicitedResponsesWithInResponseTo'] = true;
$settingsInfo['strict'] = true;
$settings = new OneLogin_Saml2_Settings($settingsInfo);

$response = new OneLogin_Saml2_Response($settings, $message);

$inResponseTo = "_57bcbf70-7b1f-012e-c821-782bcb13bb38";

$response->isValid();
$this->assertEquals('The Response has an InResponseTo attribute: '.$inResponseTo.' while no InResponseTo was expected', $response->getError());

$response->isValid($inResponseTo);
$this->assertContains('No Signature found. SAML Response rejected', $response->getError());
}

/**
* Tests the isValid method of the OneLogin_Saml2_Response class
* Case Invalid Response, No InResponseTo
*
* @covers OneLogin_Saml2_Response::isValid
*/
public function testIsInValidNoInResponseTo()
{
$xml = file_get_contents(TEST_ROOT . '/data/responses/invalids/invalid_unpaired_inresponsesto.xml.base64');

$plainMessage = base64_decode($xml);
$currentURL = OneLogin_Saml2_Utils::getSelfURLNoQuery();
$plainMessage = str_replace('http://stuff.com/endpoints/endpoints/acs.php', $currentURL, $plainMessage);
$message = base64_encode($plainMessage);

$settingsDir = TEST_ROOT .'/settings/';
include $settingsDir.'settings1.php';
$settingsInfo['security']['rejectUnsolicitedResponsesWithInResponseTo'] = true;
$settingsInfo['strict'] = true;
$settings = new OneLogin_Saml2_Settings($settingsInfo);

$response = new OneLogin_Saml2_Response($settings, $message);

$inResponseTo = "_57bcbf70-7b1f-012e-c821-782bcb13bb38";

$response->isValid();
$this->assertContains('No Signature found. SAML Response rejected', $response->getError());

$response->isValid($inResponseTo);
$this->assertEquals('No InResponseTo at the Response, but it was provided the requestId related to the AuthNRequest sent by the SP: '.$inResponseTo, $response->getError());
}

/**
* Tests the isValid method of the OneLogin_Saml2_Response class
Expand Down

0 comments on commit 0c557b4

Please sign in to comment.