diff --git a/.travis.yml b/.travis.yml index 27fc1c5d..78284458 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,9 +17,13 @@ matrix: allow_failures: - php: hhvm +before_script: + - if [[ "$TRAVIS_PHP_VERSION" == "7.3" ]]; then composer require --dev vimeo/psalm; fi + script: - if [[ $EXECUTE_COVERAGE == 'true' ]]; then phpunit --coverage-clover clover.xml tests; fi - if [[ $EXECUTE_COVERAGE != 'true' ]]; then phpunit tests; fi + - if [[ "$TRAVIS_PHP_VERSION" == "7.3" ]]; then vendor/bin/psalm; fi after_success: - if [[ $EXECUTE_COVERAGE == 'true' ]]; then bash <(curl -s https://codecov.io/bash); fi diff --git a/psalm.xml b/psalm.xml new file mode 100644 index 00000000..22bc7432 --- /dev/null +++ b/psalm.xml @@ -0,0 +1,50 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/XMLSecEnc.php b/src/XMLSecEnc.php index f2b26796..6cf7491d 100644 --- a/src/XMLSecEnc.php +++ b/src/XMLSecEnc.php @@ -2,6 +2,7 @@ namespace RobRichards\XMLSecLibs; use DOMDocument; +use DOMElement; use DOMNode; use DOMXPath; use Exception; @@ -60,8 +61,8 @@ class XMLSecEnc const URI = 3; const XMLENCNS = 'http://www.w3.org/2001/04/xmlenc#'; - /** @var null|DOMDocument */ - private $encdoc = null; + /** @var DOMDocument */ + private $encdoc; /** @var null|DOMNode */ private $rawNode = null; @@ -122,7 +123,7 @@ public function setNode($node) * @param bool $replace Whether the encrypted node should be replaced in the original tree. Default is true. * @throws Exception * - * @return DOMElement The -element. + * @return DOMNode The -element. */ public function encryptNode($objKey, $replace = true) { @@ -153,6 +154,7 @@ public function encryptNode($objKey, $replace = true) $this->encdoc->documentElement->setAttribute('Type', self::Content); break; default: + // from now on, this guarantees that $this->type is either self::Element or self::Content throw new Exception('Type is currently not supported'); } @@ -165,21 +167,23 @@ public function encryptNode($objKey, $replace = true) $cipherValue->appendChild($value); if ($replace) { - switch ($this->type) { - case (self::Element): - if ($this->rawNode->nodeType == XML_DOCUMENT_NODE) { - return $this->encdoc; - } - $importEnc = $this->rawNode->ownerDocument->importNode($this->encdoc->documentElement, true); - $this->rawNode->parentNode->replaceChild($importEnc, $this->rawNode); - return $importEnc; - case (self::Content): - $importEnc = $this->rawNode->ownerDocument->importNode($this->encdoc->documentElement, true); - while ($this->rawNode->firstChild) { - $this->rawNode->removeChild($this->rawNode->firstChild); - } - $this->rawNode->appendChild($importEnc); - return $importEnc; + if ($this->type === self::Element) { + if ($this->rawNode->nodeType == XML_DOCUMENT_NODE) { + return $this->encdoc; + } + /** @var DOMNode $importEnc */ + $importEnc = $this->rawNode->ownerDocument->importNode($this->encdoc->documentElement, true); + $this->rawNode->parentNode->replaceChild($importEnc, $this->rawNode); + return $importEnc; + + } else { // self::Content + /** @var DOMNode $importEnc */ + $importEnc = $this->rawNode->ownerDocument->importNode($this->encdoc->documentElement, true); + while ($this->rawNode->firstChild) { + $this->rawNode->removeChild($this->rawNode->firstChild); + } + $this->rawNode->appendChild($importEnc); + return $importEnc; } } else { return $this->encdoc->documentElement; @@ -249,10 +253,13 @@ public function getCipherValue() * @param XMLSecurityKey $objKey The decryption key that should be used when decrypting the node. * @param boolean $replace Whether we should replace the encrypted node in the XML document with the decrypted data. The default is true. * - * @return string|DOMElement The decrypted data. + * @return DOMNode|string The decrypted data. */ public function decryptNode($objKey, $replace=true) { + if (empty($this->rawNode)) { + throw new Exception('Node to encrypt has not been set'); + } if (! $objKey instanceof XMLSecurityKey) { throw new Exception('Invalid Key'); } @@ -272,7 +279,7 @@ public function decryptNode($objKey, $replace=true) $this->rawNode->parentNode->replaceChild($importEnc, $this->rawNode); return $importEnc; case (self::Content): - if ($this->rawNode->nodeType == XML_DOCUMENT_NODE) { + if ($this->rawNode instanceof DOMDocument) { $doc = $this->rawNode; } else { $doc = $this->rawNode->ownerDocument; @@ -316,7 +323,7 @@ public function encryptKey($srcKey, $rawKey, $append=true) $this->encKey = $encKey; } $encMethod = $encKey->appendChild($this->encdoc->createElementNS(self::XMLENCNS, 'xenc:EncryptionMethod')); - $encMethod->setAttribute('Algorithm', $srcKey->getAlgorith()); + $encMethod->setAttribute('Algorithm', $srcKey->getAlgorithm()); if (! empty($srcKey->name)) { $keyInfo = $encKey->appendChild($this->encdoc->createElementNS('http://www.w3.org/2000/09/xmldsig#', 'dsig:KeyInfo')); $keyInfo->appendChild($this->encdoc->createElementNS('http://www.w3.org/2000/09/xmldsig#', 'dsig:KeyName', $srcKey->name)); @@ -336,7 +343,7 @@ public function encryptKey($srcKey, $rawKey, $append=true) /** * @param XMLSecurityKey $encKey - * @return DOMElement|string + * @return DOMNode|string * @throws Exception */ public function decryptKey($encKey) @@ -389,7 +396,10 @@ public function locateKey($node=null) $query = ".//xmlsecenc:EncryptionMethod"; $nodeset = $xpath->query($query, $node); if ($encmeth = $nodeset->item(0)) { - $attrAlgorithm = $encmeth->getAttribute("Algorithm"); + if (!$encmeth instanceof DOMElement) { + return null; + } + $attrAlgorithm = $encmeth->getAttribute("Algorithm"); try { $objKey = new XMLSecurityKey($attrAlgorithm, array('type' => 'private')); } catch (Exception $e) { @@ -402,18 +412,19 @@ public function locateKey($node=null) } /** - * @param null|XMLSecurityKey $objBaseKey + * @param XMLSecurityKey $objBaseKey * @param null|DOMNode $node * @return null|XMLSecurityKey * @throws Exception */ - public static function staticLocateKeyInfo($objBaseKey=null, $node=null) + public static function staticLocateKeyInfo($objBaseKey, $node=null) { if (empty($node) || (! $node instanceof DOMNode)) { return null; } + /** @var DOMDocument|null $doc */ $doc = $node->ownerDocument; - if (!$doc) { + if ($doc === null) { return null; } @@ -472,6 +483,7 @@ public static function staticLocateKeyInfo($objBaseKey=null, $node=null) $id = substr($uri, 1); $query = '//xmlsecenc:EncryptedKey[@Id="'.XPath::filterAttrValue($id, XPath::DOUBLE_QUOTE).'"]'; + /** @var DOMElement|null $keyElement */ $keyElement = $xpath->query($query)->item(0); if (!$keyElement) { throw new Exception("Unable to locate EncryptedKey with @Id='$id'."); @@ -496,11 +508,11 @@ public static function staticLocateKeyInfo($objBaseKey=null, $node=null) } /** - * @param null|XMLSecurityKey $objBaseKey + * @param XMLSecurityKey $objBaseKey * @param null|DOMNode $node * @return null|XMLSecurityKey */ - public function locateKeyInfo($objBaseKey=null, $node=null) + public function locateKeyInfo($objBaseKey, $node=null) { if (empty($node)) { $node = $this->rawNode; diff --git a/src/XMLSecurityDSig.php b/src/XMLSecurityDSig.php index ada217b1..681369b1 100644 --- a/src/XMLSecurityDSig.php +++ b/src/XMLSecurityDSig.php @@ -74,8 +74,8 @@ class XMLSecurityDSig '; - /** @var DOMElement|null */ - public $sigNode = null; + /** @var DOMElement */ + public $sigNode; /** @var array */ public $idKeys = array(); @@ -86,7 +86,7 @@ class XMLSecurityDSig /** @var string|null */ private $signedInfo = null; - /** @var DomXPath|null */ + /** @var DOMXPath|null */ private $xPathCtx = null; /** @var string|null */ @@ -130,13 +130,18 @@ private function resetXPathObj() } /** - * Returns the XPathObj or null if xPathCtx is set and sigNode is empty. + * Returns the XPathObj. * - * @return DOMXPath|null + * @return DOMXPath + * + * @throws Exception if xPathCtx is set and sigNode is empty. */ private function getXPathObj() { - if (empty($this->xPathCtx) && ! empty($this->sigNode)) { + if (empty($this->xPathCtx)) { + if (empty($this->sigNode)) { + throw new Exception('Invalid signature node, cannot create XPath object.'); + } $xpath = new DOMXPath($this->sigNode->ownerDocument); $xpath->registerNamespace('secdsig', self::XMLDSIGNS); $this->xPathCtx = $xpath; @@ -153,7 +158,7 @@ private function getXPathObj() */ public static function generateGUID($prefix='pfx') { - $uuid = md5(uniqid(mt_rand(), true)); + $uuid = md5(uniqid((string)mt_rand(), true)); $guid = $prefix.substr($uuid, 0, 8)."-". substr($uuid, 8, 4)."-". substr($uuid, 12, 4)."-". @@ -193,8 +198,12 @@ public function locateSignature($objDoc, $pos=0) $xpath->registerNamespace('secdsig', self::XMLDSIGNS); $query = ".//secdsig:Signature"; $nodeset = $xpath->query($query, $objDoc); - $this->sigNode = $nodeset->item($pos); - return $this->sigNode; + $node = $nodeset->item($pos); + if ($node !== null) { + /** @var DOMElement $node */ + $this->sigNode = $node; + } + return $node; } return null; } @@ -235,7 +244,8 @@ public function setCanonicalMethod($method) $query = './'.$this->searchpfx.':SignedInfo'; $nodeset = $xpath->query($query, $this->sigNode); if ($sinfo = $nodeset->item(0)) { - $query = './'.$this->searchpfx.'CanonicalizationMethod'; + $query = './'.$this->searchpfx.':CanonicalizationMethod'; + /** @var \DOMNodeList<\DOMElement> $nodeset */ $nodeset = $xpath->query($query, $sinfo); if (! ($canonNode = $nodeset->item(0))) { $canonNode = $this->createNewSignNode('CanonicalizationMethod'); @@ -248,12 +258,12 @@ public function setCanonicalMethod($method) /** * @param DOMNode $node - * @param string $canonicalmethod - * @param null|array $arXPath - * @param null|array $prefixList + * @param string|null $canonicalmethod + * @param array|null $arXPath + * @param array|null $prefixList * @return string */ - private function canonicalizeData($node, $canonicalmethod, $arXPath=null, $prefixList=null) + private function canonicalizeData($node, $canonicalmethod = null, $arXPath=null, $prefixList=null) { $exclusive = false; $withComments = false; @@ -277,6 +287,7 @@ private function canonicalizeData($node, $canonicalmethod, $arXPath=null, $prefi if (is_null($arXPath) && ($node instanceof DOMNode) && ($node->ownerDocument !== null) && $node->isSameNode($node->ownerDocument->documentElement)) { /* Check for any PI or comments as they would have been excluded */ $element = $node; + $refnode = null; while ($refnode = $element->previousSibling) { if ($refnode->nodeType == XML_PI_NODE || (($refnode->nodeType == XML_COMMENT_NODE) && $withComments)) { break; @@ -288,6 +299,7 @@ private function canonicalizeData($node, $canonicalmethod, $arXPath=null, $prefi } } + /** @psalm-suppress PossiblyNullArgument */ return $node->C14N($exclusive, $withComments, $arXPath, $prefixList); } @@ -307,6 +319,7 @@ public function canonicalizeSignedInfo() $query = "./secdsig:CanonicalizationMethod"; $nodeset = $xpath->query($query, $signInfoNode); if ($canonNode = $nodeset->item(0)) { + /** @var DOMElement $canonNode */ $canonicalmethod = $canonNode->getAttribute('Algorithm'); } $this->signedInfo = $this->canonicalizeData($signInfoNode, $canonicalmethod); @@ -438,9 +451,10 @@ public function processTransforms($refNode, $objData, $includeCommentNodes = tru $node = $transform->firstChild; while ($node) { if ($node->localName == 'XPath') { - $arXPath = array(); - $arXPath['query'] = '(.//. | .//@* | .//namespace::*)['.$node->nodeValue.']'; - $arXpath['namespaces'] = array(); + $arXPath = [ + 'query' => '(.//. | .//@* | .//namespace::*)['.$node->nodeValue.']', + 'namespaces' => [], + ]; $nslist = $xpath->query('./namespace::*', $node); foreach ($nslist AS $nsnode) { if ($nsnode->localName != "xml") { @@ -455,13 +469,13 @@ public function processTransforms($refNode, $objData, $includeCommentNodes = tru } } if ($data instanceof DOMNode) { - $data = $this->canonicalizeData($objData, $canonicalMethod, $arXPath, $prefixList); + return $this->canonicalizeData($objData, $canonicalMethod, $arXPath, $prefixList); } return $data; } /** - * @param DOMNode $refNode + * @param DOMElement $refNode * @return bool */ public function processRefNode($refNode) @@ -476,32 +490,29 @@ public function processRefNode($refNode) if ($uri = $refNode->getAttribute("URI")) { $arUrl = parse_url($uri); - if (empty($arUrl['path'])) { - if ($identifier = $arUrl['fragment']) { - - /* This reference identifies a node with the given id by using - * a URI on the form "#identifier". This should not include comments. - */ - $includeCommentNodes = false; - - $xPath = new DOMXPath($refNode->ownerDocument); - if ($this->idNS && is_array($this->idNS)) { - foreach ($this->idNS as $nspf => $ns) { - $xPath->registerNamespace($nspf, $ns); - } + if (empty($arUrl['path']) && ($identifier = $arUrl['fragment'])) { + /* This reference identifies a node with the given id by using + * a URI on the form "#identifier". This should not include comments. + */ + $includeCommentNodes = false; + + $xPath = new DOMXPath($refNode->ownerDocument); + if ($this->idNS && is_array($this->idNS)) { + foreach ($this->idNS as $nspf => $ns) { + $xPath->registerNamespace($nspf, $ns); } - $iDlist = '@Id="'.XPath::filterAttrValue($identifier, XPath::DOUBLE_QUOTE).'"'; - if (is_array($this->idKeys)) { - foreach ($this->idKeys as $idKey) { - $iDlist .= " or @".XPath::filterAttrName($idKey).'="'. - XPath::filterAttrValue($identifier, XPath::DOUBLE_QUOTE).'"'; - } + } + $iDlist = '@Id="'.XPath::filterAttrValue($identifier, XPath::DOUBLE_QUOTE).'"'; + if (is_array($this->idKeys)) { + foreach ($this->idKeys as $idKey) { + $iDlist .= " or @".XPath::filterAttrName($idKey).'="'. + XPath::filterAttrValue($identifier, XPath::DOUBLE_QUOTE).'"'; } - $query = '//*['.$iDlist.']'; - $dataObject = $xPath->query($query)->item(0); - } else { - $dataObject = $refNode->ownerDocument; } + $query = '//*['.$iDlist.']'; + $dataObject = $xPath->query($query)->item(0); + } else { + $dataObject = $refNode->ownerDocument; } } else { /* This reference identifies the root node with an empty URI. This should @@ -511,6 +522,9 @@ public function processRefNode($refNode) $dataObject = $refNode->ownerDocument; } + if ($dataObject === null) { + throw new Exception('$refNode has no owner document or the reference was not found'); + } $data = $this->processTransforms($refNode, $dataObject, $includeCommentNodes); if (!$this->validateDigest($refNode, $data)) { return false; @@ -529,7 +543,7 @@ public function processRefNode($refNode) } /** - * @param DOMNode $refNode + * @param DOMElement $refNode * @return null */ public function getRefNodeID($refNode) @@ -799,6 +813,9 @@ public function verify($objKey) if (empty($sigValue)) { throw new Exception("Unable to locate SignatureValue"); } + if ($this->signedInfo === null) { + throw new Exception("No signed info available. Remember to call canonicalizeSignedInfo() before calling verify()."); + } return $objKey->verifySignature($this->signedInfo, base64_decode($sigValue)); } @@ -822,6 +839,7 @@ public function sign($objKey, $appendToNode = null) if ($appendToNode != null) { $this->resetXPathObj(); $this->appendSignature($appendToNode); + /** @var DOMElement $appendToNode->lastChild */ $this->sigNode = $appendToNode->lastChild; } if ($xpath = $this->getXPathObj()) { @@ -830,6 +848,7 @@ public function sign($objKey, $appendToNode = null) if ($sInfo = $nodeset->item(0)) { $query = "./secdsig:SignatureMethod"; $nodeset = $xpath->query($query, $sInfo); + /** @var DOMElement $sMethod */ $sMethod = $nodeset->item(0); $sMethod->setAttribute('Algorithm', $objKey->type); $data = $this->canonicalizeData($sInfo, $this->canonicalMethod); @@ -1090,10 +1109,6 @@ public function appendToKeyInfo($node) $baseDoc = $parentRef->ownerDocument; $xpath = $this->getXPathObj(); - if (empty($xpath)) { - $xpath = new DOMXPath($parentRef->ownerDocument); - $xpath->registerNamespace('secdsig', self::XMLDSIGNS); - } $query = "./secdsig:KeyInfo"; $nodeset = $xpath->query($query, $parentRef); diff --git a/src/XMLSecurityKey.php b/src/XMLSecurityKey.php index 6c01f0cc..df0964d2 100644 --- a/src/XMLSecurityKey.php +++ b/src/XMLSecurityKey.php @@ -62,8 +62,8 @@ class XMLSecurityKey /** @var array */ private $cryptParams = array(); - /** @var int|string */ - public $type = 0; + /** @var string */ + public $type; /** @var mixed|null */ public $key = null; @@ -252,6 +252,9 @@ public function generateSessionKey() $keysize = $this->cryptParams['keysize']; $key = openssl_random_pseudo_bytes($keysize); + if ($key === false) { + throw new Exception('Generating session key failed.'); + } if ($this->type === self::TRIPLEDES_CBC) { /* Make sure that the generated key has the proper parity bits set. @@ -601,12 +604,11 @@ public function signData($data) */ public function verifySignature($data, $signature) { - switch ($this->cryptParams['library']) { - case 'openssl': - return $this->verifyOpenSSL($data, $signature); - case (self::HMAC_SHA1): - $expectedSignature = hash_hmac("sha1", $data, $this->key, true); - return strcmp($signature, $expectedSignature) == 0; + if ($this->cryptParams['library'] === 'openssl') { + return $this->verifyOpenSSL($data, $signature); + } else { // self::HMAC_SHA1 + $expectedSignature = hash_hmac("sha1", $data, $this->key, true); + return strcmp($signature, $expectedSignature) == 0; } } @@ -632,7 +634,8 @@ public function getAlgorithm() * * @param int $type * @param string $string - * @return null|string + * @return string + * @throws \Exception If $string is 2^16 or more bytes long. */ public static function makeAsnSegment($type, $string) { @@ -655,7 +658,7 @@ public static function makeAsnSegment($type, $string) } else if ($length < 0x010000) { $output = sprintf("%c%c%c%c%s", $type, 0x82, $length / 0x0100, $length % 0x0100, $string); } else { - $output = null; + throw new \Exception('Invalid length for $string.'); } return $output; } @@ -666,6 +669,7 @@ public static function makeAsnSegment($type, $string) * @param string $modulus * @param string $exponent * @return string + * @throws \Exception */ public static function convertRSA($modulus, $exponent) { @@ -702,7 +706,7 @@ public function serializeKey($parent) * Will return the X509 certificate in PEM-format if this key represents * an X509 certificate. * - * @return string The X509 certificate or null if this key doesn't represent an X509-certificate. + * @return string|null The X509 certificate or null if this key doesn't represent an X509-certificate. */ public function getX509Certificate() {