Decrypted Assertion modified namespace results as assertion Signature not valid.

[agrisvv]

Document
Incoming saml Response are signed, Assertion also are signed and encrypted.
PHP 8.2.16

Problem
Assertion signature not valid after it are modified with this line:

 $decrypted->setAttributeNS('http://www.w3.org/2000/xmlns/', $ns, Constants::NS_SAML);

Document parts examples - response and decrypted assertion:

<samlp:Response ID="_07e4c1c6-xxxxxxxxxxxxxxxxxxxxxxxx"
                Version="2.0"
                IssueInstant="2024-03-15T09:25:10.000Z"
                Destination="https://xxxxxxxxxxxxxxxxxxxxxx"
                Consent="urn:oasis:names:tc:SAML:2.0:consent:unspecified"
                InResponseTo="ONELOGIN_719fa4a0xxxxxxxxxxxxxxxxxxxxxx"
                xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
                >

<saml:Assertion ID="_45179449-a9XXXXXXXXXXXXXXXXXX" IssueInstant="2024-03-15T14:22:42.000Z"
    Version="2.0" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">

Main elements:

<Signature xmlns="http://www.w3.org/2000/09/xmldsig#">
<EncryptedAssertion xmlns="urn:oasis:names:tc:SAML:2.0:assertion">

Auth fails at:

if ($hasSignedAssertion && !Utils::validateSign($documentToCheckAssertion, $cert, $fingerprint, $fingerprintalg, Utils::ASSERTION_SIGNATURE_XPATH, $multiCerts)) {
   throw new ValidationError(
     "Signature validation failed. SAML Response rejected",
     ValidationError::INVALID_SIGNATURE
     );
}

Opinion:
So i think what it is wrong to modify signed assertion. Maybe responses are also are not 100% perfect, but o can't modify them.
Maybe php bug in function ->hasAttributeNS dose not detect existing xmlns..

&& !$container->hasAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:saml')

In result next function write one more $qualifiedName 'xmlns'

Solutions:

1. Remove assertion namespace modification.
2. Add option to check in plain text for value.
3. Try to find why hasAttributeNS not work.

[maijs]

This issue has been around for quite a while (see SAML-Toolkits/wordpress-saml#136), and it's certainly a bug.

[pitbulk]

@maijs , have you tested that instead:

{
    if (strpos($encryptedAssertion->tagName, 'saml2:') !== false) {
        $ns = 'xmlns:saml2';
    } else if (strpos($encryptedAssertion->tagName, 'saml:') !== false) {
        $ns = 'xmlns:saml';
    } else {
        $ns = 'xmlns';
    }
    $decrypted->setAttributeNS('http://www.w3.org/2000/xmlns/', $ns, Constants::NS_SAML);
}

having:

{
    if (strpos($encryptedAssertion->tagName, 'saml2:') !== false) {
        $decrypted->setAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:saml2', Constants::NS_SAML);
    } else if (strpos($encryptedAssertion->tagName, 'saml:') !== false) {
        $decrypted->setAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns:saml', Constants::NS_SAML);
    }
}

solves your issue?

Can you generate self-signed cert and private key, use them on the Idp to generate a fresh new Signed Encrypted Assertion and share untouched encodebased64 SAMLResponse with the self-signed cert and private key to allow me explore?

[pitbulk]

@agrisvv ,@maijs were you able to check my previous comment?

[maijs]

@pitbulk Yes, the code you provided does solve the issue, because it doesn't alter the object, at least in my case where none of the two conditions return true.

[pitbulk]

@maijs thanks for confirming.

The IdPs that I usually use for testing php-saml (Okta, OneLogin, Azure, simpleSAMLphp) generate encrypted assertions
that does not have this problem.

Would you mind to generate and share with me a base64 encoded SAMLResponse with encrypted assertion payload and provide to me self-signed private key and public cert used for this specific purpose (so its ok to make them public), or you even could reuse the sp.key and sp.crt as the one to be used at the IdP, so I could properly apply the fix and add test coverage so no regressions gonna be introduced in the future?