node-saml / passport-saml

SAML 2.0 authentication with Passport
MIT License
861 stars 476 forks source link

Sample configuration for Shibboleth v3 #520

Open giacomobartoli opened 3 years ago

giacomobartoli commented 3 years ago

Is there any sample config for using passport-saml with Shibboleth?

I tried all possible combinations with passport-saml but I keep running into error 400 bad request. This is my request according to SAML tracer plugin:

saml summary

And this is the response: Screenshot 2021-01-21 at 18 46 39

markstos commented 3 years ago

There's not. You should ask the owner of that IIS server to check their logs and see why it's returning a 400 error.

giacomobartoli commented 3 years ago

After few days (even nights!) I came out with this configuration that seems to be working fine with Shibbolet v3.4.6:

callbackUrl: config.development.passport.saml.callbackUrl,
entryPoint: config.development.passport.saml.entryPoint,
issuer: config.development.passport.saml.issuer,
privateKey: config.development.passport.saml.privateKey, //SP private key in .pem format
cert: config.development.passport.saml.cert, //IdP public key in .pem format
decryptionPvk: config.development.passport.saml.decryptionPvk, //same as privateKey
identifierFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient', 
authnContext: ['urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified'],
authnRequestBinding: 'HTTP-REDIRECT',
protocol: 'https://',
signatureAlgorithm: 'sha256',
acceptedClockSkewMs: -1

I do hope this will help someone else facing the same issue. @markstos let me know if there is a wiki or something where I can report that. Thanks anyway for help.

cjbarth commented 3 years ago

I would suggest that you create a PR with an adjustment to the readme file following this example.

srd90 commented 3 years ago

@giacomobartoli based on infromation at the pictures you posted in the first message situation seemed/seems to be that

  1. you had configured your passport-saml to use Shibboleth IdPv3's HTTP-POST binding ( https://[...]/idp/profile/SAML2/POST/SSO ) as an endpoint for authnrequest, but
  2. at the same time you had configured passport-saml to use HTTP-Redirect binding format and transport mechanism for the authnrequest (meaning HTTP GET with authnrequest, signature and algorithm transported via HTTP query parameters). One of the pictures shows GET method.

So I would have said that 400 Bad Request was a result of Shibboleth IdPv3 receiving HTTP GET to interface/endpoint which accepts only HTTP POST.

https://[...]/idp/profile/SAML2/Redirect/SSO would/could have been more appropriate endpoint. https://wiki.shibboleth.net/confluence/display/IDP30/ProtocolsAndInterfaces

In fact your followup post does not rule out aforementioned scenario because that post is basically just (insecure) vanilla passport-saml configuration without information of IdP side endpoints.

Why is it insecure? Because as https://github.com/node-saml/passport-saml/blob/932da9d09a018fed4cb830e67090bb994f8539c1/README.md says:

acceptedClockSkewMs: Time in milliseconds of skew that is acceptable between client and server when checking OnBefore and NotOnOrAfter assertion condition validity timestamps. Setting to -1 will disable checking these conditions entirely. Default is 0.

and https://github.com/node-saml/passport-saml/blob/932da9d09a018fed4cb830e67090bb994f8539c1/src/passport-saml/saml.ts#L1187-L1203

So authentication response message could be used multiple times and even after NotOnOrAfter.

Configuration lacks other checks also, like validateInResponseTo, audience etc.

BTW. you had obscured information (domain name) from images you had attached to first post. Maybe due security reasons(?) but you did not obscure image which contains SAMLRequest query param value as-is. Content of that query parameter is just deflated+base64 encoded XML string(*) and it would be trivial to extract obscured domain from information provided at that image (assuming that image is from same samltracer message as other images).

(*) see https://github.com/node-saml/passport-saml/blob/932da9d09a018fed4cb830e67090bb994f8539c1/src/passport-saml/saml.ts#L495-L500 and https://github.com/node-saml/passport-saml/blob/932da9d09a018fed4cb830e67090bb994f8539c1/src/passport-saml/saml.ts#L464

giacomobartoli commented 3 years ago

@srd90 you're right. To make it work I had to change the binding request from HTTP-POST to HTTP-REDIRECT and I have already changed the acceptedClockSkewMs flag. Let's say that at the beginning I just tried to make it work, even by attempts. Then I improved the configuration gradually.. step by step. For sure, my PR wouldn't incluse those mistakes. However, I must admit I was not aware of the queryParam request (taken from SAML tracer plugin) would reveal the original data since it is made by the XML original string (base64 and deflated). So.. thank you for the information :-)

markstos commented 3 years ago

Very helpful @srd90 !

srd90 commented 3 years ago

The reason why I rushed to comment 22th Jan 2021 @giacomobartoli ’s solution he presented earlier at the same day is that it is NOT anything shibboleth specific and furthermore that it is very unsecure configuration.

Another reason for rushing was that there is/was danger that someone finds @giacomobartoli 's configuration set and does not try to understand side-effects of copy pasting those values as-is into somewhere.

It seems that unsecure configuration set has been reused: https://github.com/MayorMonty/cu-smart-native/commit/8cee0e06a39e2bb1014c30241b51b41b6cf65ac4 Based on commit comment the config values were taken as if those are THE way to use Shibboleth IdP (ping @MayorMonty consider changing at least accepted clock skew value to >= 0 and enable audience verification ).

Reason for adding this comment to this issue is to ”provide learning experience” how it is bad idea to copy paste security related stuff without understanding what is being copied.

In this particular case one project turned off notOnOrAfter validation (which is turned on by default unless accepted clock skew is set to -1) by copypasting whatever was claimed to be good configuration. Instances of that project are now open to various replay attacks.