From 631d32193dcbc41be00d3fb7fa9517ddb04f1774 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Mon, 13 Jul 2026 21:09:31 +0200 Subject: [PATCH 1/2] Fix phpdoc --- tests/XML/ds/RSAKeyValueTest.php | 2 +- tests/XML/ds/SignatureMethodTest.php | 2 +- tests/XML/ds/SignatureTest.php | 2 +- tests/XML/ds/X509IssuerSerialTest.php | 2 +- tests/XML/xenc/AgreementMethodTest.php | 6 +++--- tests/XML/xenc/DHKeyValueTest.php | 6 +++--- tests/XML/xenc/EncryptedDataTest.php | 2 +- tests/XML/xenc/EncryptedKeyTest.php | 4 ++-- tests/XML/xenc11/DerivedKeyTest.php | 4 ++-- 9 files changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/XML/ds/RSAKeyValueTest.php b/tests/XML/ds/RSAKeyValueTest.php index e44b0bd2..e842de99 100644 --- a/tests/XML/ds/RSAKeyValueTest.php +++ b/tests/XML/ds/RSAKeyValueTest.php @@ -75,7 +75,7 @@ public function testMarshallingElementOrder(): void $modulus = XPath::xpQuery($RSAKeyValueElement, './ds:Modulus', $xpCache); $this->assertCount(1, $modulus); - /** @var \DOMElement[] $RSAKeyValueElements */ + /** @var \Dom\Element[] $RSAKeyValueElements */ $RSAKeyValueElements = XPath::xpQuery($RSAKeyValueElement, './ds:Modulus/following-sibling::*', $xpCache); // Test ordering of RSAKeyValue contents diff --git a/tests/XML/ds/SignatureMethodTest.php b/tests/XML/ds/SignatureMethodTest.php index bc423af3..6f024358 100644 --- a/tests/XML/ds/SignatureMethodTest.php +++ b/tests/XML/ds/SignatureMethodTest.php @@ -96,7 +96,7 @@ public function testMarshallingElementOrder(): void $hmacOutputLength = XPath::xpQuery($signatureMethodElement, './ds:HMACOutputLength', $xpCache); $this->assertCount(1, $hmacOutputLength); - /** @var \DOMElement[] $signatureMethodElements */ + /** @var \Dom\Element[] $signatureMethodElements */ $signatureMethodElements = XPath::xpQuery( $signatureMethodElement, './ds:HMACOutputLength/following-sibling::*', diff --git a/tests/XML/ds/SignatureTest.php b/tests/XML/ds/SignatureTest.php index 5b624082..ddb72cd3 100644 --- a/tests/XML/ds/SignatureTest.php +++ b/tests/XML/ds/SignatureTest.php @@ -139,7 +139,7 @@ public function testMarshallingElementOrdering(): void $signedInfo = XPath::xpQuery($signatureElement, './ds:SignedInfo', $xpCache); $this->assertCount(1, $signedInfo); - /** @var \DOMElement[] $signatureElements */ + /** @var \Dom\Element[] $signatureElements */ $signatureElements = XPath::xpQuery($signatureElement, './ds:SignedInfo/following-sibling::*', $xpCache); // Test ordering of Signature contents diff --git a/tests/XML/ds/X509IssuerSerialTest.php b/tests/XML/ds/X509IssuerSerialTest.php index d72e14e2..673734bd 100644 --- a/tests/XML/ds/X509IssuerSerialTest.php +++ b/tests/XML/ds/X509IssuerSerialTest.php @@ -90,7 +90,7 @@ public function testMarshallingElementOrdering(): void $issuerName = XPath::xpQuery($x509IssuerSerialElement, './ds:X509IssuerName', $xpCache); $this->assertCount(1, $issuerName); - /** @var \DOMElement[] $x509IssuerSerialElements */ + /** @var \Dom\Element[] $x509IssuerSerialElements */ $x509IssuerSerialElements = XPath::xpQuery( $x509IssuerSerialElement, './ds:X509IssuerName/following-sibling::*', diff --git a/tests/XML/xenc/AgreementMethodTest.php b/tests/XML/xenc/AgreementMethodTest.php index 46f02657..3355b4ba 100644 --- a/tests/XML/xenc/AgreementMethodTest.php +++ b/tests/XML/xenc/AgreementMethodTest.php @@ -207,18 +207,18 @@ public function testMarshallingElementOrdering(): void [$digestMethod], ); - // Marshall it to a \DOMElement + // Marshall it to a \Dom\Element $agreementMethodElement = $agreementMethod->toXML(); $xpCache = XPath::getXPath($agreementMethodElement); // Test for an KA-Nonce - /** @var \DOMElement[] $kaNonceElements */ + /** @var \Dom\Element[] $kaNonceElements */ $kaNonceElements = XPath::xpQuery($agreementMethodElement, './xenc:KA-Nonce', $xpCache); $this->assertCount(1, $kaNonceElements); // Test ordering of AgreementMethod contents - /** @var \DOMElement[] $agreementMethodElements */ + /** @var \Dom\Element[] $agreementMethodElements */ $agreementMethodElements = XPath::xpQuery( $agreementMethodElement, './xenc:KA-Nonce/following-sibling::*', diff --git a/tests/XML/xenc/DHKeyValueTest.php b/tests/XML/xenc/DHKeyValueTest.php index ec25b918..f18bcde2 100644 --- a/tests/XML/xenc/DHKeyValueTest.php +++ b/tests/XML/xenc/DHKeyValueTest.php @@ -79,18 +79,18 @@ public function testMarshalling(): void */ public function testMarshallingElementOrder(): void { - // Marshall it to a \DOMElement + // Marshall it to a \Dom\Element $dhKeyValueElement = self::$dhKeyValue->toXML(); $xpCache = XPath::getXPath($dhKeyValueElement); // Test for an P - /** @var \DOMElement[] $pElements */ + /** @var \Dom\Element[] $pElements */ $pElements = XPath::xpQuery($dhKeyValueElement, './xenc:P', $xpCache); $this->assertCount(1, $pElements); // Test ordering of DHKeyValue contents - /** @var \DOMElement[] $dhKeyValueElements */ + /** @var \Dom\Element[] $dhKeyValueElements */ $dhKeyValueElements = XPath::xpQuery( $dhKeyValueElement, './xenc:P/following-sibling::*', diff --git a/tests/XML/xenc/EncryptedDataTest.php b/tests/XML/xenc/EncryptedDataTest.php index d2388f79..ad8f9875 100644 --- a/tests/XML/xenc/EncryptedDataTest.php +++ b/tests/XML/xenc/EncryptedDataTest.php @@ -143,7 +143,7 @@ public function testMarshallingElementOrdering(): void $this->assertCount(1, $encryptedDataElements); // Test ordering of EncryptedData contents - /** @var \DOMElement[] $encryptedDataElements */ + /** @var \Dom\Element[] $encryptedDataElements */ $encryptedDataElements = XPath::xpQuery( $encryptedDataElement, './xenc:EncryptionMethod/following-sibling::*', diff --git a/tests/XML/xenc/EncryptedKeyTest.php b/tests/XML/xenc/EncryptedKeyTest.php index d6f3cdb9..a63367f3 100644 --- a/tests/XML/xenc/EncryptedKeyTest.php +++ b/tests/XML/xenc/EncryptedKeyTest.php @@ -181,7 +181,7 @@ public function testMarshallingElementOrdering(): void ]), ); - // Marshall it to a \DOMElement + // Marshall it to a \Dom\Element $encryptedKeyElement = $encryptedKey->toXML(); $xpCache = XPath::getXPath($encryptedKeyElement); @@ -194,7 +194,7 @@ public function testMarshallingElementOrdering(): void $this->assertCount(1, $encryptedKeyElements); // Test ordering of EncryptedKey contents - /** @var \DOMElement[] $encryptedKeyElements */ + /** @var \Dom\Element[] $encryptedKeyElements */ $encryptedKeyElements = XPath::xpQuery( $encryptedKeyElement, './xenc:ReferenceList/following-sibling::*', diff --git a/tests/XML/xenc11/DerivedKeyTest.php b/tests/XML/xenc11/DerivedKeyTest.php index 3177193b..9d50a15a 100644 --- a/tests/XML/xenc11/DerivedKeyTest.php +++ b/tests/XML/xenc11/DerivedKeyTest.php @@ -150,12 +150,12 @@ public function testMarshallingElementOrder(): void $xpCache = XPathUtils::getXPath($dkElement); // Test for a KeyDerivationMethod - /** @var \DOMElement[] $keyDerivationMethodElements */ + /** @var \Dom\Element[] $keyDerivationMethodElements */ $keyDerivationMethodElements = XPathUtils::xpQuery($dkElement, './xenc11:KeyDerivationMethod', $xpCache); $this->assertCount(1, $keyDerivationMethodElements); // Test ordering of DerivedKey contents - /** @var \DOMElement[] $dkElements */ + /** @var \Dom\Element[] $dkElements */ $dkElements = XPathUtils::xpQuery($dkElement, './xenc11:KeyDerivationMethod/following-sibling::*', $xpCache); $this->assertCount(3, $dkElements); From e0e83126f0fb4e7487cc887606e59731108d6e5d Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Sat, 25 Jul 2026 09:13:01 +0200 Subject: [PATCH 2/2] Add hardening for ISO 10126 padding --- src/Backend/OpenSSL.php | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/Backend/OpenSSL.php b/src/Backend/OpenSSL.php index 991ac4b9..b66b570a 100644 --- a/src/Backend/OpenSSL.php +++ b/src/Backend/OpenSSL.php @@ -23,6 +23,7 @@ use function openssl_verify; use function ord; use function str_repeat; +use function strlen; use function substr; /** @@ -176,6 +177,7 @@ public function decrypt( if ($plaintext === false) { throw new OpenSSLException('Cannot decrypt data'); } + return $this->useAuthTag ? $plaintext : $this->unpad($plaintext); } @@ -299,11 +301,16 @@ public function setDigestAlg(string $digest): void * Pad a plaintext using ISO 10126 padding. * * @param string $plaintext The plaintext to pad. - * * @return string The padded plaintext. + * + * @throws \SimpleSAML\XMLSecurity\Exception\RuntimeException in case padding failed */ public function pad(string $plaintext): string { + if ($this->blocksize > 256) { + throw new RuntimeException('Block size higher than 256 not allowed'); + } + $padchr = $this->blocksize - (mb_strlen($plaintext) % $this->blocksize); $pattern = chr($padchr); return $plaintext . str_repeat($pattern, $padchr); @@ -314,11 +321,29 @@ public function pad(string $plaintext): string * Remove an existing ISO 10126 padding from a given plaintext. * * @param string $plaintext The padded plaintext. - * * @return string The plaintext without the padding. + * + * @throws \SimpleSAML\XMLSecurity\Exception\OpenSSLException in case unpadding failed */ public function unpad(string $plaintext): string { - return substr($plaintext, 0, -ord(substr($plaintext, -1))); + $len = mb_strlen($plaintext); + if ($len === 0) { + /* + * Use a single, generic error for every decryption failure. Do not + * reveal whether the padding (as opposed to the ciphertext) was the + * cause: distinguishable padding errors turn unauthenticated CBC + * decryption into a padding oracle (plaintext recovery). + */ + throw new OpenSSLException('Cannot decrypt data'); + } + + $padLen = ord($plaintext[$len - 1]); + $blocksize = $this->blocksize ?? 16; + if ($padLen < 1 || $padLen > $blocksize || $padLen > $len) { + throw new OpenSSLException('Cannot decrypt data'); + } + + return substr($plaintext, 0, -$padLen); } }