Skip to content

Commit 54d1489

Browse files
committed
Add some fixes to xmlseclibs. Extra protection verifying the Signature algorithm used on SignedInfo element, not only rely on verify / verifySignature methods
1 parent 8c491d5 commit 54d1489

File tree

2 files changed

+21
-4
lines changed

2 files changed

+21
-4
lines changed

extlib/xmlseclibs/xmlseclibs.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ public function loadKey($key, $isFile=false, $isCert = false) {
282282
} else {
283283
$this->key = openssl_get_privatekey($this->key, $this->passphrase);
284284
}
285-
} else if ($this->cryptParams['cipher'] == MCRYPT_RIJNDAEL_128) {
285+
} else if (isset($this->cryptParams['cipher']) && $this->cryptParams['cipher'] == MCRYPT_RIJNDAEL_128) {
286286
/* Check key length */
287287
switch ($this->type) {
288288
case (XMLSecurityKey::AES256_CBC):
@@ -724,7 +724,7 @@ public function validateDigest($refNode, $data) {
724724
$digValue = $this->calculateDigest($digestAlgorithm, $data, false);
725725
$query = 'string(./secdsig:DigestValue)';
726726
$digestValue = $xpath->evaluate($query, $refNode);
727-
return ($digValue == base64_decode($digestValue));
727+
return ($digValue === base64_decode($digestValue));
728728
}
729729

730730
public function processTransforms($refNode, $objData, $includeCommentNodes = true) {
@@ -846,8 +846,6 @@ public function processRefNode($refNode) {
846846
} else {
847847
$dataObject = $refNode->ownerDocument;
848848
}
849-
} else {
850-
$dataObject = file_get_contents($arUrl);
851849
}
852850
} else {
853851
/* This reference identifies the root node with an empty URI. This should

lib/Saml2/Utils.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,6 +1189,10 @@ public static function castKey(XMLSecurityKey $key, $algorithm, $type = 'public'
11891189
return $key;
11901190
}
11911191

1192+
if (!OneLogin_Saml2_Utils::isSupportedSigningAlgorithm($algorithm)) {
1193+
throw new \Exception('Unsupported signing algorithm.');
1194+
}
1195+
11921196
$keyInfo = openssl_pkey_get_details($key->key);
11931197
if ($keyInfo === false) {
11941198
throw new Exception('Unable to get key details from XMLSecurityKey.');
@@ -1201,6 +1205,17 @@ public static function castKey(XMLSecurityKey $key, $algorithm, $type = 'public'
12011205
return $newKey;
12021206
}
12031207

1208+
public static function isSupportedSigningAlgorithm($algorithm)
1209+
{
1210+
return in_array($algorithm, array(
1211+
XMLSecurityKey::RSA_1_5,
1212+
XMLSecurityKey::RSA_SHA1,
1213+
XMLSecurityKey::RSA_SHA256,
1214+
XMLSecurityKey::RSA_SHA384,
1215+
XMLSecurityKey::RSA_SHA512
1216+
));
1217+
}
1218+
12041219
/**
12051220
* Adds signature key and senders certificate to an element (Message or Assertion).
12061221
*
@@ -1312,6 +1327,10 @@ public static function validateSign($xml, $cert = null, $fingerprint = null, $fi
13121327
throw new Exception('We have no idea about the key');
13131328
}
13141329

1330+
if (!OneLogin_Saml2_Utils::isSupportedSigningAlgorithm($objKey->type)) {
1331+
throw new \Exception('Unsupported signing algorithm.');
1332+
}
1333+
13151334
$objXMLSecDSig->canonicalizeSignedInfo();
13161335

13171336
try {

0 commit comments

Comments
 (0)