search

robrichards / xmlseclibs

A PHP library for XML Security
BSD 3-Clause "New" or "Revised" License
388 stars 181 forks source link

openssl_verify() failure with PHP 8.1 (works with 7.4) #254

Closed yphoenix closed 1 year ago

yphoenix commented 1 year ago

This one is driving me nuts. I have a SAML exchange that uses an encrypted and signed assertion in the response.

Under PHP 7.4 everything works great, the call to openssl_verify($data, $signature, $this->key, $algo); in verifyOpenSSL works perfectly and returns 1

Under PHP 8.1 everything fails, with exactly the same data being passed with the error.

error:02000068:rsa routines::bad signature
error:1C880004:Provider routines::RSA lib

The key is their public cert (this is a SAML response), the signature is sha256

Any tips appreciated, especially as to why the same code works with 7.4 and not 8.1

tvdijen commented 1 year ago

openssl is a disaster when it comes to error handling.

See this comment: https://www.php.net/manual/en/function.openssl-error-string.php#119878

This library will only show you the last error, even when there's a whole stack of error messages. In your case you are probably just missing crucial information to properly debug this.

yphoenix commented 1 year ago

Yeah, already handled the error queue thing. Now working on a minimal reproducible example that works with 7.4 and fails with 8.1

yphoenix commented 1 year ago

Pinned it down, not an openssl error, wrong value is being passed as the data, will work out whether it is a xmlseclibs or a onelogin saml issue.

tvdijen commented 1 year ago

Are you using another saml-lib that uses this lib, or what? Just out of curiosity since I happen to manage one of the saml-lib around.

yphoenix commented 1 year ago

Using the OneLogin PHP SAML Library, https://github.com/SAML-Toolkits/php-saml Looks like the signature info inside the encrypted assertion is getting mangled:

<ds:Signature><ds:SignedInfo> .... translated to:
<ns3:Signature><ns3:SignedInfo> etc

But only on PHP 8.1, and only inside the assertion. Signature info on the response is left untouched.

still tracking down exactly where and why.

yphoenix commented 1 year ago

Decryption is fine....

<ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:SignedInfo>....

Amd something happens afterwards that I'll still tracking down....

tvdijen commented 1 year ago

That library seems pretty basic considering the huge saml2 protocol.. I wouldn't expect much of that tbh

yphoenix commented 1 year ago

It's been working fine so far, but maybe this will force us to change libraries, and it seems to be a 8.1 thing, and I suspect C14N, but we'll see.

yphoenix commented 1 year ago

Seems to be a DOMXPath::query issue with 8.x, more a OneLogin SAML Lib issue, or PHP issue.

tvdijen commented 1 year ago

Yeah PHP 8.0 has known issues that won't be fixed anymore. I've been having similar issues with my lib.

yphoenix commented 1 year ago

Thanks. Tracked down to: Utils::treeCopyReplace($encryptedAssertion, $decrypted); in OneLogin's Library. Closing this and moving over there.

yphoenix commented 1 year ago

FYI tracked it down to insertBefore() - https://github.com/SAML-Toolkits/php-saml/issues/562

tvdijen commented 1 year ago

From the PHP changelog:

Version 8.1.21
06 Jul 2023

    DOM:
        Fixed bug [#55294](http://bugs.php.net/55294) and #47530 and #47847 (various namespace reconciliation issues).

Could that be it? Are you on the latest dot-version?

yphoenix commented 1 year ago

Yes, latest dot version. Haven’t gone back to find out when it was introduced and maybe. libxml issue but for now I’m working around it.

yphoenix commented 1 year ago

Looking at the release log, lots of DOM fixes in 8.1.21, looks like it may have been introduced with this version.

Fix DOMElement::append() and DOMElement::prepend() hierarchy checks.