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

Sample configuration for Shibboleth v3 #520

Open
giacomobartoli opened this issue Jan 21, 2021 · 7 comments
Open

Sample configuration for Shibboleth v3 #520

giacomobartoli opened this issue Jan 21, 2021 · 7 comments
Labels
documentation Request for or contribution to documentation pr-welcome

Comments

@giacomobartoli
Copy link

giacomobartoli commented Jan 21, 2021

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Collaborator

cjbarth commented Jan 22, 2021

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

@srd90
Copy link

srd90 commented Jan 22, 2021

@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

checkTimestampsValidityError(nowMs: number, notBefore: string, notOnOrAfter: string) {
if (this.options.acceptedClockSkewMs == -1)
return null;
if (notBefore) {
const notBeforeMs = Date.parse(notBefore);
if (nowMs + this.options.acceptedClockSkewMs < notBeforeMs)
return new Error('SAML assertion not yet valid');
}
if (notOnOrAfter) {
const notOnOrAfterMs = Date.parse(notOnOrAfter);
if (nowMs - this.options.acceptedClockSkewMs >= notOnOrAfterMs)
return new Error('SAML assertion expired');
}
return null;
}

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

if (this.options.skipRequestCompression) {
requestToUrlHelper(null, Buffer.from((request || response)!, 'utf8'));
}
else {
zlib.deflateRaw((request || response)!, requestToUrlHelper);
}

and
SAMLRequest: base64

@giacomobartoli
Copy link
Author

@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
Copy link
Contributor

Very helpful @srd90 !

@cjbarth cjbarth added documentation Request for or contribution to documentation pr-welcome labels Mar 31, 2021
brenapp added a commit to brenapp/cu-smart-native that referenced this issue Aug 12, 2021
@srd90
Copy link

srd90 commented Sep 12, 2021

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: brenapp/cu-smart-native@8cee0e0 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Request for or contribution to documentation pr-welcome
Projects
None yet
Development

No branches or pull requests

4 participants