search

tngan / samlify

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

Validation of InResponseTo against ID of AuthnRequest #330

Open gingerwizard opened 4 years ago

gingerwizard commented 4 years ago

Am i correct that that currently samlify doesn't validate the InResponseTo against the ID of the AuthnRequest? If not, does this not prevent reuse of AuthnRequests and thus the limitation represents a vulnerability? If the above is true, are there examples of how samlify should be extended to provide this support - i assume a distributed cache is required e.g. Redis.

Finally, would this also need to consider the IssueInstant to prevent an ever growing cache?

tngan commented 4 years ago

@gingerwizard Yes, we don't check it right now, we just extract the essential information from the response, and delegate the checking process to user side now, we only provide decryption and signature validation right now.

We do have plan to include those basic checking like expiration, issuer, response target in the near future.

nflaig commented 4 years ago

Does it make sense to verify the InResponseTo property if you allow IdP-initiated SSO? As far as the spec goes the property will be omitted in that scenario.

What I also noticed which is even more important to verify is the login response ID to prevent replay attacks. See 4.1.4.5:

The service provider MUST ensure that bearer assertions are not replayed, by maintaining the set of used ID values for the length of time for which the assertion would be considered valid based on the NotOnOrAfter attribute in the .

We might want to open another issue for this but also in my opinion this is hard to implement in the library itself since you would have to store the ids in-memory which wont work in most system where a distributed cluster is used. Probably the best thing would be to mention this somewhere in the documentation that this has to be implemented on top of the validation the library does. According to the specification this is not optional.

tmonck commented 3 years ago

So just curious will any of the properties mentioned expiration, issuer, response target and login response id are still planed to be validated? If so is there a timeline of when this feature might be available?

tmonck commented 3 years ago

Just checking in post the new year to see if there is anything on this.

williwlwilliwll commented 1 year ago

Hello!

Has anything changed since your original response to @gingerwizard ? Does the app now validate the ID of a received SAMLResponse against IDs that it has issued in SAMLRequests?

Thanks 🥇