search

ktbartholomew / saml-20-single-sign-on

Wordpress plugin that makes a Wordpress site act as a SAML service provider
GNU General Public License v2.0
37 stars 22 forks source link

Certs in database break request signing #14

Open ktbartholomew opened 8 years ago

ktbartholomew commented 8 years ago

Introduced by #5, the way the signing certificate is read from the database and inserted into the metadata breaks the IdP's ability to verify the signature of the AuthnRequest. It looks like the certificate and key are being stored/output including the header and footer that PEM files use, whereas IdPs (at least simplesamlphp's IdP) expect to receive only the certificate data.

Here's the bad way the key appears in the metadata, notice the trimmed -----BEGINCERTIFICATE-----:

<md:KeyDescriptor use="signing">
  <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
    <ds:X509Data>
      <ds:X509Certificate>
-----BEGINCERTIFICATE-----MIID+zCCAuOgAwIBAgIBADANBgkqhkiG9w0BAQUFADCBlzELMAkGA1UEBhMCVVMxGTAXBgNVBAoMEFNBTUwgU2luZ2xlIFNpdGUxMjAwBgNVBAMMKVNBTUwgU2luZ2xlIFNpdGUgU0FNTCBTaWduaW5nIENlcnRpZmljYXRlMSQwIgYJKoZIhvcNAQkBFhVhZG1pbkBsb2NhbGhvc3QubG9jYWwxEzARBgNVBAgMClNvbWUtU3RhdGUwHhcNMTYwNTE2MDE1NjMwWhcNMjEwNTE1MDE1NjMwWjCBlzELMAkGA1UEBhMCVVMxGTAXBgNVBAoMEFNBTUwgU2luZ2xlIFNpdGUxMjAwBgNVBAMMKVNBTUwgU2luZ2xlIFNpdGUgU0FNTCBTaWduaW5nIENlcnRpZmljYXRlMSQwIgYJKoZIhvcNAQkBFhVhZG1pbkBsb2NhbGhvc3QubG9jYWwxEzARBgNVBAgMClNvbWUtU3RhdGUwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC8P8yIBSjLg9VRIHsdWu6NPc8cEiPLiL8EGnW4hD7rC5MKv4uLe0HzdsKwGjW1G6+G0TmlNnRPnh7JnUMg4l6Ql24UWHacSKSdcoAlRIfprL/du319DfYCvvKES9js88q5a11tjI5l1w4e6oMByC1/Xy+V4wpsomHCV/zCMpDBplrwSHUpMfZz8qwtdYz8KiO8KHZxPEJ67uxp3x+BSPdkQG9O8rKdd2iTCvxD7ABYMC3f6U4TjdezVZV/T7HXftGlvD2XzmLUcMaWMQt8t9l57q8IJ+CG8bQywtP1nI6DSG8iSHrNBlMMy7VkEREBhTdQcK8d5LOxaAWa9ugJd6nxAgMBAAGjUDBOMB0GA1UdDgQWBBTMglEVU6TGGJ44Y4kLl4co220ONjAfBgNVHSMEGDAWgBTMglEVU6TGGJ44Y4kLl4co220ONjAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEBBQUAA4IBAQC3RkXF6HEFo155opbwbA5BXClaoqIwSFVmBJZxJ3N9J3PV1xdsVsWJxp5wB+4OkQu36UyFQbcR5qJDMj+0A81hv9VWQCTRfUI6pBcMHzWau0Eq4BNHLU5uJ5i0+GoOWNYU52dFgIbndS87YkFkVeTdWRA/itIxxC8RqMtcItOOu6KZpYSQU8ggl4u5GBief2tI3GZtSX3NsiMgYaWRt/g8TVTIJqHOQZYq7BX+0xaKpJOLf5NOCqMPOmkQSG3cqieDpWwC6BJdwlnzLxvfG/v9UecNYBt9QfVwCGiQ63vXyAH2IQg5pUcaNJm/406/Yr3AcUKKN8hvmY8hBtXTKv7p-----ENDCERTIFICATE-----
      </ds:X509Certificate>
    </ds:X509Data>
  </ds:KeyInfo>
</md:KeyDescriptor>
bmanth60 commented 8 years ago

@ktbartholomew Thanks! Should we just trim the headers?

ktbartholomew commented 8 years ago

@bmanth60 Yes, it look like trimming the header and footer is all that's needed.