tngan / samlify

Node.js library for SAML SSO
https://samlify.js.org
MIT License
610 stars 217 forks source link

Parsing SAMLResponse from ADFS yields Error: ERR_ZERO_SIGNATURE #302

Open coreypmurphy opened 5 years ago

coreypmurphy commented 5 years ago

Sending SAML Requests to ADFS work fine and receiving POSTs from ADFS are being captured but parsing the SAML response yields the following stack trace below. I've included below the environment info as well. I can provide the SP metadata but I'm unsure about providing the IDP metadata since it's private to our organization. The SAML response is encrypted using our x509 cert and our server has the certs setup via apache mod_ssl, which is processing the POST from ADFS and proxying it on to our nodejs app running samlify. Any help is appreciated as I've gotten this far but I'm relatively green when it comes to security.

Error: ERR_ZERO_SIGNATURE at Object.verifySignature (/u01/www/lndsso/node_modules/samlify/build/src/libsaml.js:262:23) at /u01/www/lndsso/node_modules/samlify/build/src/flow.js:196:55 at step (/u01/www/lndsso/node_modules/samlify/build/src/flow.js:32:23) at Object.next (/u01/www/lndsso/node_modules/samlify/build/src/flow.js:13:53) at fulfilled (/u01/www/lndsso/node_modules/samlify/build/src/flow.js:4:58) at process._tickCallback (internal/process/next_tick.js:68:7)

Environment is as follows: Node 10.15.1 "@authenio/samlify-node-xmllint": "^1.0.1", "@authenio/samlify-xsd-schema-validator": "^1.0.2", "body-parser": "^1.19.0", "express": "^4.17.1", "samlify": "^2.6.0"

tngan commented 5 years ago

@coreypmurphy Make sure the response is signed. SAML response is restricted to be signed (either message signature or assertion signature).

coreypmurphy commented 5 years ago

In working with our AD team, they've set ADFS to return signed message and assertion. After doing so, the error has changed to "Error: ERR_EMPTY_ASSERTION" despite their being an "EncryptedAssertion" node in the SAML response that contains various other nodes including CipherValue and CipherData nodes among others. Thoughts? Are we getting close?

tngan commented 5 years ago

@coreypmurphy Is the response signed then encrypted or encrypted then signed?

tngan commented 5 years ago

Check the option messageSigningOrder, it is sign-then-encrypt by default, change to encrypt-then-sign if it is the case.

https://samlify.js.org/#/idp-configuration?id=idp-configuration

coreypmurphy commented 5 years ago

The option you reference is an option to set if we're setting up the IDP within Samlify which we're not, unless I'm confused about what you're suggeting. Are you suggesting I ask our ADFS team what their signing order is? And have them reverse what they're doing to see if it solves my error? Thanks,

tngan commented 5 years ago

@coreypmurphy You can ask your ADFS team to check the signing order first. Either it can be changed on their side or you could set the option in order to have a correct parsing pipeline.

SJAnderson commented 5 years ago

@tngan we are seeing similar behavior with cornell's IDP: metadata link

We cannot ask them to change their encryption setting and our SP is receiving the same error @coreypmurphy reported, ERR_EMPTY_ASSERTION. Setting messageSigningOrder on our SP doesn't work, nor does it appear in the types.

Environment: Node 10.10.0 "@authenio/samlify-node-xmllint": "^1.0.1", "@authenio/samlify-xsd-schema-validator": "^1.0.2", "samlify": "^2.6.1"

tngan commented 5 years ago

@SJAnderson Please send an email to passify.io@gmail.com in order to get rid of private stuffs. I will update this thread if we find out the issue.

big-kahuna-burger commented 4 years ago

Having same issue here. I don't know IDP's signing order from ADFS side. Message contains EncryptedAssertion but no

Funny thing is, same IDP works with passport-saml but I can't make it work with samlify.

big-kahuna-burger commented 4 years ago

Found it: See how passport-saml verifies signature. here

I wanted to get rid of whole passport as a "framework" thing in one of our projects, but I can't make samlify work with our ADFS.

If you can rework samlify to follow how passport-saml verifies sigs, then we could use samlify and get rid of passport, this way not yet.

See this issue and this comment on attack vectors.

Problem with samlify is that it's xPaths are absolute and it doesn't validate node-by-node, but xml as a whole.

julienfouilhe commented 4 years ago

Any news on this? I also have the same problem with one IdP

tngan commented 4 years ago

Looks like the xpath of signature node varies with different IDP. I need to check it right the way.

From the schema we have right now, the Signature node must appear in a sequence, say after Issuer node.

https://github.com/authenio/samlify-node-xmllint/blob/b486eec67a001260391028daee5594b789ed8d0f/schemas/saml-schema-assertion-2.0.xsd#L58-L70

alex-maxime commented 2 years ago

Having same issue here. I don't know IDP's signing order from ADFS side. Message contains EncryptedAssertion but no

Funny thing is, same IDP works with passport-saml but I can't make it work with samlify.

Same for me, after many day trying to configure & reconfigure i was able to login using passport-saml