From 8f1fe3269b5e4497d97a5534f5236a07140c0560 Mon Sep 17 00:00:00 2001 From: Mark Stosberg Date: Fri, 16 Feb 2018 14:46:29 -0500 Subject: [PATCH 1/3] docs: mention that disableRequestAuthnContext helps with AD FS Also, clarify that 'cert:' is for the signature cert, not the encryption certificate. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b6a1d55c8..5d554087a 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ passport.use(new SamlStrategy( * `identifierFormat`: if truthy, name identifier format to request from identity provider (default: `urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress`) * `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`. * `attributeConsumingServiceIndex`: optional `AttributeConsumingServiceIndex` attribute to add to AuthnRequest to instruct the IDP which attribute set to attach to the response ([link](http://blog.aniljohn.com/2014/01/data-minimization-front-channel-saml-attribute-requests.html)) - * `disableRequestedAuthnContext`: if truthy, do not request a specific auth context + * `disableRequestedAuthnContext`: if truthy, do not request a specific authentication context. This is [known to help when authenticating against Active Directory](https://github.com/bergie/passport-saml/issues/226) (AD FS) servers. * `authnContext`: if truthy, name identifier format to request auth context (default: `urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport`) * `forceAuthn`: if set to true, the initial SAML request from the service provider specifies that the IdP should force re-authentication of the user, even if they possess a valid session. * `providerName`: optional human-readable name of the requester for use by the presenter's user agent or the identity provider @@ -131,7 +131,7 @@ For example: ``` -It is a good idea to validate the incoming SAML Responses. For this, you can provide the Identity Provider's public PEM-encoded X.509 certificate using the `cert` confguration key. The "BEGIN CERTIFICATE" and "END CERTIFICATE" lines should be stripped out and the certificate should be provided on a single line. +It is a good idea to validate the signatures of the incoming SAML Responses. For this, you can provide the Identity Provider's public PEM-encoded X.509 signing certificate using the `cert` confguration key. The "BEGIN CERTIFICATE" and "END CERTIFICATE" lines should be stripped out and the certificate should be provided on a single line. ```javascript cert: 'MIICizCCAfQCCQCY8tKaMc0BMjANBgkqh ... W==' From 2de3528f308f2103625b191c5b32432636f1592e Mon Sep 17 00:00:00 2001 From: Mark Stosberg Date: Fri, 16 Feb 2018 14:59:18 -0500 Subject: [PATCH 2/3] package.json: bump version to 0.33.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 32268d9d9..c2cf29f0f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "passport-saml", - "version": "0.32.1", + "version": "0.33.0", "license" : "MIT", "keywords": [ "saml", From 966b97a54e7a4b25316716c20e1f524b64cf8a22 Mon Sep 17 00:00:00 2001 From: Mark Stosberg Date: Tue, 14 Aug 2018 10:34:40 -0400 Subject: [PATCH 3/3] internal: provide unique failure messages for invalid signatures. Fixes #146 Before, there were 6 different cases in which "Invalid Signature" could be returned, making it hard to debug which case had been triggered. Now we return a unique message in each case. For improvement backwards compatibility, all cases still contain the string "Invalid signature", so code that was checking for that regex will be fine. If code was checking for the exact string "Invalid Signature", it will need to be updated. --- lib/passport-saml/saml.js | 10 +++++----- test/tests.js | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/passport-saml/saml.js b/lib/passport-saml/saml.js index 1b7e4a803..e6d69d0ae 100644 --- a/lib/passport-saml/saml.js +++ b/lib/passport-saml/saml.js @@ -583,7 +583,7 @@ SAML.prototype.validatePostResponse = function (container, callback) { if (assertions.length + encryptedAssertions.length > 1) { // There's no reason I know of that we want to handle multiple assertions, and it seems like a // potential risk vector for signature scope issues, so treat this as an invalid signature - throw new Error('Invalid signature'); + throw new Error('Invalid signature: multiple assertions'); } if (assertions.length == 1) { @@ -612,7 +612,7 @@ SAML.prototype.validatePostResponse = function (container, callback) { if (self.options.cert && !validSignature && !self.validateSignature(decryptedXml, decryptedAssertions[0], certs)) - throw new Error('Invalid signature'); + throw new Error('Invalid signature from encrypted assertion'); self.processValidlySignedAssertion(decryptedAssertions[0].toString(), inResponseTo, callback); }); @@ -640,7 +640,7 @@ SAML.prototype.validatePostResponse = function (container, callback) { var nestedStatusCode = statusCode[0].StatusCode; if (nestedStatusCode && nestedStatusCode[0].$.Value === "urn:oasis:names:tc:SAML:2.0:status:NoPassive") { if (self.options.cert && !validSignature) { - throw new Error('Invalid signature'); + throw new Error('Invalid signature: NoPassive'); } return callback(null, null, false); } @@ -672,7 +672,7 @@ SAML.prototype.validatePostResponse = function (container, callback) { } } else { if (self.options.cert && !validSignature) { - throw new Error('Invalid signature'); + throw new Error('Invalid signature: No response found'); } var logoutResponse = doc.LogoutResponse; if (logoutResponse){ @@ -916,7 +916,7 @@ SAML.prototype.validatePostRequest = function (container, callback) { .then(function(certs) { // Check if this document has a valid top-level signature if (self.options.cert && !self.validateSignature(xml, dom.documentElement, certs)) { - return callback(new Error('Invalid signature')); + return callback(new Error('Invalid signature on documentElement')); } processValidlySignedPostRequest(self, doc, callback); diff --git a/test/tests.js b/test/tests.js index a4717d1b9..ce7150875 100644 --- a/test/tests.js +++ b/test/tests.js @@ -839,7 +839,7 @@ describe( 'passport-saml /', function() { var samlObj = new SAML( samlConfig ); samlObj.validatePostResponse( container, function( err, profile, logout ) { should.exist( err ); - err.message.should.match( 'Invalid signature' ); + err.message.should.match(/Invalid signature/); done(); }); }); @@ -854,7 +854,7 @@ describe( 'passport-saml /', function() { var samlObj = new SAML( samlConfig ); samlObj.validatePostResponse( container, function( err, profile, logout ) { should.exist( err ); - err.message.should.match( 'Invalid signature' ); + err.message.should.match(/Invalid signature/); done(); }); }); @@ -869,7 +869,7 @@ describe( 'passport-saml /', function() { var samlObj = new SAML( samlConfig ); samlObj.validatePostResponse( container, function( err, profile, logout ) { should.exist( err ); - err.message.should.match( 'Invalid signature' ); + err.message.should.match(/Invalid signature/); done(); }); }); @@ -887,7 +887,7 @@ describe( 'passport-saml /', function() { var samlObj = new SAML( samlConfig ); samlObj.validatePostResponse( container, function( err, profile, logout ) { should.exist( err ); - err.message.should.match( 'Invalid signature' ); + err.message.should.match(/Invalid signature/); done(); }); }); @@ -1594,7 +1594,7 @@ describe( 'passport-saml /', function() { }; samlObj.validatePostRequest(body, function(err) { should.exist(err); - err.should.eql(new Error('Invalid signature')); + err.should.eql(new Error('Invalid signature on documentElement')); done(); }); });