russellhaering / gosaml2

Pure Go implementation of SAML 2.0
Apache License 2.0
324 stars 120 forks source link

Possible Vulnerability: Auth Request sends the service providers validation certificate #152

Closed cyrusfurtado closed 10 months ago

cyrusfurtado commented 10 months ago

The signature in the auth request in signed using the service providers private key and the validation is to be done with public certificate which should be exclusively with the Idp but this certificate is sent in the Auth Request.

Is this behavior correct as we are in the understanding that the public certificate sent in the request should be that of the Identity Provider for Identification purposes and not the one of the Service Provider which would be used for signature validation as all the required information for deciphering the auth request would then be in it.

This is the actual validation certificate being sent in the request

auth-req-cert

Sample xml here

<samlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
                    AssertionConsumerServiceURL="https://localhost:10443/v1/samlacsendpoint"
                    Destination="https://auth.pingone.asia/793adfb9-e7c9-4e80-a1a2-335f27066ffe/saml20/idp/sso"
                    ID="_c46eb447-c2e0-4dcf-b2a3-ba54961e6bcf"
                    IssueInstant="2023-11-16T13:43:35Z"
                    ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
                    Version="2.0"
                    >
    <saml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">http://example.com/saml/acs/example</saml:Issuer>
    <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="#_c46eb447-c2e0-4dcf-b2a3-ba54961e6bcf">
                <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>hUi2n9LKrdfhgOPawzk7SMMxCC3Pm1Ayusup34CQ5pw=</ds:DigestValue>
            </ds:Reference>
        </ds:SignedInfo>
        <ds:SignatureValue>eaV6UNp+M6e0yifSPTqE3x5RAt3Mp14DuEqatLzkU2kA9tfjEJJkZsFQkjiSr7G+dIfqg5yzbT2fVIMToLEqrFJRU1zhFPXtz8+OwEZ2xzpW3k6DhDxwcVRRZbW7JhOD/qieYw9kDUDhAWbpmhU4QeFhnqMkrKCF5tqVtK0HOk/Z6wFEDopJGJ7OnPJAp57sfv+Lg6knw5qyz7lkx6WuPLJh6DygUYRWcZD2izV9iqinAAVI2pqWuE4uZIXvvF6vkyj1OfAV7b2d0RKrMt9ZuC3TpzxFEau3r9xxQ26iHrYRxrLE6I9nQT0V7GbNMAQzH9+x0x0pmfbWq2CWby9SSQ==</ds:SignatureValue>
        <ds:KeyInfo>
            <ds:X509Data>
                <ds:X509Certificate>MIID1zCCAr+gAwIBAgIUGLf0PTJTRxdcPtHTRCksRL0b9IswDQYJKoZIhvcNAQELBQAwezELMAkGA1UEBhMCSU4xDDAKBgNVBAgMA0dvYTEOMAwGA1UEBwwFVmVybmExEzARBgNVBAoMClBlcnNpc3RlbnQxDDAKBgNVBAsMA0libTErMCkGCSqGSIb3DQEJARYcY3lydXNfZnVydGFkb0BwZXJzaXN0ZW50LmNvbTAeFw0yMzEwMDkxMTQ5NDZaFw0yNDA0MDYxMTQ5NDZaMHsxCzAJBgNVBAYTAklOMQwwCgYDVQQIDANHb2ExDjAMBgNVBAcMBVZlcm5hMRMwEQYDVQQKDApQZXJzaXN0ZW50MQwwCgYDVQQLDANJYm0xKzApBgkqhkiG9w0BCQEWHGN5cnVzX2Z1cnRhZG9AcGVyc2lzdGVudC5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC2H1meXHs2/bc91+AD2QDiXN6MW4T3EkdT9O4m2xYyifd19ufKjU+nqeCp0j6S7q1RiG3L5MkgJLL4I6ar26Mpv9fHQlG6jhnVzh7Ks21xzhem+SjokxgbyFYDOVPD3B/yyZ7uWmQS9bsmwWeDym1Zsgb4jURl0OuWauu93mPyrUwHXZlUQBF8tcs1/uD+IyAGWeurBm9WEpoPRb9ur7YNlAQxqxMSfh8nIYpywHOPUrRXEX6yDAGsju85UANlbSwEBr8CTF7OkEF7NLxT/gESN0D8M8ZBwAEGxA5J6d//SXpqiLDt1ZfsBB2f27wO3c4s+OOGNvdq57yiYc7+mx3tAgMBAAGjUzBRMB0GA1UdDgQWBBQ5+yobfvbsYodF42pwYG5WQKiyRzAfBgNVHSMEGDAWgBQ5+yobfvbsYodF42pwYG5WQKiyRzAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBe+TuprCYWSd/0SV910Gylg6OkPdMmh7Iuum87cwQSvL0PCpe2V531mvqXcWmeeetB1ABAX38TUdG9watNxJyZPELXGqc29YwHZQ28iOpWH3GXtm73mjXfF9L2up9tgc9AbVsG7WlZp8eTE/HdLLVKsFCG02cww57mCl+Ob2OLwCm+cfr2K5aceSk/6yvP+N5W1fajFtf0PLrt7RyQ2M8k+ySvSCHPED4VEZjJo5xUCH3CS+ooHkIHBHq+Ef6WRXA+DNLxvXppko7EKTIurG6gm5Q6zwaBfExbgWHSobs2BVlZBLvUuMhY6HAbXlan0T+lZvXC2kvgy+BULIPKziX3</ds:X509Certificate>
            </ds:X509Data>
        </ds:KeyInfo>
    </ds:Signature>
    <samlp:NameIDPolicy AllowCreate="true" />
</samlp:AuthnRequest>
russellhaering commented 10 months ago

This is the intended behavior and not a security issue.

You can see a similar example of a signed request here: https://www.samltool.com/generic_sso_req.php

In a simple configuration the Identity Provider can receive this signed request, and simply ignore the KeyInfo element containing the certificate and just validate the signature using a certificate it has been statically configured with.

The reason the certificate is sent along at all is so that the Identity Provider can (if it chooses) use more complex PKIX schemes, like trusting a root certificate and verifying that the certificate supplied by the Service Provider has a valid chain of trust back to that root.

In practice a lot of identity providers don't seem to even require that requests be signed, but all of the above also applies in reverse to the Response flow.

cyrusfurtado commented 10 months ago

Thank you for clarifying that. Great minimalistic saml plugin btw.