Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
yphoenix opened this issue Jul 27, 2023 · 15 comments
Closed

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

yphoenix opened this issue Jul 27, 2023 · 15 comments

Comments

@yphoenix
Copy link

yphoenix commented Jul 27, 2023

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
Copy link
Contributor

tvdijen commented Jul 27, 2023

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
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

tvdijen commented Jul 27, 2023

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
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

tvdijen commented Jul 27, 2023

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

@yphoenix
Copy link
Author

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
Copy link
Author

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

@tvdijen
Copy link
Contributor

tvdijen commented Jul 27, 2023

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

@yphoenix
Copy link
Author

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

@yphoenix
Copy link
Author

FYI tracked it down to insertBefore() - SAML-Toolkits/php-saml#562

@tvdijen
Copy link
Contributor

tvdijen commented Jul 28, 2023

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
Copy link
Author

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants