Merge branch 'upstream' into upstream-merge_v1.2.0

* upstream: bump version to 1.2.0 Use exact match to check for a compatible crypto algorithm. Validate signatures on original query string NameIDFormat fix (node-saml#375) Remove InResponseTo value if response validation fails v1.1.0: bump for release # Conflicts: # lib/passport-saml/saml.js # package.json
vrk-kpa · Sep 16, 2019 · 0a7679b · 0a7679b
2 parents eebec20 + 3ba244d
commit 0a7679b
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 37 deletions.
diff --git a/lib/passport-saml/saml.js b/lib/passport-saml/saml.js
@@ -899,10 +899,14 @@ SAML.prototype.validatePostResponse = function (container, callback) {
   })
   .fail(function(err) {
     debug('validatePostResponse resulted in an error: %s', err);
-    Q.ninvoke(self.cacheProvider, 'remove', inResponseTo)
+    if (self.options.validateInResponseTo) {
+      Q.ninvoke(self.cacheProvider, 'remove', inResponseTo)
       .then(function() {
         callback(err);
-    });
+      });
+    } else {
+      callback(err);
+    }
   })
   .done();
 };
@@ -924,7 +928,7 @@ SAML.prototype.validateInResponseTo = function (inResponseTo) {
   }
 };
 
-SAML.prototype.validateRedirect = function(container, callback) {
+SAML.prototype.validateRedirect = function(container, originalQuery, callback) {
   var self = this;
   var samlMessageType = container.SAMLRequest ? 'SAMLRequest' : 'SAMLResponse';
 
@@ -951,7 +955,7 @@ SAML.prototype.validateRedirect = function(container, callback) {
           self.verifyLogoutResponse(doc) : self.verifyLogoutRequest(doc);
       })
       .then(function() {
-        return self.hasValidSignatureForRedirect(samlMessageType, container);
+        return self.hasValidSignatureForRedirect(container, originalQuery);
       })
       .then(function () {
         processValidlySignedSamlLogout(self, doc, callback);
@@ -976,24 +980,28 @@ function processValidlySignedSamlLogout(self, doc, callback) {
   }
 }
 
-SAML.prototype.hasValidSignatureForRedirect = function (samlMessageType, container) {
+SAML.prototype.hasValidSignatureForRedirect = function (container, originalQuery) {
   var self = this;
-  var signature = container.Signature;
-  if (signature && this.options.cert) {
-    var urlString = samlMessageType + '=' + encodeURIComponent(container[samlMessageType]);
+  var tokens = originalQuery.split('&');
+  var getParam = function (key) {
+    var exists = tokens.filter(function(t) { return new RegExp(key).test(t); });
+    return exists[0];
+  };
 
-    if (container.RelayState) {
-      urlString += '&RelayState=' +
-        encodeURIComponent(container.RelayState);
+  if (container.Signature && this.options.cert) {
+    var urlString = getParam('SAMLRequest') || getParam('SAMLResponse');
+
+    if (getParam('RelayState')) {
+      urlString += '&' + getParam('RelayState');
     }
 
-    urlString += '&SigAlg=' + encodeURIComponent(container.SigAlg);
+    urlString += '&' + getParam('SigAlg');
 
     return this.certsToCheck()
       .then(function(certs) {
         var hasValidQuerySignature = certs.some(function (cert) {
           return self.validateSignatureForRedirect(
-            urlString, signature, container.SigAlg, cert
+            urlString, container.Signature, container.SigAlg, cert
           );
         });
 
@@ -1007,15 +1015,23 @@ SAML.prototype.hasValidSignatureForRedirect = function (samlMessageType, contain
 };
 
 SAML.prototype.validateSignatureForRedirect = function (urlString, signature, alg, cert) {
-  var supportedAlgs = crypto.getHashes().filter(
-    function(h) { return new RegExp(h).test(alg); }
-  );
-
-  if (supportedAlgs.length === 0) {
+  // See if we support a matching algorithm, case-insensitive. Otherwise, throw error.
+  function hasMatch (ourAlgo) {
+    // The incoming algorithm is forwarded as a URL.
+    // We trim everything before the last # get something we can compare to the Node.js list
+    const algFromURI = alg.toLowerCase().replace(/.*#(.*)$/,'$1')
+    return ourAlgo.toLowerCase() === algFromURI
+  }
+  let i = crypto.getHashes().findIndex(hasMatch);
+  let matchingAlgo;
+  if (i > -1) {
+    matchingAlgo = crypto.getHashes()[i]
+  }
+  else {
     throw alg + ' is not supported';
   }
 
-  var verifier = crypto.createVerify(supportedAlgs[supportedAlgs.length-1]);
+  var verifier = crypto.createVerify(matchingAlgo);
   verifier.update(urlString);
 
   return verifier.verify(this.certToPEM(cert), signature, 'base64');
@@ -1480,7 +1496,10 @@ SAML.prototype.generateServiceProviderMetadata = function( decryptionCert, signi
     };
   }
 
-  metadata.EntityDescriptor.SPSSODescriptor.NameIDFormat = this.options.identifierFormat;
+  if (this.options.identifierFormat) {
+    metadata.EntityDescriptor.SPSSODescriptor.NameIDFormat = this.options.identifierFormat;
+  }
+
   metadata.EntityDescriptor.SPSSODescriptor.AssertionConsumerService = {
     '@index': '1',
     '@isDefault': 'true',

diff --git a/lib/passport-saml/strategy.js b/lib/passport-saml/strategy.js
@@ -1,6 +1,7 @@
 var passport = require('passport-strategy');
 var util = require('util');
 var saml = require('./saml');
+var url = require('url');
 
 function Strategy (options, verify) {
   if (typeof options == 'function') {
@@ -79,7 +80,8 @@ Strategy.prototype.authenticate = function (req, options) {
   }
 
   if (req.query && (req.query.SAMLResponse || req.query.SAMLRequest)) {
-    this._saml.validateRedirect(req.query, validateCallback);
+    var originalQuery = url.parse(req.url).query;
+    this._saml.validateRedirect(req.query, originalQuery, validateCallback);
   } else if (req.body && req.body.SAMLResponse) {
     this._saml.validatePostResponse(req.body, validateCallback);
   } else if (req.body && req.body.SAMLRequest) {

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "suomifi-passport-saml",
-  "version": "1.1.0-sfi.0",
+  "version": "1.2.0-sfi.0",
   "license": "MIT",
   "keywords": [
     "saml",

diff --git a/test/static/idp_slo_redirect.json b/test/static/idp_slo_redirect.json
@@ -2,5 +2,6 @@
   "SAMLRequest": "fVLbjpswEP0V5PeADQSIlaBGiipF2u62TdWHvkQDHhdUY1N72O7nF8iutL2+WeNzm6PZBxjMKO/cVzfRR/w+YaDoaTA2yPXnwCZvpYPQB2lhwCCplZfjuzuZxlyO3pFrnWGvKP9nQAjoqXeWRefTgV0rzVFrhRmvQOmmACy1wHTX5CLfbUvdZinX2yIrNIs+ow8z88BmoZkewoRnGwgszSMuqg3PN0J8ErnkmRTZFxad5m16C7SyOqJRJkmvxtg8dvGAcss5T2CiLlmCJ0AhIY+gAjmLC/AajGPRvaMH++CPmtD/4VStTvV+UZBrJl8/OxnXgulcIJnyxWmBpItsMiCBAoJ47MZ98pp607mfezufosv75fFhAtPrfvH++woseuv8APTv5kUs1kmvNnqFShygN0elPIbA6rnER+/Cmx/Of4PGYNy64TnXLcot1ygvM3wu82wVPtVXzhtdNqnaqp3K2l3JU1EitqgbIXZ5gzrLihQKUeFN7Df+y/CX46t/Ag==",
   "RelayState": "_a4653e9109c4639a2165159db4ca31e4f0ca505654",
   "SigAlg": "http://www.w3.org/2000/09/xmldsig#rsa-sha1",
-  "Signature": "HxvhXqy+UE8osj/i9PX76alvOeltoM7XaXwrYLDpOfNTcFruEhm2wMWUgqjw7XMpdDVGdEpuDwXxCByVmxwUKRBQOCORMQbzwRzPzu5jlPmYu9M0Q6oJK7S4Rlnpgxtfmbs0rlt56DCeb53JPJI1AvBdv9kfAghgVJbU0wrUDQTOkeQVwYYunJ6TaZAF4l2LqpKvwUIyReVML65UJR/T4GOGIr/QWnuRf1haq6AxhpbDdeWhgrv/CPNEmpzr/kvkDHK664Z0ruvLUlpyFUbfbYq/YQmctdr0WncyJzPnkzuw6t83pe333qLognwH8DzXS7A4ygUvZJ5/iTlr8CaYOg=="
+  "Signature": "HxvhXqy+UE8osj/i9PX76alvOeltoM7XaXwrYLDpOfNTcFruEhm2wMWUgqjw7XMpdDVGdEpuDwXxCByVmxwUKRBQOCORMQbzwRzPzu5jlPmYu9M0Q6oJK7S4Rlnpgxtfmbs0rlt56DCeb53JPJI1AvBdv9kfAghgVJbU0wrUDQTOkeQVwYYunJ6TaZAF4l2LqpKvwUIyReVML65UJR/T4GOGIr/QWnuRf1haq6AxhpbDdeWhgrv/CPNEmpzr/kvkDHK664Z0ruvLUlpyFUbfbYq/YQmctdr0WncyJzPnkzuw6t83pe333qLognwH8DzXS7A4ygUvZJ5/iTlr8CaYOg==",
+  "originalQuery": "SAMLRequest=fVLbjpswEP0V5PeADQSIlaBGiipF2u62TdWHvkQDHhdUY1N72O7nF8iutL2%2BWeNzm6PZBxjMKO%2FcVzfRR%2Fw%2BYaDoaTA2yPXnwCZvpYPQB2lhwCCplZfjuzuZxlyO3pFrnWGvKP9nQAjoqXeWRefTgV0rzVFrhRmvQOmmACy1wHTX5CLfbUvdZinX2yIrNIs%2Bow8z88BmoZkewoRnGwgszSMuqg3PN0J8ErnkmRTZFxad5m16C7SyOqJRJkmvxtg8dvGAcss5T2CiLlmCJ0AhIY%2BgAjmLC%2FAajGPRvaMH%2B%2BCPmtD%2F4VStTvV%2BUZBrJl8%2FOxnXgulcIJnyxWmBpItsMiCBAoJ47MZ98pp607mfezufosv75fFhAtPrfvH%2B%2Bwoseuv8APTv5kUs1kmvNnqFShygN0elPIbA6rnER%2B%2FCmx%2FOf4PGYNy64TnXLcot1ygvM3wu82wVPtVXzhtdNqnaqp3K2l3JU1EitqgbIXZ5gzrLihQKUeFN7Df%2By%2FCX46t%2FAg%3D%3D&RelayState=_a4653e9109c4639a2165159db4ca31e4f0ca505654&Signature=HxvhXqy%2BUE8osj%2Fi9PX76alvOeltoM7XaXwrYLDpOfNTcFruEhm2wMWUgqjw7XMpdDVGdEpuDwXxCByVmxwUKRBQOCORMQbzwRzPzu5jlPmYu9M0Q6oJK7S4Rlnpgxtfmbs0rlt56DCeb53JPJI1AvBdv9kfAghgVJbU0wrUDQTOkeQVwYYunJ6TaZAF4l2LqpKvwUIyReVML65UJR%2FT4GOGIr%2FQWnuRf1haq6AxhpbDdeWhgrv%2FCPNEmpzr%2FkvkDHK664Z0ruvLUlpyFUbfbYq%2FYQmctdr0WncyJzPnkzuw6t83pe333qLognwH8DzXS7A4ygUvZJ5%2FiTlr8CaYOg%3D%3D&SigAlg=http%3A%2F%2Fwww.w3.org%2F2000%2F09%2Fxmldsig%23rsa-sha1"
 }
diff --git a/test/static/sp_slo_redirect.json b/test/static/sp_slo_redirect.json
@@ -1,5 +1,6 @@
 {
   "SAMLResponse": "fZHNasMwEIRfxeie6Mdx4ojEUJpLIL00IYdewtra1gZZMt516ePXTluaQgnoImm+mWF3Q9D6zh7iWxz4GamLgTD5aH0ge/3aiqEPNgI1ZAO0SJYre3x4OlgzV7brI8cqenGD3CeACHtuYhDJfrcVl6VS68Va5ZlbA5bG5SkqnSKkRhuzMtUKl/miVIivIjljTyO5FaPRiBMNuA/EEHh8UjqfqcVM65PObGrG8yKSHRI3AfhK1cydlbJx3dy/1/MWbaaUkjBwLafiEpgk9wiOOAachBfycUwKP5M5xbHyau1KjStw2lSg3TLFrBTFZrKw11J98R3lYwW+jsTWqClqkpjJV7bI4IBh3tXdRt6im6+NHBl4oL+3x+gwOYMf8P6M6aq2x6GqkEjI4ivh11T+t/XiEw==",
   "SigAlg": "http://www.w3.org/2000/09/xmldsig#rsa-sha1",
-  "Signature": "RRcmtS0Qku3Y5HfSQYKax07dZOnb+CvvtP72v6ws/kttG4DYaNkRy0YmgDjrSLiVNooJe69LMb3pnHsW2PN7pKKe1p/TroGfk/7BW/QRgl3jvnLeM19qvhb3WYgwmeEXWvuX2rhopGCF2bVuzz9UDPZEDTQ6Botq39QkNTYfn1EpTlGHGQ7k/f9l2rX8DoVOu+v9Ov+PIUd0wSYiT1Nw/Nv2/xoMP+jDQCfU/x+D01wcInwlceaIKkYNb/VX3e9E3GzhOljJiUEVA7t9CEDMOACgfhiof9m/70txwhxGWPzE0XrBIIpuGp8NPU9QaiVDymbd9wzDVtll6abpx2t14A=="
+  "Signature": "RRcmtS0Qku3Y5HfSQYKax07dZOnb+CvvtP72v6ws/kttG4DYaNkRy0YmgDjrSLiVNooJe69LMb3pnHsW2PN7pKKe1p/TroGfk/7BW/QRgl3jvnLeM19qvhb3WYgwmeEXWvuX2rhopGCF2bVuzz9UDPZEDTQ6Botq39QkNTYfn1EpTlGHGQ7k/f9l2rX8DoVOu+v9Ov+PIUd0wSYiT1Nw/Nv2/xoMP+jDQCfU/x+D01wcInwlceaIKkYNb/VX3e9E3GzhOljJiUEVA7t9CEDMOACgfhiof9m/70txwhxGWPzE0XrBIIpuGp8NPU9QaiVDymbd9wzDVtll6abpx2t14A==",
+  "originalQuery": "SAMLResponse=fZHNasMwEIRfxeie6Mdx4ojEUJpLIL00IYdewtra1gZZMt516ePXTluaQgnoImm%2BmWF3Q9D6zh7iWxz4GamLgTD5aH0ge%2F3aiqEPNgI1ZAO0SJYre3x4OlgzV7brI8cqenGD3CeACHtuYhDJfrcVl6VS68Va5ZlbA5bG5SkqnSKkRhuzMtUKl%2FmiVIivIjljTyO5FaPRiBMNuA%2FEEHh8UjqfqcVM65PObGrG8yKSHRI3AfhK1cydlbJx3dy%2F1%2FMWbaaUkjBwLafiEpgk9wiOOAachBfycUwKP5M5xbHyau1KjStw2lSg3TLFrBTFZrKw11J98R3lYwW%2BjsTWqClqkpjJV7bI4IBh3tXdRt6im6%2BNHBl4oL%2B3x%2BgwOYMf8P6M6aq2x6GqkEjI4ivh11T%2Bt%2FXiEw%3D%3D&SigAlg=http%3A%2F%2Fwww.w3.org%2F2000%2F09%2Fxmldsig%23rsa-sha1&Signature=RRcmtS0Qku3Y5HfSQYKax07dZOnb%2BCvvtP72v6ws%2FkttG4DYaNkRy0YmgDjrSLiVNooJe69LMb3pnHsW2PN7pKKe1p%2FTroGfk%2F7BW%2FQRgl3jvnLeM19qvhb3WYgwmeEXWvuX2rhopGCF2bVuzz9UDPZEDTQ6Botq39QkNTYfn1EpTlGHGQ7k%2Ff9l2rX8DoVOu%2Bv9Ov%2BPIUd0wSYiT1Nw%2FNv2%2FxoMP%2BjDQCfU%2Fx%2BD01wcInwlceaIKkYNb%2FVX3e9E3GzhOljJiUEVA7t9CEDMOACgfhiof9m%2F70txwhxGWPzE0XrBIIpuGp8NPU9QaiVDymbd9wzDVtll6abpx2t14A%3D%3D"
 }
diff --git a/test/static/sp_slo_redirect_failure.json b/test/static/sp_slo_redirect_failure.json
@@ -2,5 +2,6 @@
   "SAMLResponse": "fZHLasMwEEV/xWif6OG8LBxDaTaBdJOELLoJY2taG2zJ9YxLP7+229IUSkAbaebcezWTEjR1aw/hNfR8RGqDJ4w+mtqTnUpb0XfeBqCKrIcGyXJhTw9PB2vmyrZd4FCEWtwg9wkgwo6r4EW0323FdaVUskjUZukSwNy4TYxKxwix0casTbHG1WaRK8QXEV2wo4HcikFowIl63Hti8Dw8Kb2ZqcVM67Ne2tgM51lEOySuPPBElcytlbJy7bx+L+cN2qVSSkLPpRyDS2CS3CE44uBxbLxSHQYn/zOZcxgirxOXa1yD06YA7VYxLnORpaOEnUJ12bdVHQqoy0BsjRqtxhYz6soGGRwwzNuyTeUtmn5t5MTAPf29PQaH0QXqHu/PmKZue8S3fvg/dkJmXx6/svK/vWef",
   "SigAlg": "http://www.w3.org/2000/09/xmldsig#rsa-sha1",
   "Signature": "gS9wWeutxU9nUplBYaJdOkP3vxSyyyE6HlHGBBIbLYYJVzZMVHmpeR1c4z0EirUySg1XOrlWcJxwt9YpDQue2e/XExIokc9EcgiAKNAAshYeewLfM6VGxn48rpkU4rE35kiw1Jiydm/cx/JfatBYc1YT74TyjwckNZ/pj08+7ILI6rTESkkBRfACZZ7zzFL+K41OiiKcgs3TZNP4GfalzrXClQ5eELA1NG3JfxkzUTwApjkNCTVSBQcWRa/nyko+PDKPVG1U7dEe16csYfae2u9EkP81seQqgAPsqOyrASQtuNqf+vZhN9aCb25Q4X26soA0QDSzegNb82TdifhhDQ==",
-  "RelayState": "http://idp.lvh.me:5000"
+  "RelayState": "http://idp.lvh.me:5000",
+  "originalQuery": "SAMLResponse=fZHLasMwEEV%2FxWif6OG8LBxDaTaBdJOELLoJY2taG2zJ9YxLP7%2B229IUSkAbaebcezWTEjR1aw%2FhNfR8RGqDJ4w%2BmtqTnUpb0XfeBqCKrIcGyXJhTw9PB2vmyrZd4FCEWtwg9wkgwo6r4EW0323FdaVUskjUZukSwNy4TYxKxwix0casTbHG1WaRK8QXEV2wo4HcikFowIl63Hti8Dw8Kb2ZqcVM67Ne2tgM51lEOySuPPBElcytlbJy7bx%2BL%2BcN2qVSSkLPpRyDS2CS3CE44uBxbLxSHQYn%2FzOZcxgirxOXa1yD06YA7VYxLnORpaOEnUJ12bdVHQqoy0BsjRqtxhYz6soGGRwwzNuyTeUtmn5t5MTAPf29PQaH0QXqHu%2FPmKZue8S3fvg%2FdkJmXx6%2FsvK%2FvWef&RelayState=http%3A%2F%2Fidp.lvh.me%3A5000&SigAlg=http%3A%2F%2Fwww.w3.org%2F2000%2F09%2Fxmldsig%23rsa-sha1&Signature=gS9wWeutxU9nUplBYaJdOkP3vxSyyyE6HlHGBBIbLYYJVzZMVHmpeR1c4z0EirUySg1XOrlWcJxwt9YpDQue2e%2FXExIokc9EcgiAKNAAshYeewLfM6VGxn48rpkU4rE35kiw1Jiydm%2Fcx%2FJfatBYc1YT74TyjwckNZ%2Fpj08%2B7ILI6rTESkkBRfACZZ7zzFL%2BK41OiiKcgs3TZNP4GfalzrXClQ5eELA1NG3JfxkzUTwApjkNCTVSBQcWRa%2Fnyko%2BPDKPVG1U7dEe16csYfae2u9EkP81seQqgAPsqOyrASQtuNqf%2BvZhN9aCb25Q4X26soA0QDSzegNb82TdifhhDQ%3D%3D"
 }