Multiple signing certificates not supported

[Ronnie-J]

Hi.

This issue might be for the core part of lightSAML and not the SpBundle.

We integrate against an ADFS that uses rolling switchover when their certificates expire. I would expect that I were able to use multiple certificates in the IDP XML like

        <md:KeyDescriptor use="signing">
            <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
                <ds:X509Data>
                    <ds:X509Certificate>{certificate1}</ds:X509Certificate>
                </ds:X509Data>
            </ds:KeyInfo>
        </md:KeyDescriptor>
        <md:KeyDescriptor use="encryption">
            <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
                <ds:X509Data>
                    <ds:X509Certificate>{certificate2}</ds:X509Certificate>
                </ds:X509Data>
            </ds:KeyInfo>
        </md:KeyDescriptor>
        <md:KeyDescriptor use="signing">
            <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
                <ds:X509Data>
                    <ds:X509Certificate>{certificate2}</ds:X509Certificate>
                </ds:X509Data>
            </ds:KeyInfo>
        </md:KeyDescriptor>

But I get the following error when verifying the response.

Call to a member function removeChild() on null

in vendor/robrichards/xmlseclibs/src/XMLSecurityDSig.php at line 489

        {
            $docElem = $this->sigNode->ownerDocument->documentElement;
            if (! $docElem->isSameNode($this->sigNode)) {
                $this->sigNode->parentNode->removeChild($this->sigNode);
            }

As I read the description on saml2 http://docs.oasis-open.org/security/saml/v2.0/sstc-saml-approved-errata-2.0.html multiple KeyDescriptor should be valid.

The inclusion of multiple elements with the same use attribute (or no such attribute) indicates that any of the included keys may be used by the containing role or affiliation. A relying party SHOULD allow for the use of any of the included keys. When possible the signing or encrypting party SHOULD indicate as specifically as possible which key it used to enable more efficient processing.

Thanks.

[sverraest]

We're getting the same error when integrating with Azure ADFS. Were you able to find a solution? For now the only possible way I see is to fork this repo and change the dependency to a modified version of the robrichards/xmlseclibs library which includes a null check.

[Ronnie-J]

Hi @sverraest - sorry for the late response. No I haven't gotten around to play with this. For starters I was hoping to get some initial feedback from the contributors. Maybe @tmilos would have some comments?

But it might not be so easy when it seems to be a problem with a vendor library.

[tmilos]

@Ronnie-J multiple certificates are supported and implemented in validateMulti() method of AbstractSignatureReader. Also, MetadataCredentialStore extracts all certificates from metadata.

The error you are getting is from robrichards/xmlseclibs dependency lib, and is already reported in https://github.com/lightSAML/SpBundle/issues/15 and https://github.com/robrichards/xmlseclibs/issues/108 but it was not documented how to reproduce it. I'm not sure what's causing it, thus also not sure how it should properly be fixed.

I would greatly appreciate if you could document how to reproduce that error, so finally we can investigate it and properly fix it. We (me or @robrichards) would need original unmodified encrypted xml message, idp metadata, and your key pair. Original message you can get from the symfony log file starting with app.INFO: Received message. After decryption, it will be logged with app.INFO: Assertion decrypted and you could include that decrypted assertion also, but so important as original message and your key pair so we can decrypt it also.

Thank you

[Ronnie-J]

Hi @tmilos

Thanks for your response. I didn't think the two issues were on the same subject. I hope I will get around to debug it further soon and provide you with some more data.

I cant provide you with our private keys though, and getting a new setup with new keys might take some time. Would it not be surfactant with the unencrypted SAML communication along with my configuration? When I am using one certificate everything is fine, even if I switch them around.

Best regards Ronnie

[tmilos]

As indicated in #15 issue was pinned down to robrichards/xmlseclibs and waiting https://github.com/robrichards/xmlseclibs/pull/113 fix to be merged... for a while already. LightSAML have tried to reduce number of credential candidates and avoid multiple signature validations in lightSAML/lightSAML#60.