From 51527c3cfe17a71d26cd45810cddc089bec5215f Mon Sep 17 00:00:00 2001
From: Olav Morken <olav.morken@uninett.no>
Date: Tue, 4 Aug 2009 13:24:00 +0000
Subject: [PATCH] saml2_Message: Fix signing of assertion and response when
 redirect.sign === FALSE.

git-svn-id: https://simplesamlphp.googlecode.com/svn/trunk@1628 44740490-163a-0410-bde0-09ae8108e29a
---
 lib/SAML2/SignedElement.php   | 27 +++++++++++++++++++
 modules/saml2/lib/Message.php | 50 ++++++++++++++++++++++-------------
 2 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/lib/SAML2/SignedElement.php b/lib/SAML2/SignedElement.php
index aef6eaf5c..903cf568c 100644
--- a/lib/SAML2/SignedElement.php
+++ b/lib/SAML2/SignedElement.php
@@ -21,6 +21,16 @@ interface SAML2_SignedElement {
 	public function validate(XMLSecurityKey $key);
 
 
+	/**
+	 * Set the certificates that should be included in the element.
+	 *
+	 * The certificates should be strings with the PEM encoded data.
+	 *
+	 * @param array $certificates  An array of certificates.
+	 */
+	public function setCertificates(array $certificates);
+
+
 	/**
 	 * Retrieve the certificates that are included in the element (if any).
 	 *
@@ -28,4 +38,21 @@ interface SAML2_SignedElement {
 	 */
 	public function getCertificates();
 
+
+	/**
+	 * Retrieve the private key we should use to sign the element.
+	 *
+	 * @return XMLSecurityKey|NULL The key, or NULL if no key is specified.
+	 */
+	public function getSignatureKey();
+
+
+	/**
+	 * Set the private key we should use to sign the element.
+	 *
+	 * If the key is NULL, the message will be sent unsigned.
+	 *
+	 * @param XMLSecurityKey|NULL $key
+	 */
+	public function setSignatureKey(XMLsecurityKey $signatureKey = NULL);
 }
\ No newline at end of file
diff --git a/modules/saml2/lib/Message.php b/modules/saml2/lib/Message.php
index 705bde06c..31a4c2091 100644
--- a/modules/saml2/lib/Message.php
+++ b/modules/saml2/lib/Message.php
@@ -31,21 +31,13 @@ class sspmod_saml2_Message {
 
 
 	/**
-	 * Add signature key and and senders certificate to message.
+	 * Add signature key and and senders certificate to an element (Message or Assertion).
 	 *
-	 * @param SAML2_Message $message  The message we should add the data to.
-	 * @param SimpleSAML_Configuration $metadata  The metadata of the sender.
+	 * @param SimpleSAML_Configuration $srcMetadata  The metadata of the sender.
+	 * @param SimpleSAML_Configuration $dstMetadata  The metadata of the recipient.
+	 * @param SAML2_Message $element  The element we should add the data to.
 	 */
-	private static function addSign(SimpleSAML_Configuration $srcMetadata, SimpleSAML_Configuration $dstMetadata, SAML2_message $message) {
-
-		$signingEnabled = $dstMetadata->getBoolean('redirect.sign', NULL);
-		if ($signingEnabled === NULL) {
-			$signingEnabled = $srcMetadata->getBoolean('redirect.sign', FALSE);
-		}
-		if (!$signingEnabled) {
-			return;
-		}
-
+	private static function addSign(SimpleSAML_Configuration $srcMetadata, SimpleSAML_Configuration $dstMetadata, SAML2_SignedElement $element) {
 
 		$srcMetadata = $srcMetadata->toArray();
 
@@ -58,7 +50,7 @@ class sspmod_saml2_Message {
 		}
 		$privateKey->loadKey($keyArray['PEM'], FALSE);
 
-		$message->setSignatureKey($privateKey);
+		$element->setSignatureKey($privateKey);
 
 		if ($certArray === NULL) {
 			/* We don't have a certificate to add. */
@@ -70,7 +62,28 @@ class sspmod_saml2_Message {
 			return;
 		}
 
-		$message->setCertificates(array($certArray['PEM']));
+		$element->setCertificates(array($certArray['PEM']));
+	}
+
+
+	/**
+	 * Add signature key and and senders certificate to 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 add the data to.
+	 */
+	private static function addRedirectSign(SimpleSAML_Configuration $srcMetadata, SimpleSAML_Configuration $dstMetadata, SAML2_message $message) {
+
+		$signingEnabled = $dstMetadata->getBoolean('redirect.sign', NULL);
+		if ($signingEnabled === NULL) {
+			$signingEnabled = $srcMetadata->getBoolean('redirect.sign', FALSE);
+		}
+		if (!$signingEnabled) {
+			return;
+		}
+
+		self::addSign($srcMetadata, $dstMetadata, $message);
 	}
 
 
@@ -345,7 +358,7 @@ class sspmod_saml2_Message {
 		$ar->setForceAuthn($spMetadata->getBoolean('ForceAuthn', FALSE));
 		$ar->setIsPassive($spMetadata->getBoolean('IsPassive', FALSE));
 
-		self::addSign($spMetadata, $idpMetadata, $ar);
+		self::addRedirectSign($spMetadata, $idpMetadata, $ar);
 
 		return $ar;
 	}
@@ -364,7 +377,7 @@ class sspmod_saml2_Message {
 		$lr->setIssuer($srcMetadata->getString('entityid'));
 		$lr->setDestination($dstMetadata->getString('SingleLogoutService'));
 
-		self::addSign($srcMetadata, $dstMetadata, $lr);
+		self::addRedirectSign($srcMetadata, $dstMetadata, $lr);
 
 		return $lr;
 	}
@@ -388,7 +401,7 @@ class sspmod_saml2_Message {
 		}
 		$lr->setDestination($dst);
 
-		self::addSign($srcMetadata, $dstMetadata, $lr);
+		self::addRedirectSign($srcMetadata, $dstMetadata, $lr);
 
 		return $lr;
 	}
@@ -492,6 +505,7 @@ class sspmod_saml2_Message {
 		$config = SimpleSAML_Configuration::getInstance();
 
 		$a = new SAML2_Assertion();
+		self::addSign($srcMetadata, $dstMetadata, $a);
 		$a->setIssuer($srcMetadata->getString('entityid'));
 		$a->setDestination($dstMetadata->getString('AssertionConsumerService'));
 		$a->setValidAudiences(array($dstMetadata->getString('entityid')));
-- 
GitLab