Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add_sign tampering with the xml, resulting in invalid signature #231

Closed
RamezIssac opened this issue May 1, 2018 · 10 comments
Closed

add_sign tampering with the xml, resulting in invalid signature #231

RamezIssac opened this issue May 1, 2018 · 10 comments

Comments

@RamezIssac
Copy link

RamezIssac commented May 1, 2018

Good day dear,
I'm implementing HTTP-POST which is requiring enveloped signatures.

I implemented your hint on the php repo (and here on other issues) using add_sign
But, indeed add_sign do tamper with the resulting xml (as in #148 ).
I reviewed your commit 5e08bb2 , it certainly look way better at solving the issue but unfortunately it still yields an invalid signature from the idp.

On a side note: If i sign the xml request on the onelogin site online tools, i get through and have no errors.

How can i help resolving this issue ?

Thank you.

@RamezIssac RamezIssac changed the title add_sign tempering with the xml, resulting in invalid signature add_sign tampering with the xml, resulting in invalid signature May 1, 2018
@pitbulk
Copy link
Contributor

pitbulk commented May 1, 2018

@RamezIssac have you tried to validate the generated XML with the validate_sign method?

Also, try to use python3-saml and see if it also give you the same issue.

@RamezIssac
Copy link
Author

@pitbulk Thank you for the quick reply
The validate_sign is returning False (on parameter validate_cert = True or False)
Here is a complete transcript of the flow

saml_settings = OneLogin_Saml2_Settings(custom_base_path=settings.SAML_FOLDER, sp_validation_only=True)
security = saml_settings.get_security_data()  #
authn_req = authn_request.OneLogin_Saml2_Authn_Request(saml_settings, force_authn=True,
                                                       is_passive=False,
                                                       set_nameid_policy=False)

auth_req_xml = authn_req.get_xml()

key = saml_settings.get_sp_key()
cert = saml_settings.get_sp_cert()
signatureAlgorithm = security['signatureAlgorithm']
xmlAlgorithm = security['digestAlgorithm']

signed_xml = OneLogin_Saml2_Utils.add_sign(auth_req_xml, key, cert, sign_algorithm=signatureAlgorithm)
print(OneLogin_Saml2_Utils.validate_sign(signed_xml, cert, key, signatureAlgorithm, validatecert=True))

`

I'll test wtih Python3 and get back to you.

@RamezIssac
Copy link
Author

Same behavior on Python3-saml , validate_sign return False.
Adding sign via same procedure as above.

@RamezIssac
Copy link
Author

Hello again ,
Can you please share with your findings / insights ?

Were this bug re-produce-able on your end ?
What do you think we should investigate ? Any further ideas?

Thank you in advance.

@pitbulk
Copy link
Contributor

pitbulk commented May 16, 2018

The method OneLogin_Saml2_Utils.validate_sign by default only supports validate signatures of Message or Assertion if no xpath parameter provided. So the use you do on your script will get always a False value on validation due in the XML there is no Response or Assertion element.

You may modify the use of validate_sign as follows.

Replace:

OneLogin_Saml2_Utils.validate_sign(signed_xml, cert, key, signatureAlgorithm, validatecert=False)

by

OneLogin_Saml2_Utils.validate_sign(signed_xml, cert, key, signatureAlgorithm, validatecert=False, xpath='/samlp:AuthnRequest/ds:Signature')

@RamezIssac
Copy link
Author

Thank you for the reply and for the guide on how to use validate_sign validating authn_request.

Unfortunately,
I'm afraid validate_sign is still not returning True for the signed xml via add_sign while it is returning True when passed the "correctly signed xml" from onelogin online tools.

@pitbulk
Copy link
Contributor

pitbulk commented May 16, 2018

It worked for me with the code on master

@RamezIssac
Copy link
Author

Ok can you please share the code you use to add the signature to the authn_request please ?

@pitbulk
Copy link
Contributor

pitbulk commented May 16, 2018

The same script code that you shared but with the changes I provided

@RamezIssac
Copy link
Author

Ok, This is embracing.
While i was writing the test case to send it as a proof, Falses "magically" turned to Trues and idp is happy, No clear explanation. (just maybe *.pyc files messed with me.. )
And indeed works on master after the commit 5e08bb2

Thank you indeed for your help and time. Closing issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants