From 99cb9218ee69a0d2d64f17fb050c0a15ec521b31 Mon Sep 17 00:00:00 2001 From: Tim van Dijen <tvdijen@gmail.com> Date: Mon, 31 Jan 2022 21:10:40 +0100 Subject: [PATCH] Rewrite --- docs/simplesamlphp-upgrade-notes-2.0.md | 2 +- modules/saml/docs/sp.md | 3 +++ modules/saml/lib/Auth/Source/SP.php | 16 +++++++--------- modules/saml/www/sp/saml2-acs.php | 23 +++++++++-------------- 4 files changed, 20 insertions(+), 24 deletions(-) diff --git a/docs/simplesamlphp-upgrade-notes-2.0.md b/docs/simplesamlphp-upgrade-notes-2.0.md index dc305801e..c878fceae 100644 --- a/docs/simplesamlphp-upgrade-notes-2.0.md +++ b/docs/simplesamlphp-upgrade-notes-2.0.md @@ -30,7 +30,7 @@ Functional changes 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 can denied by disabling it by setting `enable.saml20-unsolicited` to `false`. +- Unsolicited responses can denied by disabling it by setting `disable_unsolicited` to `true` in the SP authsource. Configuration changes --------------------- diff --git a/modules/saml/docs/sp.md b/modules/saml/docs/sp.md index b6d174eec..1e3c864ff 100644 --- a/modules/saml/docs/sp.md +++ b/modules/saml/docs/sp.md @@ -219,6 +219,9 @@ Options in the IdP-remote metadata overrides this the option in the SP configuration. +`disable_unsolicited` +: Whether this SP will refuse to process unsolicited responses. The default value is `false`. + `discoURL` : Set which IdP discovery service this SP should use. If this is unset, the IdP discovery service specified in the global option `idpdisco.url.saml20` in `config/config.php` will be used. diff --git a/modules/saml/lib/Auth/Source/SP.php b/modules/saml/lib/Auth/Source/SP.php index 508ad15b8..a0b3829ae 100644 --- a/modules/saml/lib/Auth/Source/SP.php +++ b/modules/saml/lib/Auth/Source/SP.php @@ -1137,21 +1137,19 @@ class SP extends \SimpleSAML\Auth\Source $state['Attributes'] = $authProcState['Attributes']; - $config = Configuration::getInstance(); - $allowUnsolicited = $config->getBoolean('enable.saml20-unsolicited', true); - - Assert::true( - $allowUnsolicited, - 'Unsolicited responsed are denied by configuration.', - Error\BadRequest::class, - ); - if (isset($state['saml:sp:isUnsolicited']) && (bool) $state['saml:sp:isUnsolicited']) { + $spMetadata = $source->getMetadata(); + $disableUnsolicited = $spMetadata->getBoolean('disable_unsolicited', false); + if ($disableUnsolicited === true) { + throw new Error\BadRequest('Unsolicited responses are denied by configuration.'); + } + if (!empty($state['saml:sp:RelayState'])) { $redirectTo = $state['saml:sp:RelayState']; } else { $redirectTo = $source->getMetadata()->getString('RelayState', '/'); } + self::handleUnsolicitedAuth($sourceId, $state, $redirectTo); } diff --git a/modules/saml/www/sp/saml2-acs.php b/modules/saml/www/sp/saml2-acs.php index 097463cf9..7b943c988 100644 --- a/modules/saml/www/sp/saml2-acs.php +++ b/modules/saml/www/sp/saml2-acs.php @@ -94,23 +94,17 @@ if (!empty($stateId)) { $state = Auth\State::loadState($stateId, 'saml:sp:sso'); } catch (Exception $e) { // something went wrong, - Logger::warning( - sprintf( - 'Could not load state specified by InResponseTo: %s Processing response as unsolicited.', - $e->getMessage(), - ), - ); + 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', true); - -Assert::true( - $allowUnsolicited, - 'Unsolicited responses are denied by configuration.', - Error\BadRequest::class, -); +$disableUnsolicited = $spMetadata->getBoolean('disable_unsolicited', false); +if ($state === null && $disableUnsolicited === true) { + throw new Error\BadRequest('Unsolicited responses are denied by configuration.'); +} if ($state) { // check that the authentication source is correct @@ -165,6 +159,7 @@ $attributes = []; $foundAuthnStatement = false; // check for duplicate assertion (replay attack) +$config = Configuration::getInstance(); $storeType = $config->getString('store.type', 'phpsession'); $store = StoreFactory::getInstance($storeType); -- GitLab