From d4c8acd98efa325aef310c55e00133a939e7409a Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tvdijen@gmail.com>
Date: Mon, 31 Jan 2022 00:34:14 +0100
Subject: [PATCH] Only allow unsolicited responses conditionally
---
config-templates/config.php | 1 +
docs/simplesamlphp-upgrade-notes-2.0.md | 5 ++++-
modules/saml/lib/Auth/Source/SP.php | 10 +++++++++
modules/saml/www/sp/saml2-acs.php | 28 ++++++++++++++++++-------
4 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/config-templates/config.php b/config-templates/config.php
index 16a3910a8..f1f53d1e6 100644
--- a/config-templates/config.php
+++ b/config-templates/config.php
@@ -461,6 +461,7 @@ $config = [
* In example when you are setting up a federation bridge.
*/
'enable.saml20-idp' => false,
+ 'enable.saml20-unsolicited' => false,
'enable.adfs-idp' => false,
diff --git a/docs/simplesamlphp-upgrade-notes-2.0.md b/docs/simplesamlphp-upgrade-notes-2.0.md
index 16361546d..3c00209ab 100644
--- a/docs/simplesamlphp-upgrade-notes-2.0.md
+++ b/docs/simplesamlphp-upgrade-notes-2.0.md
@@ -27,8 +27,11 @@ Functional changes
validated if present and requests not passing validation will be refused.
- In the core:TargetedID authproc-filter, the `attributename` setting has been renamed to `identifyingAttribute`.
- The default encryption algorithm is set from `AES128_CBC` to `AES128_GCM`.
- It is possible to switch back via the `sharedkey_algorithm`. Note however that CBC is vulnerable to the Padding oracle attack.
+ It is possible to switch back via the `sharedkey_algorithm`.
+ Note however that CBC is vulnerable to the Padding oracle attack.
- All support for the Shibboleth 1.3 / SAML 1.1 protocol has been removed.
+- Unsolicited responses are denied by default. If you need this functionality,
+ it can be enabled by setting `enable.saml20-unsolicited` to `true`.
Configuration changes
---------------------
diff --git a/modules/saml/lib/Auth/Source/SP.php b/modules/saml/lib/Auth/Source/SP.php
index 7a23034d6..ad5b7ab0e 100644
--- a/modules/saml/lib/Auth/Source/SP.php
+++ b/modules/saml/lib/Auth/Source/SP.php
@@ -10,6 +10,7 @@ use SAML2\Constants;
use SAML2\Exception\Protocol\NoAvailableIDPException;
use SAML2\Exception\Protocol\NoPassiveException;
use SAML2\Exception\Protocol\NoSupportedIDPException;
+use SAML2\Exception\ProtocolViolationException;
use SAML2\LogoutRequest;
use SAML2\XML\saml\NameID;
use SimpleSAML\Assert\Assert;
@@ -1137,6 +1138,15 @@ class SP extends \SimpleSAML\Auth\Source
$state['Attributes'] = $authProcState['Attributes'];
+ $config = Configuration::getInstance();
+ $allowUnsolicited = $config->getBoolean('enable.saml20-unsolicited', false);
+
+ Assert::true(
+ $allowUnsolicited,
+ 'Received an unsolicited response, which is against SAML2INT specification.',
+ ProtocolViolationException::class,
+ );
+
if (isset($state['saml:sp:isUnsolicited']) && (bool) $state['saml:sp:isUnsolicited']) {
if (!empty($state['saml:sp:RelayState'])) {
$redirectTo = $state['saml:sp:RelayState'];
diff --git a/modules/saml/www/sp/saml2-acs.php b/modules/saml/www/sp/saml2-acs.php
index 7cc0ce42f..87b95ad36 100644
--- a/modules/saml/www/sp/saml2-acs.php
+++ b/modules/saml/www/sp/saml2-acs.php
@@ -7,6 +7,7 @@
use SAML2\Binding;
use SAML2\Assertion;
use SAML2\Exception\Protocol\UnsupportedBindingException;
+use SAML2\Exception\ProtocolViolationException;
use SAML2\HTTPArtifact;
use SAML2\Response;
use SimpleSAML\Assert\Assert;
@@ -94,11 +95,24 @@ if (!empty($stateId)) {
$state = Auth\State::loadState($stateId, 'saml:sp:sso');
} catch (Exception $e) {
// something went wrong,
- Logger::warning('Could not load state specified by InResponseTo: ' . $e->getMessage() .
- ' Processing response as unsolicited.');
+ Logger::warning(
+ sprintf(
+ 'Could not load state specified by InResponseTo: %s Processing response as unsolicited.',
+ $e->getMessage(),
+ ),
+ );
}
}
+$config = Configuration::getInstance();
+$allowUnsolicited = $config->getBoolean('enable.saml20-unsolicited', false);
+
+Assert::true(
+ $allowUnsolicited,
+ 'Received an unsolicited response, which is against SAML2INT specification.',
+ ProtocolViolationException::class,
+);
+
if ($state) {
// check that the authentication source is correct
Assert::keyExists($state, 'saml:sp:AuthId');
@@ -151,12 +165,12 @@ $expire = null;
$attributes = [];
$foundAuthnStatement = false;
-foreach ($assertions as $assertion) {
- // check for duplicate assertion (replay attack)
- $config = Configuration::getInstance();
- $storeType = $config->getString('store.type', 'phpsession');
+// check for duplicate assertion (replay attack)
+$storeType = $config->getString('store.type', 'phpsession');
+
+$store = StoreFactory::getInstance($storeType);
- $store = StoreFactory::getInstance($storeType);
+foreach ($assertions as $assertion) {
if ($store !== false) {
$aID = $assertion->getId();
if ($store->get('saml.AssertionReceived', $aID) !== null) {
--
GitLab