PHP 8.1 fixes in AttributeValue are broken

[tommy2d]

The __serialize and __unserialize additions in AttributeValue are causing fatal errors when simplesaml is used with the Dutch Digid IDP:

: PHP Fatal error: Uncaught TypeError: SAML2\XML\saml\AttributeValue::getElement(): Return value must be of type DOMElement, null returned

I can work around this issue by commenting out both the __serialize() and __unserialize() implementations, falling back to default PHP behavior.

How to proceed? does it help to provide serialized session contents for both the patched and the unpatched situation?

[tvdijen]

I'm not sure what you mean by session contents and I fail to see the relation between the TypeError and the magic methods.. Are you running v4.6.0 of this library and can you give us a full stack trace?

[tommy2d]

The type errors are caused by $element never being set (as part of the deserialization / serialization routine). Yes, I am using tag v4.6.0 (46d93daa9a96a1d896fe35cb75897a91e1795442). This is the stack trace:

==> /var/log/nginx/ssl-brightfish-sharedservices-tvs-proxy.error.log <== 2022/05/23 19:13:32 [error] 13220#13220: *49603 FastCGI sent in stderr: "PHP message: PHP Fatal error: Uncaught TypeError: SAML2\XML\saml\AttributeValue::getElement(): Return value must be of type DOMElement, null returned in /mnt/sharedservices-code/tvs/vendor/simplesamlphp/saml2/src/SAML2/XML/saml/AttributeValue.php:72 Stack trace: #0 /mnt/sharedservices-code/tvs/vendor/simplesamlphp/simplesamlphp/lib/SimpleSAML/Session.php(241): SAML2\XML\saml\AttributeValue->getElement() #1 [internal function]: SimpleSAML\Session->unserialize() #2 /mnt/sharedservices-code/tvs/vendor/simplesamlphp/simplesamlphp/lib/SimpleSAML/Store/SQL.php(337): unserialize() #3 /mnt/sharedservices-code/tvs/vendor/simplesamlphp/simplesamlphp/lib/SimpleSAML/SessionHandlerStore.php(55): SimpleSAML\Store\SQL->get() #4 /mnt/sharedservices-code/tvs/vendor/simplesamlphp/simplesamlphp/lib/SimpleSAML/Session.php(348): SimpleSAML\SessionHandlerStore->loadSession() #5 /mnt/sharedservices-code/tvs/vendor/simplesamlphp/simplesamlphp/lib/SimpleSAML/Session.php(263): SimpleSAML\Session::getSession()

[tommy2d]

The problem is 100% reproducible for me when authenticating against ToegangsVerleningsService(TVS), and it seems to happen as part of the Unserialization of ActingSubjectID.

[tvdijen]

My best guess by looking at this is that it breaks when AtributeValue contains an XML-structure instead of SimpleContent. The way they changed serialization in PHP between 7.4 and 8.1 is absolutely horrible and I'm not sure if we can fix this in a way we can support all those versions at the same time.

[tvdijen]

The scenario's are covered by a unit test though: https://github.com/simplesamlphp/saml2/blob/release-4.x/tests/SAML2/XML/saml/AttributeValueTest.php#L94-L118

[tommy2d]

I can confirm that the test case works for me.

[tvdijen]

I think I understand what's going on here.. What happens if you set the element-property to public instead of the current private?

[tommy2d]

It doesn't change a thing I'm afraid. Same error, even after cleaning the sqlite db.

[tvdijen]

I don't see what a database has to do with serializing xml, but unfortunately that leaves me with no clues again.. Can you get us an actual AuthnResponse that triggers this?

@thijskh You have any clue here?

[tommy2d]

The serialized attributeValue is stored in sqlite (SimpleSAMLphp_kvstore) right?

none-working (stock) version:

a:1:{i:0;O:29:"SAML2\XML\saml\AttributeValue":1:{i:0;s:2716:"<saml:AttributeValue xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xenc="http://www.w3.org/2001/04/xmlenc#" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"><saml:EncryptedID xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion"><xenc:EncryptedData xmlns:xenc="http://www.w3.org/2001/04/xmlenc#" Id="_df46efefd4d8e83b9f352be544afb3e1-1" Type="http://www.w3.org/2001/04/xmlenc#Element"><xenc:EncryptionMethod Algorithm="http://www.w3.org/2001/04/xmlenc#aes256-cbc"/><ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:RetrievalMethod Type="http://www.w3.org/2001/04/xmlenc#EncryptedKey" URI="#_9e23b6bd3fb43d820553fc88230c5901-1"/></ds:KeyInfo><xenc:CipherData><xenc:CipherValue>/ser96syVNo+mPtHCvYg2oqs7cPgGzczehq8+DQB08gy19RjCEat6YCRSDrilpcRw+lgqyEcr1zJKeU0i84tlGrUdfJ5uWND5H9+FQclLWD36BBtJ4GfsyzSADFfKcu4R6MubZpTxAdDApTopAt8+ZAy9PV9QVWvDZntfzTlise2v9pnTs1xWHmBAO1AmsIoZyFwaoZQUgJrsp2gyuXRTbM4dL96naxj3T0vpPsCUHbNUx1Vw7JFuEZn/JW98gab429w4zM+OY6rPdwwSfAyLq+zGFbxOd33ZihlQHSGLQU=</xenc:CipherValue></xenc:CipherData></xenc:EncryptedData><xenc:EncryptedKey xmlns:xenc="http://www.w3.org/2001/04/xmlenc#" Id="_9e23b6bd3fb43d820553fc88230c5901-1" Recipient="urn:nl-eid-gdi:1.0:DV:00000009900006840000:entities:9780"><xenc:EncryptionMethod Algorithm="http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p"><ds:DigestMethod xmlns:ds="http://www.w3.org/2000/09/xmldsig#" Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/></xenc:EncryptionMethod><ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:KeyName>_b420654655d491b49555c698f80efb7bda3ac6ef</ds:KeyName></ds:KeyInfo><xenc:CipherData><xenc:CipherValue>xcF7/aS3RW0yGEiG1mfDSAmqhwLWpWC8msGp2vlaHsXHpBrDbQb4NYE7AQ/t9cRbDDvmsZmwIZe2B7oMUPz1jhHymmQqJ7WTJOBc7V04zbA/cuzgkxa0gSIPoTHkrRhF7bEvDkkTTZn+X+U2QdSFZc+AB8dIvc83hc0hvgtE1hQUlAzgnw3FO0lnQuVTDI57j9mJqyjfHaN3BqusSqmwuOcZsN+cx9fbWsyFznRlUxoK9Sj5Sn0xuc3Qg5IBjDWTB8y8Ab+vLp3DlNjGfEQIpg9mzbKaMf2fN4Dx7GUya7tdOR5XBkLmu58+opsKb73t5OgjBEdo4q+ETW8fe9w4nRhFD6skRSFA5XyIc0iF3YsJkCfxbUt9BefjwuKAbg8T4FBxyao7kQckfl6k5+wcyX6RQ6wK23orrihgNQCfvvSzmsivtArpXwB7kg4M/H1DUeZYLjh3xrVPZ37wrcwc9fnudKUgzkWjTAc8/si6PqL4LWb35Qk3LqwkdX+dy4qYSvawOa9eqjtalAJYFjYbhAb4csQUm10KFnFNTy2SJRXkf6If2k38UfFJok0cN2RTpaSrn1O1U0BMmhLh3K/bdSmWT6lVGgr7OXdhLfADAqu7BORI/gpFMsDhs41ul0i1vv6hBa0ng/1B2BR4qo1yCaerYV0u0+oF0K5nd5xxFI0=</xenc:CipherValue></xenc:CipherData><xenc:ReferenceList><xenc:DataReference URI="#_df46efefd4d8e83b9f352be544afb3e1-1"/></xenc:ReferenceList></xenc:EncryptedKey></saml:EncryptedID></saml:AttributeValue>";}}

v.s. (working, with the serialization/deserilization functiones removed in AttributeValue):

a:1:{i:0;C:29:"SAML2\XML\saml\AttributeValue":2726:{s:2716:"<saml:AttributeValue xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xenc="http://www.w3.org/2001/04/xmlenc#" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"><saml:EncryptedID xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion"><xenc:EncryptedData xmlns:xenc="http://www.w3.org/2001/04/xmlenc#" Id="_4ea05f00adb06c642e0cb52f063e2570-1" Type="http://www.w3.org/2001/04/xmlenc#Element"><xenc:EncryptionMethod Algorithm="http://www.w3.org/2001/04/xmlenc#aes256-cbc"/><ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:RetrievalMethod Type="http://www.w3.org/2001/04/xmlenc#EncryptedKey" URI="#_dc9043a7cbec55c6fcc61f1cf64cf868-1"/></ds:KeyInfo><xenc:CipherData><xenc:CipherValue>vErnRkA0oSmtQGamjZGa9RFN25SUx1UVLsLAOtopt7pyywTD7wu9pyocfD4HqduXCsvaiZpJykz11utZdvtJ0sOdm9oE+lAtNTUnKzGSNoSopGCzwNu5pqwhIEvWEWeilmJayAC2elpRYOnUs/rePxibz0Wbqa7BItLt6ZkKTtMkv0U0PpgGenF1pWzsahRtw6Y5tFq7xFQkG/z0Lz5rJ+IxExYXgB3LN6FBmVcB1ioahk2ovOwbLQ+lNAdqUMhpZx6fgdL2v7g4OYPK0rDgSALU3gU3dvU4hC/Kk9N5Rkw=</xenc:CipherValue></xenc:CipherData></xenc:EncryptedData><xenc:EncryptedKey xmlns:xenc="http://www.w3.org/2001/04/xmlenc#" Id="_dc9043a7cbec55c6fcc61f1cf64cf868-1" Recipient="urn:nl-eid-gdi:1.0:DV:00000009900006840000:entities:9780"><xenc:EncryptionMethod Algorithm="http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p"><ds:DigestMethod xmlns:ds="http://www.w3.org/2000/09/xmldsig#" Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/></xenc:EncryptionMethod><ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:KeyName>_b420654655d491b49555c698f80efb7bda3ac6ef</ds:KeyName></ds:KeyInfo><xenc:CipherData><xenc:CipherValue>d5X+psMTEy9DWDoVotB5sPHdNpofv5BdPUleflhGfjbjGIfWbV9fK+jMkQ4cqwSWwmTGSQ8OO+lYA9IwZasnWygAu3dSSlYd+sd/m2waz83MrTBsTtZUzwy8N18tNMu4xB/tb45XPvis9agg6b2RdpDSS1m5BKK+MMKgX2ZYIqOW6cbXxl73YJHHHpcTi1TsI+tx6DLNWB2ku5wpS3cFB3c0Tws9qG+YBwTzfp1gFhzvBQDNxLWVjPKEWEw5Do1dp8f7jOPWq0sscz6DaTJH3RStwOSLF2vsayGZUgqNXyPHX12dFNGHiTXEWVYcuXsqG8sXJQyUmE5kYqN/D8NT1BIq50L6xIKkQGxNSlyAAkvkV87b1z6X2Fvk6Hvmx+eqauJX/BKknqgNZL/I5xfysDcNG5i0vA/kOVai4LvmdEoUSq1dIWcGsiW5pbM13R+DddkUegGcZctmFLcafBM2A3WztGclLWSJea+hh24YurcRGQVoOaZcYRmGVA0cYRQjfcSb3mvGxl01I9f27Hzv6/pXMWmEetpNhO4Pxy4GIwatJ7clW+s679f6N4+qepzbnxqjMLOmRPilJ0t9+wHYQ9wc2+NN4lssLNNz852rb5Rs9HDTvOyvtV7ew4HdMqGjXeJt545pQzSonuzCVVs67Nuhi/rmOktsav8tP0N8izE=</xenc:CipherValue></xenc:CipherData><xenc:ReferenceList><xenc:DataReference URI="#_4ea05f00adb06c642e0cb52f063e2570-1"/></xenc:ReferenceList></xenc:EncryptedKey></saml:EncryptedID></saml:AttributeValue>";}}

The first difference that I notice is C versus O.

[tvdijen]

Ah, you're using SQLite for session storage? I didn't get that from the issue description.. The C/O difference appears to be the only difference here.. I'll try and investigate some more

[tommy2d]

I'm indeed using sqlite for session storage. But this is only to make it easier to access the serialized data: the issue exists on all session storage providers. Let me know if I can help in any way.

[bartlangelaan]

We have the exact same issue. Re-naming the __serialize and __unserialize function to something random fixed the problem. We're still running PHP 7.4 (will upgrade to 8.1 soon).

If this bug was indeed introduced by #292 then it would be introduced in version 4.5.0.

We will revert to 4.4.1.

---

Could it be that this is specific to EncryptedID attribute values? There is no specific test for that.

[tvdijen]

~~That could be part of the problem, yes.. When I unserialize, the value is that of the CipherValue instead of the entire xml-structure.~~

Update: I was able to narrow it down to the constructor of AttributeValue.. I think for some reason Utils::copyElement is not doing what we want it to do; https://github.com/simplesamlphp/saml2/commit/8a7d72eb28f890e2725f28b1fd4004485b88402a

[tommy2d]

Interesting. I just mirrored the branch onto our gitlab environment so I will be able to test. Keep me posted in case I need to perform additional tests using this branch.

[bartlangelaan]

I tried to adjust the php81_issue branch to create a failing test, as a start point for a fix. But I don't really understand where it is broken.

In this test:

    public function testSerializeEncryptedID() : void
    {
        $element = new \DOMDocument();
        $element->loadXML(<<<ATTRIBUTEVALUE
<saml:EncryptedID xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"><xenc:EncryptedData xmlns:xenc="http://www.w3.org/2001/04/xmlenc#" Id="_4ea05f00adb06c642e0cb52f063e2570-1" Type="http://www.w3.org/2001/04/xmlenc#Element"><xenc:EncryptionMethod Algorithm="http://www.w3.org/2001/04/xmlenc#aes256-cbc"/><ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:RetrievalMethod Type="http://www.w3.org/2001/04/xmlenc#EncryptedKey" URI="#_dc9043a7cbec55c6fcc61f1cf64cf868-1"/></ds:KeyInfo><xenc:CipherData><xenc:CipherValue>vErnRkA0oSmtQGamjZGa9RFN25SUx1UVLsLAOtopt7pyywTD7wu9pyocfD4HqduXCsvaiZpJykz11utZdvtJ0sOdm9oE+lAtNTUnKzGSNoSopGCzwNu5pqwhIEvWEWeilmJayAC2elpRYOnUs/rePxibz0Wbqa7BItLt6ZkKTtMkv0U0PpgGenF1pWzsahRtw6Y5tFq7xFQkG/z0Lz5rJ+IxExYXgB3LN6FBmVcB1ioahk2ovOwbLQ+lNAdqUMhpZx6fgdL2v7g4OYPK0rDgSALU3gU3dvU4hC/Kk9N5Rkw=</xenc:CipherValue></xenc:CipherData></xenc:EncryptedData><xenc:EncryptedKey xmlns:xenc="http://www.w3.org/2001/04/xmlenc#" Id="_dc9043a7cbec55c6fcc61f1cf64cf868-1" Recipient="urn:nl-eid-gdi:1.0:DV:00000009900006840000:entities:9780"><xenc:EncryptionMethod Algorithm="http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p"><ds:DigestMethod xmlns:ds="http://www.w3.org/2000/09/xmldsig#" Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/></xenc:EncryptionMethod><ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:KeyName>_b420654655d491b49555c698f80efb7bda3ac6ef</ds:KeyName></ds:KeyInfo><xenc:CipherData><xenc:CipherValue>d5X+psMTEy9DWDoVotB5sPHdNpofv5BdPUleflhGfjbjGIfWbV9fK+jMkQ4cqwSWwmTGSQ8OO+lYA9IwZasnWygAu3dSSlYd+sd/m2waz83MrTBsTtZUzwy8N18tNMu4xB/tb45XPvis9agg6b2RdpDSS1m5BKK+MMKgX2ZYIqOW6cbXxl73YJHHHpcTi1TsI+tx6DLNWB2ku5wpS3cFB3c0Tws9qG+YBwTzfp1gFhzvBQDNxLWVjPKEWEw5Do1dp8f7jOPWq0sscz6DaTJH3RStwOSLF2vsayGZUgqNXyPHX12dFNGHiTXEWVYcuXsqG8sXJQyUmE5kYqN/D8NT1BIq50L6xIKkQGxNSlyAAkvkV87b1z6X2Fvk6Hvmx+eqauJX/BKknqgNZL/I5xfysDcNG5i0vA/kOVai4LvmdEoUSq1dIWcGsiW5pbM13R+DddkUegGcZctmFLcafBM2A3WztGclLWSJea+hh24YurcRGQVoOaZcYRmGVA0cYRQjfcSb3mvGxl01I9f27Hzv6/pXMWmEetpNhO4Pxy4GIwatJ7clW+s679f6N4+qepzbnxqjMLOmRPilJ0t9+wHYQ9wc2+NN4lssLNNz852rb5Rs9HDTvOyvtV7ew4HdMqGjXeJt545pQzSonuzCVVs67Nuhi/rmOktsav8tP0N8izE=</xenc:CipherValue></xenc:CipherData><xenc:ReferenceList><xenc:DataReference URI="#_4ea05f00adb06c642e0cb52f063e2570-1"/></xenc:ReferenceList></xenc:EncryptedKey></saml:EncryptedID>
ATTRIBUTEVALUE
        );

        $av3 = new AttributeValue($element->documentElement);
        $this->assertEquals(
            $av3->getElement()->ownerDocument->saveXML(),
            unserialize(serialize($av3))->getElement()->ownerDocument->saveXML()
        );
    }

The test passes, because the ->saveXML() returns an empty xml string even without serialize. So it doesn't seem related to the serialization, right?

---

EDIT: I did find a way to test this, see PR: #299

Would be great to see this merged, so we can safely update to newer versions again :-)

[tvdijen]

Was solved in v4.6.5