Fix XML Signature Removal Bug

[SandersJ16]

Currently we are removing the XML signature from a Login Response prior to validating the signature in the library. The usage of xmldom's removeChild is incorrect. removeChild must be called on the direct parent of the node to be removed, in the code right now it is being called on the root document which is not always the parent of the signature doc (for example, an XML response with a Prolog). This throws an error in newer versions of xmldom preventing upgrade of this module and can produce unexpected invalid XML for some XMLs (sample reproduced in tests). See xmldom/xmldom#135 for more details on this issue and why this now throws an error on newer version of XML.

This PR fixes that issue by ensuring it is always calling removeChild on the direct parent of the signature node. This PR also fixes an issue breaking existing tests with template replacement where false values provided to a template would produce an empty string instead of the value false.

---

As mentioned in some older issues and other PRs, the removal of the XML signature is not necessary for xml-crypto for the library to validate. Another solution would be to remove this entirely (This currently open PR: #515 and Issue: #514 are about this). I do see in the original PR mentioned removing this helps with debugging but is also necessary when there are multiple signatures d382bbc#r31013727

here is a bug fix in v2.4.0. The order the two signature proceeds is sign assertion -> sign entire message. Therefore we need to verify the message signature first, then we verify the assertion signature. In this case, no matter it removes the signature node or not, it doesn’t affect the outcome. Removal of signature node here makes more sense for debugging, since the document is reverted to the state when it is signed. For example, when you debug and verify the message signature, you will see there is an assertion signature, that's the same state when you sign the message signature.

However, we accidentally verify assertion signature first in the rc version (and before), once the assertion signature is verified, then I remove the assertion signature, it will affect the next verification for message signature, because the assertion signature exists when you sign entire message signature, that's why it works after you comment out the line but it's not correct.

For more detail about the fix, please take a look in #219

I didn't come across any test cases for that specific issue so I was not able to replicate the bug that not removing the signature introduces so think to minimize change updating this to correctly remove the signature element is the safer option.

[SandersJ16]

This change makes it so that Login Responses produce for this SP will add their signature as the first element of the Response tag. This and the presence of and XML Prolog (making the Response tag not the owner doc) causes the bug in question where the XML returned by removeChild is incorrect.

ex.
Before replaceChild

<?xml version="1.0" encoding="utf-8"?><samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="_8e8dc5f69a98cc4c1ff3427e5ce34606fd672f91e6" Version="2.0" IssueInstant="2024-10-02T23:03:36.499Z" Destination="" InResponseTo="_4606cc1f427fa981e6ffd653ee8d6972fc5ce398c4"><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:SignedInfo><ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/><ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/><ds:Reference URI="#_8e8dc5f69a98cc4c1ff3427e5ce34606fd672f91e6"><ds:Transforms><ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/><ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></ds:Transforms><ds:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/><ds:DigestValue>V4e4IN/fpSq2B5ygB1wE94TS5tS+U+giS4OWEH/NCak=</ds:DigestValue></ds:Reference></ds:SignedInfo><ds:SignatureValue>n1sqIxMB3vcEYrUt8mlFJoF8M4IjTmLwXiZEqkn1yFPaxD87gLG7tfcRGeL4DcsUp/xjt0qIK4004nLGBfp9pIYsM+NMyPkr8SnoXObO75TuFZ7e0owe0WAs0QiAQ7gbNbzBSd/q5YHckYJWxDEJXchjN63f0phdEgB7wMatkBpqNFsKoT+VONoAVauQp7on7a3s42JgcRzvWIlROmMMiPSasSp2My1UA0Gx9kdUDQWeLVlmgTl2/W3EcE4K7VIe5L2EQcnBRmUE1Q5kngnFOmAkIok76jcqJOmzmheA7FpgrMe/kzqikyQy9ZsJQxS5SCFQNhCQLdenNHSpY/GCog==</ds:SignatureValue><ds:KeyInfo><ds:X509Data><ds:X509Certificate>MIIDlzCCAn+gAwIBAgIJAO1ymQc33+bWMA0GCSqGSIb3DQEBCwUAMGIxCzAJBgNVBAYTAkhLMRMwEQYDVQQIDApTb21lLVN0YXRlMRowGAYDVQQKDBFJZGVudGl0eSBQcm92aWRlcjEUMBIGA1UECwwLRGV2ZWxvcG1lbnQxDDAKBgNVBAMMA0lEUDAeFw0xNTA3MDUxODAyMjdaFw0xODA3MDQxODAyMjdaMGIxCzAJBgNVBAYTAkhLMRMwEQYDVQQIDApTb21lLVN0YXRlMRowGAYDVQQKDBFJZGVudGl0eSBQcm92aWRlcjEUMBIGA1UECwwLRGV2ZWxvcG1lbnQxDDAKBgNVBAMMA0lEUDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAODZsWhCe+yG0PalQPTUoD7yko5MTWMCRxJ8hSm2k7mG3Eg/Y2v0EBdCmTw7iDCevRqUmbmFnq7MROyV4eriJzh0KabAdZf7/k6koghst3ZUtWOwzshyxkBtWDwGmBpQGTGsKxJ8M1js3aSqNRXBT4OBWM9w2Glt1+8ty30RhYv3pSF+/HHLH7Ac+vLSIAlokaFW34RWTcJ/8rADuRWlXih4GfnIu0W/ncm5nTSaJiRAvr3dGDRO/khiXoJdbbOj7dHPULxVGbH9IbPK76TCwLbF7ikIMsPovVbTrpyL6vsbVUKeEl/5GKppTwp9DLAOeoSYpCYkkDkYKu9TRQjF02MCAwEAAaNQME4wHQYDVR0OBBYEFP2ut2AQdy6D1dwdwK740IHmbh38MB8GA1UdIwQYMBaAFP2ut2AQdy6D1dwdwK740IHmbh38MAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBANMZUoPNmHzgja2PYkbvBYMHmpvUkVoiuvQ9cJPlqGTB2CRfG68BNNs/Clz8P7cIrAdkhCUwi1rSBhDuslGFNrSaIpv6B10FpBuKwef3G7YrPWFNEN6khY7aHNWSTHqKgs1DrGef2B9hvkrnHWbQVSVXrBFKe1wTCqcgGcOpYoSK7L8C6iX6uIA/uZYnVQ4NgBrizJ0azkjdegz3hwO/gt4malEURy8D85/AAVt6PAzhpb9VJUGxSXr/EfntVUEz3L2gUFWWk1CnZFyz0rIOEt/zPmeAY8BLyd/Tjxm4Y+gwNazKq5y9AJS+m858b/nM4QdCnUE4yyoWAJDUHiAmvFA=</ds:X509Certificate></ds:X509Data></ds:KeyInfo></ds:Signature><saml:Issuer>https://idp.example.com/metadata</saml:Issuer><samlp:Status><samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/></samlp:Status><saml:Assertion ID="_519cc29f-aba8-4517-aa4b-6ce2da634a47" Version="2.0" IssueInstant="2024-10-02T23:03:36.499Z" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"><saml:Issuer>https://idp.example.com/metadata</saml:Issuer><saml:Subject><saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">[email protected]</saml:NameID><saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><saml:SubjectConfirmationData NotOnOrAfter="2024-10-02T23:08:36.499Z" Recipient="https://sp.example.org/metadata" InResponseTo="_4606cc1f427fa981e6ffd653ee8d6972fc5ce398c4"/></saml:SubjectConfirmation></saml:Subject><saml:Conditions NotBefore="2024-10-02T23:03:36.499Z" NotOnOrAfter="2024-10-02T23:08:36.499Z"><saml:AudienceRestriction><saml:Audience>https://sp.example.org/metadata</saml:Audience></saml:AudienceRestriction></saml:Conditions><saml:AttributeStatement><saml:Attribute Name="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">[email protected]</saml:AttributeValue></saml:Attribute><saml:Attribute Name="name" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">mynameinsp</saml:AttributeValue></saml:Attribute></saml:AttributeStatement></saml:Assertion></samlp:Response>

After replaceChild (Broken XML)

<saml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">https://idp.example.com/metadata</saml:Issuer><samlp:Status xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"><samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/></samlp:Status><saml:Assertion ID="_519cc29f-aba8-4517-aa4b-6ce2da634a47" Version="2.0" IssueInstant="2024-10-02T23:03:36.499Z" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"><saml:Issuer>https://idp.example.com/metadata</saml:Issuer><saml:Subject><saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">[email protected]</saml:NameID><saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><saml:SubjectConfirmationData NotOnOrAfter="2024-10-02T23:08:36.499Z" Recipient="https://sp.example.org/metadata" InResponseTo="_4606cc1f427fa981e6ffd653ee8d6972fc5ce398c4"/></saml:SubjectConfirmation></saml:Subject><saml:Conditions NotBefore="2024-10-02T23:03:36.499Z" NotOnOrAfter="2024-10-02T23:08:36.499Z"><saml:AudienceRestriction><saml:Audience>https://sp.example.org/metadata</saml:Audience></saml:AudienceRestriction></saml:Conditions><saml:AttributeStatement><saml:Attribute Name="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">[email protected]</saml:AttributeValue></saml:Attribute><saml:Attribute Name="name" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic"><saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">mynameinsp</saml:AttributeValue></saml:Attribute></saml:AttributeStatement></saml:Assertion>

[SandersJ16]

If value is null or undefined leave as is (this will produce an empty string), otherwise convert to string (fixes issue with false values). This fixes failure happening for tests:

• flow > create login request with redirect binding using default template and parse it
• flow > create login request with post simpleSign binding using default template and parse it
• flow > create login request with post binding using default template and parse it

t.is(extract.nameIDPolicy.allowCreate, 'false');

  Difference:

  - ''
  + 'false'