search

SAML-Toolkits / python-saml

Python SAML Toolkit
MIT License
660 stars 308 forks source link

Trying to use add_sign adds new line characters in embedded signature #148

Closed gsvitak closed 8 years ago

gsvitak commented 8 years ago

Hello,

I was noticed in https://github.com/onelogin/python-saml/pull/78 that the signature was not using the signature api. I am trying to use the OneLogin_Saml2_Utils.add_sign method to embed the signature in the AuthnRequest for the HTTP-POST binding. The method works great but it appears to add new lines/carriage returns to the signature value.

Based on the HTTP-POST sample (https://www.samltool.com/generic_sso_req.php), I think the new lines/carriage returns will cause issues with the IDP.

Can you please confirm if the extra characters will cause an issue? Also, can you please offer a suggestion on how to eliminate the characters?

I have validated my SP cert and private key are formatted properly by stepping through the constructor of OneLogin_Saml2_Settings

self.format_sp_cert()
self.format_sp_key()

Thanks for the help in advance. Greg

<ds:SignatureValue>R/+/4XFlWhEptIXukn5hkkalFimlutchfYlEoVMhTWd+QlId818h1CiplPSlUnGZ
         Eor55SNaXdn8vdyDv54MmYkMJRW1+kp+/P7HijVWaCisR5hXghZnazLXWkFJOyuH
         k2MQCT7ZIe4EDmsRZterDfRlcpqdlNfWJkXKq07EmuqgiPF6t8WtVBxrEZ/Pcu3+
         GY8K4+p0KgJp3ocr985+Zbyo9jtepO4+xRjyEfL3CqfHLR6+Xa91A1DH2oAVJTZZ
         HCC2iy30zRqGACoHHTd2mEcyG3i5n456W3LEYE/xNeVu3bL3/EdRNUP+gcyLGL9f STDwaxGbuJx00cjtT8bKjQ==</ds:SignatureValue>
pitbulk commented 8 years ago

AuthnRequest HTTP-POST binding is not supported by this toolkit.

What extra spaces are added by OneLogin_Saml2_Utils.add_sign ?

gsvitak commented 8 years ago

After further debugging the issue, the problem with using the OneLogin_Saml2_Utils.add_sign for a post is not the extra spaces. If you use the method to sign the XML request for posting to an IDP, the method add the namespace attributes to the XML after the document is signed. This causes the signature to become invalid.

I used your suggestion from #78 and forked the repo to add some methods to support HTTP-POST.