From b1263c7cd66416b33e81ed63bd38e86a8b6ac5b6 Mon Sep 17 00:00:00 2001 From: Nicolas Morel Date: Thu, 17 Nov 2022 19:22:42 +0100 Subject: [PATCH] fix: correct handling of XML entities in signature attributes (#221) Fixes #200. --- src/xml.ts | 3 +- ...signed.assertion-unsigned-13-signature.xml | 68 +++++++++++++++++++ ...signed.assertion-unsigned-xd-signature.xml | 68 +++++++++++++++++++ test/test-signatures.spec.ts | 30 +++++++- 4 files changed, 165 insertions(+), 4 deletions(-) create mode 100644 test/static/signatures/valid/response.root-signed.assertion-unsigned-13-signature.xml create mode 100644 test/static/signatures/valid/response.root-signed.assertion-unsigned-xd-signature.xml diff --git a/src/xml.ts b/src/xml.ts index cf1619f1..8d9e3bc2 100644 --- a/src/xml.ts +++ b/src/xml.ts @@ -129,8 +129,7 @@ export const validateXmlSignatureForCert = ( getKeyInfo: () => "", getKey: () => Buffer.from(certPem), }; - const signatureStr = normalizeNewlines(signature.toString()); - sig.loadSignature(signatureStr); + sig.loadSignature(signature); // We expect each signature to contain exactly one reference to the top level of the xml we // are validating, so if we see anything else, reject. if (sig.references.length != 1) return false; diff --git a/test/static/signatures/valid/response.root-signed.assertion-unsigned-13-signature.xml b/test/static/signatures/valid/response.root-signed.assertion-unsigned-13-signature.xml new file mode 100644 index 00000000..23751b5e --- /dev/null +++ b/test/static/signatures/valid/response.root-signed.assertion-unsigned-13-signature.xml @@ -0,0 +1,68 @@ + + + https://evil-corp.com + + + uAP/VBhhaK2+Bn1lYjHB5tSHTcE=Q2F/63pvENmI+eURRIRmVI1fNpW+CxDan6YmGo2G5il+XOx72sBHS+FspoWDyezYUfO/Wfi+tBupK +/9eHOeg60uPUkQFwkdPmUsH2hJAPVCatQReSUJDhHCO6a2rrQixecPDBhzJjstCpibgvgNzevVVu +9h3eMH/BNzdwO5EbnVTcb2JQj2F18MAh1LlVMBWWDaZmIQTk7npMY/NVEajM1fbwXyrPdNRU8poK +bjVoM7IL9s0TesQIeyQ01QOQQQ3kHZWnoqlWE6VbbTqHwuidfkqaQLLvF9sDneqXKBa7y2YEJXVm4 +GMzNoL/JOdSuNnU3rF0ET8UDSleV953/lbUA== +MIIDtTCCAp2gAwIBAgIJAKg4VeVcIDz1MA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNVBAYTAlVTMRMwE +QYDVQQIEwpTb21lLVN0YXRlMSEwHwYDVQQKExhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMT +UwODEzMDE1NDIwWhcNMTUwOTEyMDE1NDIwWjBFMQswCQYDVQQGEwJVUzETMBEGA1UECBMKU29tZS1 +TdGF0ZTEhMB8GA1UEChMYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIIBIjANBgkqhkiG9w0BAQEF +AAOCAQ8AMIIBCgKCAQEAxG3ouM7U+fXbJt69X1H6d4UNg/uRr06pFuU9RkfIwNC+yaXyptqB3ynXK +sL7BFt4DCd0fflRvJAx3feJIDp16wN9GDVHcufWMYPhh2j5HcTW/j9JoIJzGhJyvO00YKBt+hHy83 +iN1SdChKv5y0iSyiPP5GnqFw+ayyHoM6hSO0PqBou1Xb0ZSIE+DHosBnvVna5w2AiPY4xrJl9yZHZ +4Q7DfMiYTgstjETio4bX+6oLiBnYktn7DjdEslqhffVme4PuBxNojI+uCeg/sn4QVLd/iogMJfDWN +uLD8326Mi/FE9cCRvFlvAiMSaebMI3zPaySsxTK7Zgj5TpEbmbHI9wIDAQABo4GnMIGkMB0GA1UdD +gQWBBSVGgvoW4MhMuzBGce29PY8vSzHFzB1BgNVHSMEbjBsgBSVGgvoW4MhMuzBGce29PY8vSzHF6 +FJpEcwRTELMAkGA1UEBhMCVVMxEzARBgNVBAgTClNvbWUtU3RhdGUxITAfBgNVBAoTGEludGVybmV +0IFdpZGdpdHMgUHR5IEx0ZIIJAKg4VeVcIDz1MAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQAD +ggEBAJu1rqs+anD74dbdwgd3CnqnQsQDJiEXmBhG2leaGt3ve9b/9gKaJg2pyb2NyppDe1uLqh6nN +XDuzg1oNZrPz5pJL/eCXPl7FhxhMUi04TtLf8LeNTCIWYZiFuO4pmhohHcv8kRvYR1+6SkLTC8j/T +Zerm7qvesSiTQFNapa1eNdVQ8nFwVkEtWl+JzKEM1BlRcn42sjJkijeFp7DpI7pU+PnYeiaXpRv5p +Jo8ogM1iFxN+SnfEs0EuQ7fhKIG9aHKi7bKZ7L6SyX7MDIGLeulEU6lf5D9BfXNmcMambiS0pXhL2 +QXajt96UBq8FT2KNXY8XNtR4y6MyyCzhaiZZcc8= + + + + + https://evil-corp.com + + vincent.vega@evil-corp.com + + + + + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + + + + + vincent.vega@evil-corp.com + + + + Vincent + + + + VEGA + + + + 123 Main St. +Suite 11 + + + + + diff --git a/test/static/signatures/valid/response.root-signed.assertion-unsigned-xd-signature.xml b/test/static/signatures/valid/response.root-signed.assertion-unsigned-xd-signature.xml new file mode 100644 index 00000000..d48b61e4 --- /dev/null +++ b/test/static/signatures/valid/response.root-signed.assertion-unsigned-xd-signature.xml @@ -0,0 +1,68 @@ + + + https://evil-corp.com + + + uAP/VBhhaK2+Bn1lYjHB5tSHTcE=Q2F/63pvENmI+eURRIRmVI1fNpW+CxDan6YmGo2G5il+XOx72sBHS+FspoWDyezYUfO/Wfi+tBupK + /9eHOeg60uPUkQFwkdPmUsH2hJAPVCatQReSUJDhHCO6a2rrQixecPDBhzJjstCpibgvgNzevVVu + 9h3eMH/BNzdwO5EbnVTcb2JQj2F18MAh1LlVMBWWDaZmIQTk7npMY/NVEajM1fbwXyrPdNRU8poK + bjVoM7IL9s0TesQIeyQ01QOQQQ3kHZWnoqlWE6VbbTqHwuidfkqaQLLvF9sDneqXKBa7y2YEJXVm4 + GMzNoL/JOdSuNnU3rF0ET8UDSleV953/lbUA== +MIIDtTCCAp2gAwIBAgIJAKg4VeVcIDz1MA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNVBAYTAlVTMRMwE + QYDVQQIEwpTb21lLVN0YXRlMSEwHwYDVQQKExhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMT + UwODEzMDE1NDIwWhcNMTUwOTEyMDE1NDIwWjBFMQswCQYDVQQGEwJVUzETMBEGA1UECBMKU29tZS1 + TdGF0ZTEhMB8GA1UEChMYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIIBIjANBgkqhkiG9w0BAQEF + AAOCAQ8AMIIBCgKCAQEAxG3ouM7U+fXbJt69X1H6d4UNg/uRr06pFuU9RkfIwNC+yaXyptqB3ynXK + sL7BFt4DCd0fflRvJAx3feJIDp16wN9GDVHcufWMYPhh2j5HcTW/j9JoIJzGhJyvO00YKBt+hHy83 + iN1SdChKv5y0iSyiPP5GnqFw+ayyHoM6hSO0PqBou1Xb0ZSIE+DHosBnvVna5w2AiPY4xrJl9yZHZ + 4Q7DfMiYTgstjETio4bX+6oLiBnYktn7DjdEslqhffVme4PuBxNojI+uCeg/sn4QVLd/iogMJfDWN + uLD8326Mi/FE9cCRvFlvAiMSaebMI3zPaySsxTK7Zgj5TpEbmbHI9wIDAQABo4GnMIGkMB0GA1UdD + gQWBBSVGgvoW4MhMuzBGce29PY8vSzHFzB1BgNVHSMEbjBsgBSVGgvoW4MhMuzBGce29PY8vSzHF6 + FJpEcwRTELMAkGA1UEBhMCVVMxEzARBgNVBAgTClNvbWUtU3RhdGUxITAfBgNVBAoTGEludGVybmV + 0IFdpZGdpdHMgUHR5IEx0ZIIJAKg4VeVcIDz1MAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQAD + ggEBAJu1rqs+anD74dbdwgd3CnqnQsQDJiEXmBhG2leaGt3ve9b/9gKaJg2pyb2NyppDe1uLqh6nN + XDuzg1oNZrPz5pJL/eCXPl7FhxhMUi04TtLf8LeNTCIWYZiFuO4pmhohHcv8kRvYR1+6SkLTC8j/T + Zerm7qvesSiTQFNapa1eNdVQ8nFwVkEtWl+JzKEM1BlRcn42sjJkijeFp7DpI7pU+PnYeiaXpRv5p + Jo8ogM1iFxN+SnfEs0EuQ7fhKIG9aHKi7bKZ7L6SyX7MDIGLeulEU6lf5D9BfXNmcMambiS0pXhL2 + QXajt96UBq8FT2KNXY8XNtR4y6MyyCzhaiZZcc8= + + + + + https://evil-corp.com + + vincent.vega@evil-corp.com + + + + + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + + + + + vincent.vega@evil-corp.com + + + + Vincent + + + + VEGA + + + + 123 Main St. +Suite 11 + + + + + diff --git a/test/test-signatures.spec.ts b/test/test-signatures.spec.ts index 73386f32..7d6282b4 100644 --- a/test/test-signatures.spec.ts +++ b/test/test-signatures.spec.ts @@ -429,17 +429,43 @@ describe("Signatures", function () { }); it( - "Attribute with with ", + "Attribute with ", testOneResponse("/valid/response.root-signed.assertion-unsigned-13.xml", false, 1, { wantAssertionsSigned: false, }) ); it( - "Attribute with with ", + "Attribute with ", testOneResponse("/valid/response.root-signed.assertion-unsigned-xd.xml", false, 1, { wantAssertionsSigned: false, }) ); }); + + describe("Signature on saml:Response with XML-encoded carriage returns in signature value and certificate", () => { + let fakeClock: sinon.SinonFakeTimers; + + beforeEach(function () { + fakeClock = sinon.useFakeTimers(Date.parse("2020-09-25T16:59:00Z")); + }); + + afterEach(function () { + fakeClock.restore(); + }); + + it( + "Signature attributes with ", + testOneResponse("/valid/response.root-signed.assertion-unsigned-13-signature.xml", false, 1, { + wantAssertionsSigned: false, + }) + ); + + it( + "Signature attributes with ", + testOneResponse("/valid/response.root-signed.assertion-unsigned-xd-signature.xml", false, 1, { + wantAssertionsSigned: false, + }) + ); + }); });