Security fix for consent

git-svn-id: https://simplesamlphp.googlecode.com/svn/trunk@394 44740490-163a-0410-bde0-09ae8108e29a

3 files changed, 28 insertions, 14 deletions

diff --git a/lib/SimpleSAML/Consent/Consent.php b/lib/SimpleSAML/Consent/Consent.php
index 0865e0f..0ada2fe 100644
--- a/lib/SimpleSAML/Consent/Consent.php
+++ b/lib/SimpleSAML/Consent/Consent.php
@@ -25,21 +25,28 @@ class SimpleSAML_Consent_Consent {
 	
 	private $attributes;
 	private $filteredattributes;
+	private $consent_cookie;
 	
 	private $storageerror;
 	
 	/**
 	 * Constructor
 	 */
-	public function __construct($config, $session, $spentityid, $idpentityid, $attributes, $filteredattributes) {
+	public function __construct($config, $session, $spentityid, $idpentityid, $attributes, $filteredattributes, $consent_cookie) {
 
 		$this->config = $config;
-		$this->salt = $this->config->getValue('consent_salt', 'eae46a3d5cb6e8546dded65be9855e5c');
+		$this->salt = $this->config->getValue('consent_salt');
+		
+		if (!isset($this->salt)) {
+			throw new Exception('Configuration parameter [consent_salt] is not set.');
+		}
+		
 		$this->attributes = $attributes;
 		$this->filteredattributes = $filteredattributes;
 		$this->session = $session;
 		$this->spentityid = $spentityid;
 		$this->idpentityid = $idpentityid;
+		$this->consent_cookie = $consent_cookie;
 		
 		$this->storageerror = false;
 	}
@@ -83,7 +90,7 @@ class SimpleSAML_Consent_Consent {
 	 */
 	private function getTargetedID($hashed_userid) {
 		
-		return hash('sha1', $hashed_userid . $salt . $this->spentityid);
+		return hash('sha1', $hashed_userid . $this->salt . $this->spentityid);
 		
 	}
 
@@ -103,6 +110,14 @@ class SimpleSAML_Consent_Consent {
 	public function consent() {
 		
 
+		if (isset($_GET['consent']) ) {
+			
+			if ($_GET['consent'] != $this->consent_cookie) {
+				throw new Exception('Consent cookie set to wrong value.');
+			}
+			
+		}
+
 		/**
 		 * The user has manually accepted consent and chosen not to store the consent
 		 * for later.
diff --git a/templates/default/en/consent.php b/templates/default/en/consent.php
index 3ee6e0f..716e12f 100644
--- a/templates/default/en/consent.php
+++ b/templates/default/en/consent.php
@@ -9,15 +9,15 @@
 
 
 		<form action="<?php echo htmlspecialchars($data['consenturl']); ?>">
-			<input type="submit" value="Yes">
-			<input type="hidden" name="consent" value="1">
-			<input type="hidden" name="RequestID" value="<?php echo $this->data['requestid']; ?>">
+			<input type="submit" value="Yes" />
+			<input type="hidden" name="consent" value="<?php echo htmlspecialchars($this->data['consent_cookie']); ?>" />
+			<input type="hidden" name="RequestID" value="<?php echo htmlspecialchars($this->data['requestid']); ?>" />
 			<?php if($this->data['usestorage']) { ?>
-				<input type="checkbox" name="saveconsent" id="saveconsent" value="1"> remember consent
+				<input type="checkbox" name="saveconsent" id="saveconsent" value="1" /> remember consent
 			<?php } ?>
 		</form>
 		<form action="<?php echo htmlspecialchars($this->data['noconsent']); ?>" method="GET">
-			<input type="submit" value="No">
+			<input type="submit" value="No" />
 		</form>
 
 
diff --git a/www/saml2/idp/SSOService.php b/www/saml2/idp/SSOService.php
index 21df10b..b21a583 100644
--- a/www/saml2/idp/SSOService.php
+++ b/www/saml2/idp/SSOService.php
@@ -32,7 +32,7 @@ try {
 	$idpentityid = $metadata->getMetaDataCurrentEntityID('saml20-idp-hosted');
 	$idpmetadata = $metadata->getMetaDataCurrent('saml20-idp-hosted');
 	
-	if (!array_key_exists($idpmetadata, 'auth')) {
+	if (!array_key_exists('auth', $idpmetadata)) {
 		throw new Exception('Missing mandatory parameter in SAML 2.0 IdP Hosted Metadata: [auth]');
 	}
 	
@@ -69,7 +69,8 @@ if (isset($_GET['SAMLRequest'])) {
 		 * Create an assoc array of the request to store in the session cache.
 		 */
 		$requestcache = array(
-			'Issuer'    => $issuer
+			'Issuer'    => $issuer,
+			'ConsentCookie' => SimpleSAML_Utilities::generateID(),
 		);
 		if ($relaystate = $authnrequest->getRelayState() )
 			$requestcache['RelayState'] = $relaystate;
@@ -202,7 +203,7 @@ if (!isset($session) || !$session->isValid($authority) ) {
 		}
 		if ($requireconsent) {
 			
-			$consent = new SimpleSAML_Consent_Consent($config, $session, $spentityid, $idpentityid, $attributes, $filteredattributes);
+			$consent = new SimpleSAML_Consent_Consent($config, $session, $spentityid, $idpentityid, $attributes, $filteredattributes, $requestcache['ConsentCookie']);
 			
 			if (!$consent->consent()) {
 				
@@ -212,6 +213,7 @@ if (!isset($session) || !$session->isValid($authority) ) {
 				$t->data['attributes'] = $filteredattributes;
 				$t->data['consenturl'] = SimpleSAML_Utilities::selfURLNoQuery();
 				$t->data['requestid'] = $requestid;
+				$t->data['consent_cookie'] = $requestcache['ConsentCookie'];
 				$t->data['usestorage'] = $consent->useStorage();
 				$t->data['noconsent'] = '/' . $config->getBaseURL() . 'noconsent.php';
 				$t->show();
@@ -223,9 +225,6 @@ if (!isset($session) || !$session->isValid($authority) ) {
 		
 		
 		
-		
-		
-		
 		// Generate an SAML 2.0 AuthNResponse message
 		$ar = new SimpleSAML_XML_SAML20_AuthnResponse($config, $metadata);
 		$authnResponseXML = $ar->generate($idpentityid, $spentityid, $requestid, null, $filteredattributes);