From 83b67d2a944eea21790ac8ae7a75aebda24ba1e4 Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tim.dijen@minbzk.nl>
Date: Mon, 15 Mar 2021 13:13:05 +0100
Subject: [PATCH] Conform SAML2INT; check AuthnReqsignature unless specifically
disabled (#1440)
* Conform SAML2INT; check AuthnRequest signature unless specifically disabled
---
docs/simplesamlphp-reference-idp-hosted.md | 5 +++++
docs/simplesamlphp-reference-sp-remote.md | 5 +++++
docs/simplesamlphp-upgrade-notes-2.0.md | 1 +
modules/saml/lib/Message.php | 13 +++++++++----
4 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/docs/simplesamlphp-reference-idp-hosted.md b/docs/simplesamlphp-reference-idp-hosted.md
index 8291a610b..c0655fee3 100644
--- a/docs/simplesamlphp-reference-idp-hosted.md
+++ b/docs/simplesamlphp-reference-idp-hosted.md
@@ -355,6 +355,11 @@ The following SAML 2.0 options are available:
`validate.authnrequest`
: Whether we require signatures on authentication requests sent to this IdP.
+ Set it to:
+
+ true: authnrequest must be signed (and signature will be validated)
+ null: authnrequest may be signed, if it is, signature will be validated
+ false: authnrequest signature is never checked
: Note that this option also exists in the SP-remote metadata, and
any value in the SP-remote metadata overrides the one configured
diff --git a/docs/simplesamlphp-reference-sp-remote.md b/docs/simplesamlphp-reference-sp-remote.md
index 2acc6e8e4..b3557c0e9 100644
--- a/docs/simplesamlphp-reference-sp-remote.md
+++ b/docs/simplesamlphp-reference-sp-remote.md
@@ -296,6 +296,11 @@ The following options can be set:
`validate.authnrequest`
: Whether we require signatures on authentication requests sent from this SP.
+ Set it to:
+
+ true: authnrequest must be signed (and signature will be validated)
+ null: authnrequest may be signed, if it is, signature will be validated
+ false: authnrequest signature is never checked
: 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.
diff --git a/docs/simplesamlphp-upgrade-notes-2.0.md b/docs/simplesamlphp-upgrade-notes-2.0.md
index 34732656d..979d6d96e 100644
--- a/docs/simplesamlphp-upgrade-notes-2.0.md
+++ b/docs/simplesamlphp-upgrade-notes-2.0.md
@@ -13,6 +13,7 @@ Upgrade notes for SimpleSAMLphp 2.0
- If you're using the core:TargetedID authproc-filter, note that the `attributename` setting has been renamed to `identifyingAttribute`.
- The default encryption algorithm is set from AES128_CBC to AES128_GCM. If you're upgrading from an existing implementation, you may want
to manually switch back the `sharedkey_algorithm`. Note that CBC is vulnerable to the Padding oracle attack.
+- In compliancy with SAML2INT, AuthnRequests that are signed will have their signature validated unless specifically disabled by setting `validate.authnrequest` to `false`. If unset, or set to true, signatures will be validated and requests not passing validation will be refused.
- The following classes have been migrated to non-static:
+ lib/SimpleSAMLphp\Utils\Arrays
+ lib/SimpleSAMLphp\Utils\Attributes
diff --git a/modules/saml/lib/Message.php b/modules/saml/lib/Message.php
index 1d3c9ab5d..2ff01ea52 100644
--- a/modules/saml/lib/Message.php
+++ b/modules/saml/lib/Message.php
@@ -213,7 +213,14 @@ class Message
}
}
- if ($enabled === null) {
+ // If not specifically set to false, the signature must be checked to conform to SAML2INT
+ if (
+ (isset($_REQUEST['Signature'])
+ || $message->isMessageConstructedWithSignature() === true)
+ && ($enabled !== false)
+ ) {
+ $enabled = true;
+ } elseif ($enabled === null) {
$enabled = $srcMetadata->getBoolean('redirect.validate', null);
if ($enabled === null) {
$enabled = $dstMetadata->getBoolean('redirect.validate', false);
@@ -222,9 +229,7 @@ class Message
if (!$enabled) {
return;
- }
-
- if (!self::checkSign($srcMetadata, $message)) {
+ } elseif (!self::checkSign($srcMetadata, $message)) {
throw new SSP_Error\Exception(
'Validation of received messages enabled, but no signature found on message.'
);
--
GitLab