Skip to content

Commit

Permalink
Add IssueInstant check to responses
Browse files Browse the repository at this point in the history
  • Loading branch information
pdavide committed Oct 5, 2019
1 parent 5a4d237 commit 7912450
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 2 deletions.
1 change: 1 addition & 0 deletions lib/Saml2/Error.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ class OneLogin_Saml2_ValidationError extends Exception
const NOT_SUPPORTED = 46;
const KEY_ALGORITHM_ERROR = 47;
const MISSING_ENCRYPTED_ELEMENT = 48;
const INVALID_ISSUEINSTANT = 49;


/**
Expand Down
15 changes: 14 additions & 1 deletion lib/Saml2/LogoutResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public function getStatus()
*
* @return bool Returns if the SAML LogoutResponse is or not valid
*/
public function isValid($requestId = null, $retrieveParametersFromServer = false)
public function isValid($requestId = null, $retrieveParametersFromServer = false, $requestIssueInstant = null)
{
$this->_error = null;
try {
Expand Down Expand Up @@ -147,6 +147,19 @@ public function isValid($requestId = null, $retrieveParametersFromServer = false
}
}

// Check if the IssueInstant of the Logout Response is later than the one of the Logout Request if provided (considering a configured clock skew)
if (isset($requestIssueInstant)) {
$issueInstant = $this->document->documentElement->getAttribute('IssueInstant');
$issueInstantTime = OneLogin_Saml2_Utils::parseSAML2Time($issueInstant);
$requestIssueInstantTime = OneLogin_Saml2_Utils::parseSAML2Time($requestIssueInstant);
if ($requestIssueInstantTime > $issueInstantTime + $security['clockSkewTolerance']) {
throw new OneLogin_Saml2_ValidationError(
"The IssueInstant of the Logout Response: $issueInstant is not equal or later than the IssueInstant of the Logout Request: $requestIssueInstant (considering the configured clock skew)",
OneLogin_Saml2_ValidationError::INVALID_ISSUEINSTANT
);
}
}

// Check issuer
$issuer = $this->getIssuer();
if (!empty($issuer) && $issuer != $idPEntityId) {
Expand Down
15 changes: 14 additions & 1 deletion lib/Saml2/Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public function __construct(OneLogin_Saml2_Settings $settings, $response)
*
* @return bool Validate the document
*/
public function isValid($requestId = null)
public function isValid($requestId = null, $requestIssueInstant = null)
{
$this->_error = null;
try {
Expand Down Expand Up @@ -178,6 +178,19 @@ public function isValid($requestId = null)
);
}

// Check if the IssueInstant of the Response is later than the one of the AuthNRequest if provided (considering a configured clock skew)
if (isset($requestIssueInstant)) {
$issueInstant = $this->document->documentElement->getAttribute('IssueInstant');
$issueInstantTime = OneLogin_Saml2_Utils::parseSAML2Time($issueInstant);
$requestIssueInstantTime = OneLogin_Saml2_Utils::parseSAML2Time($requestIssueInstant);
if ($requestIssueInstantTime > $issueInstantTime + $security['clockSkewTolerance']) {
throw new OneLogin_Saml2_ValidationError(
"The IssueInstant of the Response: $issueInstant is not equal or later than the IssueInstant of the Request: $requestIssueInstant (considering the configured clock skew)",
OneLogin_Saml2_ValidationError::INVALID_ISSUEINSTANT
);
}
}

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
42 changes: 42 additions & 0 deletions tests/src/OneLogin/Saml2/LogoutResponseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,48 @@ public function testIsInValidRequestId()
$this->assertContains('The InResponseTo of the Logout Response:', $response2->getError());
}

/**
* Tests the isValid method of the OneLogin_Saml2_LogoutResponse class
* Case invalid IssueInstant
*
* @covers OneLogin_Saml2_LogoutResponse::isValid
*/
public function testIsInvalidIssueInstant()
{
$_SERVER['HTTP_HOST'] = 'stuff.com';
$_SERVER['HTTPS'] = 'off';
$_SERVER['REQUEST_URI'] = '/endpoints/endpoints/sls.php';

$message = file_get_contents(TEST_ROOT . '/data/logout_responses/logout_response.xml.base64');

// Not strict, always valid
$response = new OneLogin_Saml2_LogoutResponse($this->_settings, $message);
$this->assertTrue($response->isValid(null, false, '2013-12-10T04:39:31Z'));
$this->assertTrue($response->isValid(null, false, '2013-12-10T04:42:31Z'));

// Strict, valid/invalid with 0 seconds (default) of clock skew tolerance
$this->_settings->setStrict(true);
$response2 = new OneLogin_Saml2_LogoutResponse($this->_settings, $message);
$this->assertTrue($response2->isValid(null, false, '2013-12-10T04:39:30Z'));
$this->assertTrue($response2->isValid(null, false, '2013-12-10T04:39:31Z'));
$this->assertFalse($response2->isValid(null, false, '2013-12-10T04:39:32Z'));
$this->assertContains('The IssueInstant of the Logout Response', $response2->getError());

// Strict, valid/invalid with 180 seconds of configured clock skew tolerance
include TEST_ROOT .'/settings/settings1.php';
$settingsInfo['strict'] = true;
$settingsInfo['security']['clockSkewTolerance'] = 180;
$settings = new OneLogin_Saml2_Settings($settingsInfo);
$response3 = new OneLogin_Saml2_LogoutResponse($settings, $message);
$this->assertTrue($response3->isValid(null, false, '2013-12-10T04:42:31Z'));
$this->assertFalse($response3->isValid(null, false, '2013-12-10T04:42:32Z'));
$this->assertContains('The IssueInstant of the Logout Response', $response3->getError());

unset($_SERVER['HTTP_HOST']);
unset($_SERVER['HTTPS']);
unset($_SERVER['REQUEST_URI']);
}

/**
* Tests the isValid method of the OneLogin_Saml2_LogoutResponse
* Case invalid Issuer
Expand Down
41 changes: 41 additions & 0 deletions tests/src/OneLogin/Saml2/ResponseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1280,6 +1280,47 @@ 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 IssueInstant
*
* @covers OneLogin_Saml2_Response::isValid
*/
public function testIsInvalidIssueInstant()
{
$_SERVER['HTTP_HOST'] = 'pitbulk.no-ip.org';
$_SERVER['HTTPS'] = 'https';
$_SERVER['REQUEST_URI'] = '/newonelogin/demo1/index.php?acs';

$xml = file_get_contents(TEST_ROOT . '/data/responses/valid_response.xml.base64');

// Not strict, always valid
$response = new OneLogin_Saml2_Response($this->_settings, $xml);
$this->assertTrue($response->isValid(null, '2014-02-19T01:37:01Z'));
$this->assertTrue($response->isValid(null, '2014-02-19T01:39:01Z'));

// Strict, valid/invalid with 0 seconds (default) of clock skew tolerance
$this->_settings->setStrict(true);
$response2 = new OneLogin_Saml2_Response($this->_settings, $xml);
$this->assertTrue($response2->isValid(null, '2014-02-19T01:37:00Z'));
$this->assertTrue($response2->isValid(null, '2014-02-19T01:37:01Z'));
$this->assertFalse($response2->isValid(null, '2014-02-19T01:37:02Z'));
$this->assertContains('The IssueInstant of the Response', $response2->getError());

// Strict, valid/invalid with 180 seconds of configured clock skew tolerance
include TEST_ROOT .'/settings/settings1.php';
$settingsInfo['strict'] = true;
$settingsInfo['security']['clockSkewTolerance'] = 180;
$settings = new OneLogin_Saml2_Settings($settingsInfo);
$response3 = new OneLogin_Saml2_Response($settings, $xml);
$this->assertTrue($response3->isValid(null, '2014-02-19T01:40:01Z'));
$this->assertFalse($response3->isValid(null, '2014-02-19T01:40:02Z'));
$this->assertContains('The IssueInstant of the Response', $response3->getError());

unset($_SERVER['HTTP_HOST']);
unset($_SERVER['HTTPS']);
unset($_SERVER['REQUEST_URI']);
}

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

0 comments on commit 7912450

Please sign in to comment.