Skip to content

Commit cb3800a

Browse files
committed
Restore enveloped Signature after canonicalization
Preserve the caller DOM and allow later references to resolve after an enveloped transform.
1 parent 1c98e9e commit cb3800a

2 files changed

Lines changed: 91 additions & 24 deletions

File tree

src/XMLSecurityDSig.php

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,7 @@ public function processTransforms($refNode, $objData, $includeCommentNodes = tru
587587
$arXPath = null;
588588
$prefixList = null;
589589
$xpathTransformCount = 0;
590+
$enveloped = false;
590591
foreach ($nodelist AS $transform) {
591592
$algorithm = $transform->getAttribute("Algorithm");
592593
switch ($algorithm) {
@@ -637,29 +638,7 @@ public function processTransforms($refNode, $objData, $includeCommentNodes = tru
637638

638639
break;
639640
case self::ENVELOPED:
640-
/*
641-
* Enveloped-signature: remove the Signature from the nodeset.
642-
* validateReference() strips sigNode from the document when any
643-
* Reference declares this transform; when signing, the Signature
644-
* is typically not yet attached. If it is still a proper descendant
645-
* of the data node, detach it here so C14N matches verification.
646-
*/
647-
if ($data instanceof DOMNode) {
648-
$sig = $this->sigNode;
649-
if ($sig instanceof DOMNode && $sig->parentNode !== null) {
650-
$ancestor = $data instanceof DOMDocument ? $data->documentElement : $data;
651-
if ($ancestor !== null && ! $sig->isSameNode($ancestor)) {
652-
$walk = $sig->parentNode;
653-
while ($walk !== null) {
654-
if ($walk->isSameNode($ancestor)) {
655-
$sig->parentNode->removeChild($sig);
656-
break;
657-
}
658-
$walk = $walk->parentNode;
659-
}
660-
}
661-
}
662-
}
641+
$enveloped = true;
663642
break;
664643
case 'http://www.w3.org/TR/1999/REC-xpath-19991116':
665644
/*
@@ -709,7 +688,42 @@ public function processTransforms($refNode, $objData, $includeCommentNodes = tru
709688
}
710689
}
711690
if ($data instanceof DOMNode) {
712-
$data = $this->canonicalizeData($objData, $canonicalMethod, $arXPath, $prefixList);
691+
$sig = null;
692+
$sigParent = null;
693+
$sigNextSibling = null;
694+
695+
/*
696+
* Temporarily detach an enveloped Signature for canonicalization,
697+
* then restore it so validation does not mutate the caller's DOM.
698+
*/
699+
if ($enveloped) {
700+
$candidate = $this->sigNode;
701+
$ancestor = $data instanceof DOMDocument ? $data->documentElement : $data;
702+
if ($candidate instanceof DOMNode
703+
&& $candidate->parentNode !== null
704+
&& $ancestor !== null
705+
&& ! $candidate->isSameNode($ancestor)) {
706+
$walk = $candidate->parentNode;
707+
while ($walk !== null) {
708+
if ($walk->isSameNode($ancestor)) {
709+
$sig = $candidate;
710+
$sigParent = $sig->parentNode;
711+
$sigNextSibling = $sig->nextSibling;
712+
$sigParent->removeChild($sig);
713+
break;
714+
}
715+
$walk = $walk->parentNode;
716+
}
717+
}
718+
}
719+
720+
try {
721+
$data = $this->canonicalizeData($objData, $canonicalMethod, $arXPath, $prefixList);
722+
} finally {
723+
if ($sig !== null && $sig->parentNode === null) {
724+
$sigParent->insertBefore($sig, $sigNextSibling);
725+
}
726+
}
713727
}
714728
return $data;
715729
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
--TEST--
2+
Enveloped transform restores Signature before validating later references
3+
--FILE--
4+
<?php
5+
require(dirname(__FILE__) . '/../xmlseclibs.php');
6+
use RobRichards\XMLSecLibs\XMLSecurityDSig;
7+
use RobRichards\XMLSecLibs\XMLSecurityKey;
8+
9+
$doc = new DOMDocument();
10+
$doc->loadXML('<Envelope><Payload>data</Payload></Envelope>');
11+
12+
$signer = new XMLSecurityDSig();
13+
$signer->setCanonicalMethod(XMLSecurityDSig::EXC_C14N);
14+
$signer->addReference(
15+
$doc,
16+
XMLSecurityDSig::SHA256,
17+
array(XMLSecurityDSig::ENVELOPED, XMLSecurityDSig::EXC_C14N),
18+
array('force_uri' => true)
19+
);
20+
21+
$object = $signer->addObject('inside-object');
22+
$object->setAttribute('Id', 'obj1');
23+
$signer->addReference(
24+
$object,
25+
XMLSecurityDSig::SHA256,
26+
array(XMLSecurityDSig::EXC_C14N),
27+
array('overwrite' => false)
28+
);
29+
30+
$privateKey = new XMLSecurityKey(XMLSecurityKey::RSA_SHA256, array('type' => 'private'));
31+
$privateKey->loadKey(dirname(__FILE__) . '/privkey.pem', true);
32+
$signer->sign($privateKey);
33+
$signer->appendSignature($doc->documentElement);
34+
35+
$verifier = new XMLSecurityDSig();
36+
$publicKey = new XMLSecurityKey(XMLSecurityKey::RSA_SHA256, array('type' => 'public'));
37+
$publicKey->loadKey(dirname(__FILE__) . '/mycert.pem', true);
38+
$verifier->allowedSignatureAlgorithms = array(XMLSecurityKey::RSA_SHA256);
39+
$verifier->allowedDigestAlgorithms = array(XMLSecurityDSig::SHA256);
40+
41+
try {
42+
$nodes = $verifier->verifyDocument($publicKey, $doc);
43+
echo "VERIFY: OK\n";
44+
echo "SIGNATURES: ".$doc->getElementsByTagNameNS(XMLSecurityDSig::XMLDSIGNS, 'Signature')->length."\n";
45+
echo isset($nodes['obj1']) ? "OBJECT: OK\n" : "OBJECT: missing\n";
46+
} catch (Exception $e) {
47+
echo "VERIFY: ".$e->getMessage()."\n";
48+
}
49+
?>
50+
--EXPECTF--
51+
VERIFY: OK
52+
SIGNATURES: 1
53+
OBJECT: OK

0 commit comments

Comments
 (0)