From 5ba8f3809a02c9b601d3d25f44f01d16ed439b28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fermi=CC=81n?= Date: Thu, 1 May 2025 11:54:54 +0200 Subject: [PATCH 1/4] New XmlSignatureVerifier property to set canonicalization (C14N) $exclusive param. --- src/XmlSignatureVerifier.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/XmlSignatureVerifier.php b/src/XmlSignatureVerifier.php index d9ea208..1ffeb3a 100644 --- a/src/XmlSignatureVerifier.php +++ b/src/XmlSignatureVerifier.php @@ -19,16 +19,19 @@ final class XmlSignatureVerifier private bool $preserveWhiteSpace; + private bool $exclusive; + /** * The constructor. * * @param CryptoVerifierInterface $cryptoVerifier * @param bool $preserveWhiteSpace To remove redundant white spaces */ - public function __construct(CryptoVerifierInterface $cryptoVerifier, bool $preserveWhiteSpace = true) + public function __construct(CryptoVerifierInterface $cryptoVerifier, bool $preserveWhiteSpace = true, $exclusive = true) { $this->cryptoVerifier = $cryptoVerifier; $this->preserveWhiteSpace = $preserveWhiteSpace; + $this->exclusive = $exclusive; $this->xmlReader = new XmlReader(); } @@ -82,13 +85,13 @@ public function verifyDocument(DOMDocument $xml): bool $signatureValueElement = $this->xmlReader->queryDomNode($xpath, '//xmlns:SignatureValue', $signedInfoNode); $signatureValueElement->nodeValue = ''; - $canonicalData = $signedInfoNode->C14N(true, false); + $canonicalData = $signedInfoNode->C14N($this->exclusive, false); $xml2 = new DOMDocument(); $xml2->preserveWhiteSpace = true; $xml2->formatOutput = true; $xml2->loadXML($canonicalData); - $canonicalData = $xml2->C14N(true, false); + $canonicalData = $xml2->C14N($this->exclusive, false); $isValidSignature = $this->cryptoVerifier->verify($canonicalData, $signatureValue, $signatureAlgorithm); @@ -124,8 +127,8 @@ private function checkDigest(DOMDocument $xml, DOMXPath $xpath, string $algorith $signatureNode->parentNode->removeChild($signatureNode); } - // Canonicalize the content, exclusive and without comments - $canonicalData = $xml->C14N(true, false); + // Canonicalize the content without comments + $canonicalData = $xml->C14N($this->exclusive, false); $digestValue2 = $this->cryptoVerifier->computeDigest($canonicalData, $algorithm); From ea1bad2b60160dc190464597852e0bfae792701d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fermi=CC=81n?= Date: Thu, 1 May 2025 17:15:25 +0200 Subject: [PATCH 2/4] New XmlSigner property to set canonicalization (C14N) $exclusive param. --- src/XmlSignatureVerifier.php | 2 +- src/XmlSigner.php | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/XmlSignatureVerifier.php b/src/XmlSignatureVerifier.php index 1ffeb3a..c7a9aa3 100644 --- a/src/XmlSignatureVerifier.php +++ b/src/XmlSignatureVerifier.php @@ -27,7 +27,7 @@ final class XmlSignatureVerifier * @param CryptoVerifierInterface $cryptoVerifier * @param bool $preserveWhiteSpace To remove redundant white spaces */ - public function __construct(CryptoVerifierInterface $cryptoVerifier, bool $preserveWhiteSpace = true, $exclusive = true) + public function __construct(CryptoVerifierInterface $cryptoVerifier, bool $preserveWhiteSpace = true, bool $exclusive = true) { $this->cryptoVerifier = $cryptoVerifier; $this->preserveWhiteSpace = $preserveWhiteSpace; diff --git a/src/XmlSigner.php b/src/XmlSigner.php index 1b28466..0478d1e 100644 --- a/src/XmlSigner.php +++ b/src/XmlSigner.php @@ -20,9 +20,15 @@ final class XmlSigner private CryptoSignerInterface $cryptoSigner; - public function __construct(CryptoSignerInterface $cryptoSigner) + private bool $preserveWhiteSpace; + + private bool $exclusive; + + public function __construct(CryptoSignerInterface $cryptoSigner, bool $preserveWhiteSpace = true, bool $exclusive = true) { $this->xmlReader = new XmlReader(); + $this->preserveWhiteSpace = $preserveWhiteSpace; + $this->exclusive = $exclusive; $this->cryptoSigner = $cryptoSigner; } @@ -42,7 +48,7 @@ public function signXml(string $data): string $xml = new DOMDocument(); // Whitespaces must be preserved - $xml->preserveWhiteSpace = true; + $xml->preserveWhiteSpace = $this->preserveWhiteSpace; $xml->formatOutput = false; $xml->loadXML($data); @@ -71,7 +77,7 @@ public function signDocument(DOMDocument $document, DOMElement $element = null): throw new XmlSignerException('Invalid XML document element'); } - $canonicalData = $element->C14N(true, false); + $canonicalData = $element->C14N($this->exclusive, false); // Calculate and encode digest value $digestValue = $this->cryptoSigner->computeDigest($canonicalData); @@ -176,7 +182,7 @@ private function appendSignature(DOMDocument $xml, string $digestValue): void } // http://www.soapclient.com/XMLCanon.html - $c14nSignedInfo = $signedInfoElement->C14N(true, false); + $c14nSignedInfo = $signedInfoElement->C14N($this->exclusive, false); $signatureValue = $this->cryptoSigner->computeSignature($c14nSignedInfo); From 2841b379106bba736f332ba19833c1df22ddad25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fermi=CC=81n?= Date: Thu, 15 May 2025 11:56:34 +0200 Subject: [PATCH 3/4] Fixes PHP CS Fixer warnings --- src/Algorithm.php | 2 +- src/XmlSignatureVerifier.php | 1 + src/XmlSigner.php | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Algorithm.php b/src/Algorithm.php index 54151c8..f8a3058 100644 --- a/src/Algorithm.php +++ b/src/Algorithm.php @@ -56,7 +56,7 @@ final class Algorithm * @param string $signatureMethodAlgorithm * @param string|null $digestMethodAlgorithm */ - public function __construct(string $signatureMethodAlgorithm, string $digestMethodAlgorithm = null) + public function __construct(string $signatureMethodAlgorithm, ?string $digestMethodAlgorithm = null) { $this->setSignatureMethodAlgorithm($signatureMethodAlgorithm); $this->setDigestMethodAlgorithm($digestMethodAlgorithm ?? $signatureMethodAlgorithm); diff --git a/src/XmlSignatureVerifier.php b/src/XmlSignatureVerifier.php index c7a9aa3..694d8b4 100644 --- a/src/XmlSignatureVerifier.php +++ b/src/XmlSignatureVerifier.php @@ -26,6 +26,7 @@ final class XmlSignatureVerifier * * @param CryptoVerifierInterface $cryptoVerifier * @param bool $preserveWhiteSpace To remove redundant white spaces + * @param bool $exclusive Exclusive canonicalization. @see https://www.php.net/manual/en/domnode.c14n.php */ public function __construct(CryptoVerifierInterface $cryptoVerifier, bool $preserveWhiteSpace = true, bool $exclusive = true) { diff --git a/src/XmlSigner.php b/src/XmlSigner.php index 0478d1e..77a0adc 100644 --- a/src/XmlSigner.php +++ b/src/XmlSigner.php @@ -69,7 +69,7 @@ public function signXml(string $data): string * * @return string The signed XML as string */ - public function signDocument(DOMDocument $document, DOMElement $element = null): string + public function signDocument(DOMDocument $document, ?DOMElement $element = null): string { $element = $element ?? $document->documentElement; @@ -186,7 +186,7 @@ private function appendSignature(DOMDocument $xml, string $digestValue): void $signatureValue = $this->cryptoSigner->computeSignature($c14nSignedInfo); - $xpath = new DOMXpath($xml); + $xpath = new DOMXPath($xml); $signatureValueElement = $this->xmlReader->queryDomNode($xpath, '//SignatureValue', $signatureElement); $signatureValueElement->nodeValue = base64_encode($signatureValue); } From bb0ffa181751b0c2b2ec26c7d2c1ad235be52856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fermi=CC=81n?= Date: Fri, 20 Jun 2025 18:07:03 +0200 Subject: [PATCH 4/4] Fixes phpstan warnings and errors --- .cs.php | 2 +- src/X509Reader.php | 2 +- src/XmlSignatureVerifier.php | 7 +++-- src/XmlSigner.php | 7 +++-- tests/XmlSignatureTest.php | 53 ++++++++++++++++++++---------------- 5 files changed, 41 insertions(+), 30 deletions(-) diff --git a/.cs.php b/.cs.php index 2bba5e9..a113324 100644 --- a/.cs.php +++ b/.cs.php @@ -19,7 +19,7 @@ 'array_syntax' => ['syntax' => 'short'], 'cast_spaces' => ['space' => 'none'], 'concat_space' => ['spacing' => 'one'], - 'compact_nullable_typehint' => true, + 'compact_nullable_type_declaration' => true, 'declare_equal_normalize' => ['space' => 'single'], 'general_phpdoc_annotation_remove' => [ 'annotations' => [ diff --git a/src/X509Reader.php b/src/X509Reader.php index 125b4e7..a9b1c67 100644 --- a/src/X509Reader.php +++ b/src/X509Reader.php @@ -74,6 +74,6 @@ public function toRawBase64(OpenSSLCertificate $certificate): string preg_match(self::PEM_REGEX_PATTERN, $exportedCertificate, $matches); - return str_replace(["\r\n", "\n"], '', trim($matches[1])); + return str_replace(["\r\n", "\n"], '', trim($matches[1] ?? '')); } } diff --git a/src/XmlSignatureVerifier.php b/src/XmlSignatureVerifier.php index 694d8b4..af43fe6 100644 --- a/src/XmlSignatureVerifier.php +++ b/src/XmlSignatureVerifier.php @@ -28,8 +28,11 @@ final class XmlSignatureVerifier * @param bool $preserveWhiteSpace To remove redundant white spaces * @param bool $exclusive Exclusive canonicalization. @see https://www.php.net/manual/en/domnode.c14n.php */ - public function __construct(CryptoVerifierInterface $cryptoVerifier, bool $preserveWhiteSpace = true, bool $exclusive = true) - { + public function __construct( + CryptoVerifierInterface $cryptoVerifier, + bool $preserveWhiteSpace = true, + bool $exclusive = true, + ) { $this->cryptoVerifier = $cryptoVerifier; $this->preserveWhiteSpace = $preserveWhiteSpace; $this->exclusive = $exclusive; diff --git a/src/XmlSigner.php b/src/XmlSigner.php index 77a0adc..21da694 100644 --- a/src/XmlSigner.php +++ b/src/XmlSigner.php @@ -24,8 +24,11 @@ final class XmlSigner private bool $exclusive; - public function __construct(CryptoSignerInterface $cryptoSigner, bool $preserveWhiteSpace = true, bool $exclusive = true) - { + public function __construct( + CryptoSignerInterface $cryptoSigner, + bool $preserveWhiteSpace = true, + bool $exclusive = true, + ) { $this->xmlReader = new XmlReader(); $this->preserveWhiteSpace = $preserveWhiteSpace; $this->exclusive = $exclusive; diff --git a/tests/XmlSignatureTest.php b/tests/XmlSignatureTest.php index 53d6246..f237f31 100644 --- a/tests/XmlSignatureTest.php +++ b/tests/XmlSignatureTest.php @@ -48,38 +48,43 @@ public function testSignAndVerify(string $privateKeyFile, string $publicKeyFile, foreach ($files as $filename) { foreach ($algos as $algo) { - $privateKeyStore = new PrivateKeyStore(); + $this->testFileAndAlgo($filename, $algo, $privateKeyFile, $publicKeyFile, $password); + } + } + } - if (pathinfo($privateKeyFile, PATHINFO_EXTENSION) === 'p12') { - $privateKeyStore->loadFromPkcs12(file_get_contents($privateKeyFile), $password); - } else { - $privateKeyStore->loadFromPem(file_get_contents($privateKeyFile), $password); - } + private function testFileAndAlgo(string $filename, string $algo, string $privateKeyFile, string $publicKeyFile, string $password) + { + $privateKeyStore = new PrivateKeyStore(); - $algorithm = new Algorithm($algo, $algo); - $cryptoSigner = new CryptoSigner($privateKeyStore, $algorithm); + if (pathinfo($privateKeyFile, PATHINFO_EXTENSION) === 'p12') { + $privateKeyStore->loadFromPkcs12(file_get_contents($privateKeyFile), $password); + } else { + $privateKeyStore->loadFromPem(file_get_contents($privateKeyFile), $password); + } - $xmlSigner = new XmlSigner($cryptoSigner); - $xmlSigner->setReferenceUri(''); + $algorithm = new Algorithm($algo, $algo); + $cryptoSigner = new CryptoSigner($privateKeyStore, $algorithm); - $signedXml = $xmlSigner->signXml(file_get_contents($filename)); + $xmlSigner = new XmlSigner($cryptoSigner); + $xmlSigner->setReferenceUri(''); - // verify - $publicKeyStore = new PublicKeyStore(); - if (pathinfo($publicKeyFile, PATHINFO_EXTENSION) === 'p12') { - $publicKeyStore->loadFromPkcs12(file_get_contents($publicKeyFile), $password); - } else { - $publicKeyStore->loadFromPem(file_get_contents($publicKeyFile)); - } + $signedXml = $xmlSigner->signXml(file_get_contents($filename)); - $cryptoVerifier = new CryptoVerifier($publicKeyStore); - $xmlSignatureVerifier = new XmlSignatureVerifier($cryptoVerifier); + // verify + $publicKeyStore = new PublicKeyStore(); + if (pathinfo($publicKeyFile, PATHINFO_EXTENSION) === 'p12') { + $publicKeyStore->loadFromPkcs12(file_get_contents($publicKeyFile), $password); + } else { + $publicKeyStore->loadFromPem(file_get_contents($publicKeyFile)); + } - $isValid = $xmlSignatureVerifier->verifyXml($signedXml); + $cryptoVerifier = new CryptoVerifier($publicKeyStore); + $xmlSignatureVerifier = new XmlSignatureVerifier($cryptoVerifier); - $this->assertTrue($isValid); - } - } + $isValid = $xmlSignatureVerifier->verifyXml($signedXml); + + $this->assertTrue($isValid); } /**