Fix #283.New method of importing a decrypted assertion into the XML document to replace the EncryptedAssertion. Fix signature issues on Signed Encrypted Assertions with default namespace

Author: pitbulk

lib/Saml2/Response.php

@@ -720,7 +720,7 @@ public function getAttributesWithFriendlyName()
         return $this->_getAttributesByKeyName('FriendlyName');
     }
 
-    private function _getAttributesByKeyName($keyName="Name")
+    private function _getAttributesByKeyName($keyName = "Name")
     {
         $attributes = array();
 
@@ -1028,14 +1028,12 @@ private function _query($query)
     protected function _decryptAssertion($dom)
     {
         $pem = $this->_settings->getSPkey();
-
         if (empty($pem)) {
             throw new OneLogin_Saml2_Error(
                 "No private key available, check settings",
                 OneLogin_Saml2_Error::PRIVATE_KEY_NOT_FOUND
             );
         }
-
         $objenc = new XMLSecEnc();
         $encData = $objenc->locateEncryptedData($dom);
         if (!$encData) {
@@ -1044,7 +1042,6 @@ protected function _decryptAssertion($dom)
                 OneLogin_Saml2_ValidationError::MISSING_ENCRYPTED_ELEMENT
             );
         }
-
         $objenc->setNode($encData);
         $objenc->type = $encData->getAttribute("Type");
         if (!$objKey = $objenc->locateKey()) {
@@ -1053,7 +1050,6 @@ protected function _decryptAssertion($dom)
                 OneLogin_Saml2_ValidationError::KEY_ALGORITHM_ERROR
             );
         }
-
         $key = null;
         if ($objKeyInfo = $objenc->locateKeyInfo($objKey)) {
             if ($objKeyInfo->isEncrypted) {
@@ -1069,36 +1065,41 @@ protected function _decryptAssertion($dom)
         if (empty($objKey->key)) {
             $objKey->loadKey($key);
         }
-
-        $decrypted = $objenc->decryptNode($objKey, true);
-
-        if ($decrypted instanceof DOMDocument) {
+        $decryptedXML = $objenc->decryptNode($objKey, false);
+        $decrypted = new DOMDocument();
+        $check = OneLogin_Saml2_Utils::loadXML($decrypted, $decryptedXML);
+        if ($check === false) {
+            throw new Exception('Error: string from decrypted assertion could not be loaded into a XML document');
+        }
+        if ($encData->parentNode instanceof DOMDocument) {
             return $decrypted;
         } else {
-            $encryptedAssertion = $decrypted->parentNode;
+            $decrypted = $decrypted->documentElement;
+            $encryptedAssertion = $encData->parentNode;
             $container = $encryptedAssertion->parentNode;
 
-            # Fix possible issue with saml namespace
-            if (!$decrypted->hasAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:saml') &&
-              !$decrypted->hasAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:saml2') &&
-              !$decrypted->hasAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns') &&
-              !$container->hasAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:saml') &&
-              !$container->hasAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:saml2')
-              ) {
+            // Fix possible issue with saml namespace
+            if (!$decrypted->hasAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:saml')
+                && !$decrypted->hasAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:saml2')
+                && !$decrypted->hasAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns')
+                && !$container->hasAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:saml')
+                && !$container->hasAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:saml2')
+            ) {
                 if (strpos($encryptedAssertion->tagName, 'saml2:') !== false) {
                     $ns = 'xmlns:saml2';
                 } else if (strpos($encryptedAssertion->tagName, 'saml:') !== false) {
                     $ns = 'xmlns:saml';
                 } else {
                     $ns = 'xmlns';
                 }
-
                 $decrypted->setAttributeNS('http://www.w3.org/2000/xmlns/', $ns, OneLogin_Saml2_Constants::NS_SAML);
             }
 
-            $container->replaceChild($decrypted, $encryptedAssertion);
+            OneLogin_Saml2_Utils::treeCopyReplace($encryptedAssertion, $decrypted);
 
-            return $decrypted->ownerDocument;
+            // Rebuild the DOM will fix issues with namespaces as well
+            $dom = new DOMDocument();
+            return OneLogin_Saml2_Utils::loadXML($dom, $container->ownerDocument->saveXML());
         }
     }
 

lib/Saml2/Utils.php

@@ -145,6 +145,48 @@ public static function validateXML($xml, $schema, $debug = false)
         return $dom;
     }
 
+    /**
+     * Import a node tree into a target document
+     * Copy it before a reference node as a sibling
+     * and at the end of the copy remove
+     * the reference node in the target document
+     * As it were 'replacing' it
+     * Leaving nested default namespaces alone
+     * (Standard importNode with deep copy
+     *  mangles nested default namespaces)
+     *
+     * The reference node must not be a DomDocument
+     * It CAN be the top element of a document
+     * Returns the copied node in the target document
+     *
+     * @param DomNode $targetNode
+     * @param DomNode $sourceNode
+     * @param bool $recurse
+     * @return DOMNode
+     * @throws Exception
+     */
+    public static function treeCopyReplace(DomNode $targetNode, DomNode $sourceNode, $recurse = false)
+    {
+        if ($targetNode->parentNode === null) {
+            throw new Exception('Illegal argument targetNode. It has no parentNode.');
+        }
+        $clonedNode = $targetNode->ownerDocument->importNode($sourceNode, false);
+        if ($recurse) {
+            $resultNode = $targetNode->appendChild($clonedNode);
+        } else {
+            $resultNode = $targetNode->parentNode->insertBefore($clonedNode, $targetNode);
+        }
+        if ($sourceNode->childNodes !== null) {
+            foreach ($sourceNode->childNodes as $child) {
+                self::treeCopyReplace($resultNode, $child, true);
+            }
+        }
+        if (!$recurse) {
+            $targetNode->parentNode->removeChild($targetNode);
+        }
+        return $resultNode;
+    }
+
     /**
      * Returns a x509 cert (adding header & footer if required).
      *