Subject: | CA and Signature verification should not be required |
Date: | Mon, 14 May 2012 12:36:11 -0400 |
To: | "bug-Net-SAML2 [...] rt.cpan.org" <bug-Net-SAML2 [...] rt.cpan.org> |
From: | Jason Chang <jchang [...] athenahealth.com> |
Under Net::SAML2 0.17, the POST and Redirect binding modules require the certificate for the certificate authority that issued the IdP certificate; and they sign the request before sending it to the identity provider, and assume that the resulting assertion has been signed.
However this verification is optional under the SAML 2 specification. (In the Core specification with errata inline at http://www.oasis-open.org/committees/download.php/35711/sstc-saml-core-errata-2.0-wd-06-diff.pdf, lines 3033 and 3036 indicate "All SAML assertions MAY be signed using XML signature." and "All SAML protocol request and response messages MAY be signed using XML signature.".)
Thus these modules cannot be used in a standards-compliant way with some identity providers. For example, OneLogin does not use XML signature by default. Assertions from them without signatures cause an instantiation error.
I suggest the following changes:
Make cacert an optional parameter in POST and Redirect.
Make key an optional parameter in Redirect.
In POST::handle_response, do not call $x->verify unless a Signature element is present.
In POST::handle_response, do not instantiate a VerifyX509 from $self->cacert nor call $ca->verify($cert) unless $self->cacert is defined.
In POST::handle_response, do not append " (verified)" to the subject if verification was not attempted.
Add new functions in Redirect that are modified versions of sign and verify (leaving sign and verify alone, since it would be confusing to have a function named "sign" that could optionally not sign); for example, "generate_uri" and "handle_response".
The new Redirect::generate_uri would add the SigAlg and Signature params, and would call new_private_key and would calculate $sig, only if $self->key is defined.
The new Redirect::handle_response would instantiate $cert and $rsa_pub, and would check the SigAlg and Signature params, only if Signature is present.