From 435d3394b67252a667402f84b40de2f8324976c8 Mon Sep 17 00:00:00 2001
From: Bobby Lawrence <dub357@gmail.com>
Date: Tue, 18 Jan 2022 10:36:16 -0500
Subject: [PATCH] =?UTF-8?q?added=20ability=20to=20skip=20validating=20the?=
=?UTF-8?q?=20AuthnRequest=20response=20destinatio=E2=80=A6=20(#1304)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Added ability to skip validating the AuthnRequest response destination against metadata if the message is signed and the endpoint doesn't already exist in the SP metadata.
---
docs/simplesamlphp-reference-sp-remote.md | 6 +++++
modules/saml/lib/IdP/SAML2.php | 31 ++++++++++++++++++++---
modules/saml/lib/Message.php | 6 +++--
3 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/docs/simplesamlphp-reference-sp-remote.md b/docs/simplesamlphp-reference-sp-remote.md
index 2235b35b0..176a71483 100644
--- a/docs/simplesamlphp-reference-sp-remote.md
+++ b/docs/simplesamlphp-reference-sp-remote.md
@@ -289,6 +289,12 @@ The following options can be set:
: Note that this option also exists in the IdP-hosted metadata.
The value in the SP-remote metadata overrides the value in the IdP-hosted metadata.
+`skipEndpointValidationWhenSigned`
+: Whether to skip validating that the AssertionConsumerServiceURL sent in authentication
+ requests exist in SP metadata. Only allowed for signed requests.
+ This option must be a simple boolean (true/false - although a value of false essentially has
+ no effect) or a callable. When used as a callable, the static class method must accept the
+ SP metadata config as a parameter and return a boolean.
### Encrypting assertions
diff --git a/modules/saml/lib/IdP/SAML2.php b/modules/saml/lib/IdP/SAML2.php
index f141ff0cf..ee2dc1132 100644
--- a/modules/saml/lib/IdP/SAML2.php
+++ b/modules/saml/lib/IdP/SAML2.php
@@ -190,6 +190,7 @@ class SAML2
* @param string|null $AssertionConsumerServiceURL AssertionConsumerServiceURL from request.
* @param string|null $ProtocolBinding ProtocolBinding from request.
* @param int|null $AssertionConsumerServiceIndex AssertionConsumerServiceIndex from request.
+ * @param bool $authnRequestSigned Whether or not the authn request was signed.
*
* @return array|null Array with the Location and Binding we should use for the response.
*/
@@ -198,7 +199,8 @@ class SAML2
Configuration $spMetadata,
string $AssertionConsumerServiceURL = null,
string $ProtocolBinding = null,
- int $AssertionConsumerServiceIndex = null
+ int $AssertionConsumerServiceIndex = null,
+ bool $authnRequestSigned = false
): ?array {
/* We want to pick the best matching endpoint in the case where for example
* only the ProtocolBinding is given. We therefore pick endpoints with the
@@ -251,6 +253,26 @@ class SAML2
return $firstFalse;
}
+ $skipEndpointValidation = false;
+ if ($authnRequestSigned === true) {
+ $skipEndpointValidationWhenSigned = $spMetadata->getValue('skipEndpointValidationWhenSigned', false);
+ if (is_bool($skipEndpointValidationWhenSigned) === true) {
+ $skipEndpointValidation = $skipEndpointValidationWhenSigned;
+ } elseif (is_callable($skipEndpointValidationWhenSigned) === true) {
+ $shouldSkipEndpointValidation = $skipEndpointValidationWhenSigned($spMetadata);
+ if (is_bool($shouldSkipEndpointValidation) === true) {
+ $skipEndpointValidation = $shouldSkipEndpointValidation;
+ }
+ }
+ }
+
+ if (($AssertionConsumerServiceURL !== null) && ($skipEndpointValidation === true)) {
+ Logger::info(
+ 'AssertionConsumerService specified in AuthnRequest not in metadata, using anyway because AuthnRequest signed and skipEndpointValidationWhenSigned was true'
+ );
+ return ['Location' => $AssertionConsumerServiceURL, 'Binding' => $ProtocolBinding];
+ }
+
Logger::warning('Authentication request specifies invalid AssertionConsumerService:');
if ($AssertionConsumerServiceURL !== null) {
Logger::warning('AssertionConsumerServiceURL: ' . var_export($AssertionConsumerServiceURL, true));
@@ -292,6 +314,8 @@ class SAML2
$supportedBindings[] = Constants::BINDING_PAOS;
}
+ $authnRequestSigned = false;
+
if (isset($_REQUEST['spentityid']) || isset($_REQUEST['providerId'])) {
/* IdP initiated authentication. */
@@ -372,7 +396,7 @@ class SAML2
$spEntityId = $issuer->getValue();
$spMetadata = $metadata->getMetaDataConfig($spEntityId, 'saml20-sp-remote');
- \SimpleSAML\Module\saml\Message::validateMessage($spMetadata, $idpMetadata, $request);
+ $authnRequestSigned = \SimpleSAML\Module\saml\Message::validateMessage($spMetadata, $idpMetadata, $request);
$relayState = $request->getRelayState();
@@ -424,7 +448,8 @@ class SAML2
$spMetadata,
$consumerURL,
$protocolBinding,
- $consumerIndex
+ $consumerIndex,
+ $authnRequestSigned
);
if ($acsEndpoint === null) {
throw new Exception('Unable to use any of the ACS endpoints found for SP \'' . $spEntityId . '\'');
diff --git a/modules/saml/lib/Message.php b/modules/saml/lib/Message.php
index 3074dc85c..5a91a32cd 100644
--- a/modules/saml/lib/Message.php
+++ b/modules/saml/lib/Message.php
@@ -192,6 +192,7 @@ class Message
* @param \SimpleSAML\Configuration $srcMetadata The metadata of the sender.
* @param \SimpleSAML\Configuration $dstMetadata The metadata of the recipient.
* @param \SAML2\Message $message The message we should check the signature on.
+ * @return bool Whether or not the message was validated.
*
* @throws \SimpleSAML\Error\Exception if message validation is enabled, but there is no signature in the message.
*/
@@ -199,7 +200,7 @@ class Message
Configuration $srcMetadata,
Configuration $dstMetadata,
\SAML2\Message $message
- ): void {
+ ): bool {
$enabled = null;
if ($message instanceof LogoutRequest || $message instanceof LogoutResponse) {
$enabled = $srcMetadata->getBoolean('validate.logout', null);
@@ -228,12 +229,13 @@ class Message
}
if (!$enabled) {
- return;
+ return false;
} elseif (!self::checkSign($srcMetadata, $message)) {
throw new SSP_Error\Exception(
'Validation of received messages enabled, but no signature found on message.'
);
}
+ return true;
}
--
GitLab