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

Reference validation failed after (minor!) PHP update from 8.2.7 to 8.2.8 #257

Open
Navi2016 opened this issue Sep 18, 2023 · 8 comments
Open

Comments

@Navi2016
Copy link

I'm running into an issue after updating my server from PHP 8.2.7 to 8.2.8.

Getting:
Reference validation failed from vendor/robrichards/xmlseclibs/src/XMLSecurityDSig.php:614

and SAML login fails.

It's in a Laravel 10 app using 24slides/laravel-saml2 (2.2.0), which uses onelogin/php-saml (4.1.0), which uses robrichards/xmlseclibs (3.1.1)

Everything works great on PHP 8.2.7 and before, no longer on 8.2.8 and up.

I have tried a possible fix in:
SAML-Toolkits/php-saml#562

But the author of onelogin/php-saml thinks the issue might be in xmlseclibs.

Any idea?

@tvdijen
Copy link
Contributor

tvdijen commented Sep 18, 2023

PHP has been messing a lot with the builtin XML/DOM-libraries and as an author of simplesamlphp/xml-security I've seen similar issues over the last few months.

Most notably:

  • Fixed bug #55294 and #47530 and #47847 (various namespace reconciliation issues).

This for me was my primary suspect, because in my case marshalling/unmarshalling XML documents led to different documents with namespace declarations in different places

@Navi2016
Copy link
Author

Navi2016 commented Sep 20, 2023

Well it seems this package no longer works in PHP 8.2.8 and up because of these changes. Pretty strange they would make these updates in a minor release.

When looking at $data in method validateDigest i do see a difference between PHP 8.2.7 and 8.2.8.

In 8.2.7 there are no prefixes in the <Assertion message, while in 8.2.8 every node in the <Assertion message has a saml: prefix.

afbeelding

So it does 2 passes, the first SAML response validates fine, it's the second pass validating the Assertion which does not validate.

Could this be the cause? How could this be fixed?

Update: When disabling assertion encryption on the idp (codegreencreative/laravel-samlidp) via encrypt_assertion false, it works. See also the related issue below. Still needs a fix though.

@nielsdos
Copy link

PHP DOM maintainer here.
This is caused by a regression introduced by a bugfix in PHP. This is a behaviour change that's caused by an unintended side-effect of using a newer namespace API from libxml2.
I have opened a PR to fix this at PHP's end by restoring the original behaviour.

Pretty strange they would make these updates in a minor release.

This was not an intentional behaviour change, and our tests didn't catch this.
In any case, if you see such a sudden break during a minor upgrade, please also report this to PHP's bug tracker so we can help quicker. Thanks.

@tvdijen

This comment was marked as off-topic.

@nielsdos
Copy link

I think we've seen a similar thing around June/July this year, but these things are so hard to pinpoint..

Yes, I found too that DOM bugs often occur when specific sequences of actions/tree changes are performed. So when they happen it's hard to find why and where.
We had a regression in June I think with tree manipulations but that should be fixed.
In any case, just let me know if you see something weird by opening a bug. I rather have a bug report too much than one too little 🙂

@tvdijen

This comment was marked as off-topic.

@Navi2016
Copy link
Author

Navi2016 commented Nov 1, 2023

Issue seems to be resolved in PHP 8.2.12!

@tvdijen
Copy link
Contributor

tvdijen commented Nov 1, 2023

From the changelog:

Version 8.2.12
26 Oct 2023

[..]

DOM:
    Restore old namespace reconciliation behaviour.

Makes sense

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

3 participants