Skip to content

Commit 5996366

Browse files
pitbulkpitbulk
committed
Support rejecting unsolicited SAMLResponses. Reject SAMLResponse if requestID was provided to the validotr but the InResponseTo attributeof the SAMLResponse is missing
1 parent d831b49 commit 5996366

6 files changed

Lines changed: 89 additions & 3 deletions

File tree

‎README.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ 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.
14+
1315
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.
1416

1517
Version 3.3.0 sets strict mode active by default
@@ -480,6 +482,10 @@ $advancedSettings = array(
480482
// attribute will not be rejected for this fact.
481483
'relaxDestinationValidation' => false,
482484

485+
// If true, SAMLResponses with an InResponseTo value will be rejectd if not
486+
// AuthNRequest ID provided to the validation method.
487+
'rejectUnsolicitedResponsesWithInResponseTo' => false,
488+
483489
// Algorithm that the toolkit will use on signing process. Options:
484490
// 'http://www.w3.org/2000/09/xmldsig#rsa-sha1'
485491
// 'http://www.w3.org/2000/09/xmldsig#dsa-sha1'

‎advanced_settings_example.php‎

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

88+
// If true, SAMLResponses with an InResponseTo value will be rejectd if not
89+
// AuthNRequest ID provided to the validation method.
90+
'rejectUnsolicitedResponsesWithInResponseTo' => false,
91+
8892
// Algorithm that the toolkit will use on signing process. Options:
8993
// 'http://www.w3.org/2000/09/xmldsig#rsa-sha1'
9094
// 'http://www.w3.org/2000/09/xmldsig#dsa-sha1'

‎src/Saml2/Response.php‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,18 +195,33 @@ public function isValid($requestId = null)
195195

196196
$currentURL = Utils::getSelfRoutedURLNoQuery();
197197

198+
$responseInResponseTo = null;
198199
if ($this->document->documentElement->hasAttribute('InResponseTo')) {
199200
$responseInResponseTo = $this->document->documentElement->getAttribute('InResponseTo');
200201
}
201202

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

210+
// Check if the InResponseTo of the Response matchs the ID of the AuthNRequest (requestId) if provided
211+
if (isset($requestId) && $requestId != $responseInResponseTo) {
212+
if ($responseInResponseTo == null) {
213+
throw new ValidationError(
214+
"No InResponseTo at the Response, but it was provided the requestId related to the AuthNRequest sent by the SP: $requestId",
215+
ValidationError::WRONG_INRESPONSETO
216+
);
217+
} else {
218+
throw new ValidationError(
219+
"The InResponseTo of the Response: $responseInResponseTo, does not match the ID of the AuthNRequest sent by the SP: $requestId",
220+
ValidationError::WRONG_INRESPONSETO
221+
);
222+
}
223+
}
224+
210225
if (!$this->encrypted && $security['wantAssertionsEncrypted']) {
211226
throw new ValidationError(
212227
"The assertion of the Response is not encrypted and the SP requires it",

‎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+
// InResponseTo
382+
if (!isset($this->_security['rejectUnsolicitedResponsesWithInResponseTo'])) {
383+
$this->_security['rejectUnsolicitedResponsesWithInResponseTo'] = false;
384+
}
385+
381386
// encrypt expected
382387
if (!isset($this->_security['wantAssertionsEncrypted'])) {
383388
$this->_security['wantAssertionsEncrypted'] = false;

‎tests/data/responses/invalids/invalid_unpaired_inresponsesto.xml.base64‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
PD94bWwgdmVyc2lvbj0iMS4wIj8+DQo8c2FtbHA6UmVzcG9uc2UgeG1sbnM6c2FtbHA9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpwcm90b2NvbCIgeG1sbnM6c2FtbD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOmFzc2VydGlvbiIgSUQ9InBmeGMzMmFlZDY3LTgyMGYtNDI5Ni0wYzIwLTIwNWExMGRkNTc4NyIgVmVyc2lvbj0iMi4wIiBJc3N1ZUluc3RhbnQ9IjIwMTEtMDYtMTdUMTQ6NTQ6MTRaIiBEZXN0aW5hdGlvbj0iaHR0cDovL3N0dWZmLmNvbS9lbmRwb2ludHMvZW5kcG9pbnRzL2Fjcy5waHAiPg0KICA8c2FtbDpJc3N1ZXI+aHR0cDovL2lkcC5leGFtcGxlLmNvbS88L3NhbWw6SXNzdWVyPg0KICA8c2FtbHA6U3RhdHVzPg0KICAgIDxzYW1scDpTdGF0dXNDb2RlIFZhbHVlPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6c3RhdHVzOlN1Y2Nlc3MiLz4NCiAgPC9zYW1scDpTdGF0dXM+DQogIDxzYW1sOkFzc2VydGlvbiB4bWxuczp4c2k9Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvWE1MU2NoZW1hLWluc3RhbmNlIiB4bWxuczp4cz0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEiIElEPSJwZng3ODQxOTkxYy1jNzNmLTQwMzUtZTJlZS1jMTcwYzBlMWQzZTQiIFZlcnNpb249IjIuMCIgSXNzdWVJbnN0YW50PSIyMDExLTA2LTE3VDE0OjU0OjE0WiI+DQogICAgPHNhbWw6SXNzdWVyPmh0dHA6Ly9pZHAuZXhhbXBsZS5jb20vPC9zYW1sOklzc3Vlcj4gICAgDQogICAgPHNhbWw6U3ViamVjdD4NCiAgICAgIDxzYW1sOk5hbWVJRCBTUE5hbWVRdWFsaWZpZXI9ImhlbGxvLmNvbSIgRm9ybWF0PSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoxLjE6bmFtZWlkLWZvcm1hdDplbWFpbEFkZHJlc3MiPnNvbWVvbmVAZXhhbXBsZS5jb208L3NhbWw6TmFtZUlEPg0KICAgICAgPHNhbWw6U3ViamVjdENvbmZpcm1hdGlvbiBNZXRob2Q9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpjbTpiZWFyZXIiPg0KICAgICAgICA8c2FtbDpTdWJqZWN0Q29uZmlybWF0aW9uRGF0YSBOb3RPbk9yQWZ0ZXI9IjIwMjAtMDYtMTdUMTQ6NTk6MTRaIiBSZWNpcGllbnQ9Imh0dHA6Ly9zdHVmZi5jb20vZW5kcG9pbnRzL2VuZHBvaW50cy9hY3MucGhwIiBJblJlc3BvbnNlVG89ImludmFsaWRfaW5yZXNwb25zZSIvPg0KICAgICAgPC9zYW1sOlN1YmplY3RDb25maXJtYXRpb24+DQogICAgPC9zYW1sOlN1YmplY3Q+DQogICAgPHNhbWw6Q29uZGl0aW9ucyBOb3RCZWZvcmU9IjIwMTAtMDYtMTdUMTQ6NTM6NDRaIiBOb3RPbk9yQWZ0ZXI9IjIwMjEtMDYtMTdUMTQ6NTk6MTRaIj4NCiAgICAgIDxzYW1sOkF1ZGllbmNlUmVzdHJpY3Rpb24+DQogICAgICAgIDxzYW1sOkF1ZGllbmNlPmh0dHA6Ly9zdHVmZi5jb20vZW5kcG9pbnRzL21ldGFkYXRhLnBocDwvc2FtbDpBdWRpZW5jZT4NCiAgICAgIDwvc2FtbDpBdWRpZW5jZVJlc3RyaWN0aW9uPg0KICAgIDwvc2FtbDpDb25kaXRpb25zPg0KICAgIDxzYW1sOkF1dGhuU3RhdGVtZW50IEF1dGhuSW5zdGFudD0iMjAxMS0wNi0xN1QxNDo1NDowN1oiIFNlc3Npb25Ob3RPbk9yQWZ0ZXI9IjIwMjEtMDYtMTdUMjI6NTQ6MTRaIiBTZXNzaW9uSW5kZXg9Il81MWJlMzc5NjVmZWI1NTc5ZDgwMzE0MTA3NjkzNmRjMmU5ZDFkOThlYmYiPg0KICAgICAgPHNhbWw6QXV0aG5Db250ZXh0Pg0KICAgICAgICA8c2FtbDpBdXRobkNvbnRleHRDbGFzc1JlZj51cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YWM6Y2xhc3NlczpQYXNzd29yZDwvc2FtbDpBdXRobkNvbnRleHRDbGFzc1JlZj4NCiAgICAgIDwvc2FtbDpBdXRobkNvbnRleHQ+DQogICAgPC9zYW1sOkF1dGhuU3RhdGVtZW50Pg0KICAgIDxzYW1sOkF0dHJpYnV0ZVN0YXRlbWVudD4NCiAgICAgIDxzYW1sOkF0dHJpYnV0ZSBOYW1lPSJtYWlsIiBOYW1lRm9ybWF0PSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXR0cm5hbWUtZm9ybWF0OmJhc2ljIj4NCiAgICAgICAgPHNhbWw6QXR0cmlidXRlVmFsdWUgeHNpOnR5cGU9InhzOnN0cmluZyI+c29tZW9uZUBleGFtcGxlLmNvbTwvc2FtbDpBdHRyaWJ1dGVWYWx1ZT4NCiAgICAgIDwvc2FtbDpBdHRyaWJ1dGU+DQogICAgPC9zYW1sOkF0dHJpYnV0ZVN0YXRlbWVudD4NCiAgPC9zYW1sOkFzc2VydGlvbj4NCjwvc2FtbHA6UmVzcG9uc2U+

‎tests/src/OneLogin/Saml2/ResponseTest.php‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1308,6 +1308,61 @@ public function testIsInValidRequestId()
13081308
$this->assertContains('No Signature found. SAML Response rejected', $response2->getError());
13091309
}
13101310

1311+
/**
1312+
* Tests the isValid method of the Response class
1313+
* Case Invalid Response, Unexpected InResponseTo
1314+
*
1315+
* @covers OneLogin\Saml2\Response::isValid
1316+
*/
1317+
public function testIsInValidUnexpectedInResponseTo()
1318+
{
1319+
$xml = file_get_contents(TEST_ROOT . '/data/responses/unsigned_response.xml.base64');
1320+
$plainMessage = base64_decode($xml);
1321+
$currentURL = Utils::getSelfURLNoQuery();
1322+
$plainMessage = str_replace('http://stuff.com/endpoints/endpoints/acs.php', $currentURL, $plainMessage);
1323+
$message = base64_encode($plainMessage);
1324+
$settingsDir = TEST_ROOT .'/settings/';
1325+
include $settingsDir.'settings1.php';
1326+
$settingsInfo['security']['rejectUnsolicitedResponsesWithInResponseTo'] = true;
1327+
$settingsInfo['strict'] = true;
1328+
$settings = new Settings($settingsInfo);
1329+
$response = new Response($settings, $message);
1330+
$inResponseTo = "_57bcbf70-7b1f-012e-c821-782bcb13bb38";
1331+
$response->isValid();
1332+
$this->assertEquals('The Response has an InResponseTo attribute: '.$inResponseTo.' while no InResponseTo was expected', $response->getError());
1333+
$response->isValid($inResponseTo);
1334+
$this->assertContains('No Signature found. SAML Response rejected', $response->getError());
1335+
}
1336+
/**
1337+
* Tests the isValid method of the Response class
1338+
* Case Invalid Response, No InResponseTo
1339+
*
1340+
* @covers OneLogin\Saml2\Response::isValid
1341+
*/
1342+
public function testIsInValidNoInResponseTo()
1343+
{
1344+
$xml = file_get_contents(TEST_ROOT . '/data/responses/invalids/invalid_unpaired_inresponsesto.xml.base64');
1345+
$plainMessage = base64_decode($xml);
1346+
$currentURL = Utils::getSelfURLNoQuery();
1347+
$plainMessage = str_replace('http://stuff.com/endpoints/endpoints/acs.php', $currentURL, $plainMessage);
1348+
$message = base64_encode($plainMessage);
1349+
$settingsDir = TEST_ROOT .'/settings/';
1350+
include $settingsDir.'settings1.php';
1351+
$settingsInfo['security']['rejectUnsolicitedResponsesWithInResponseTo'] = true;
1352+
$settingsInfo['strict'] = true;
1353+
$settings = new Settings($settingsInfo);
1354+
1355+
$response = new Response($settings, $message);
1356+
1357+
$inResponseTo = "_57bcbf70-7b1f-012e-c821-782bcb13bb38";
1358+
1359+
$response->isValid();
1360+
$this->assertContains('No Signature found. SAML Response rejected', $response->getError());
1361+
1362+
$response->isValid($inResponseTo);
1363+
$this->assertEquals('No InResponseTo at the Response, but it was provided the requestId related to the AuthNRequest sent by the SP: '.$inResponseTo, $response->getError());
1364+
}
1365+
13111366
/**
13121367
* Tests the isValid method of the Response class
13131368
* Case Invalid Response, Invalid signing issues

0 commit comments

Comments
 (0)