Include option to check SAML certificate expiration (#96)

* add certificate expiration checking as an option (off by default) Drops Node4 support as well, though it was already failing on Node4 in master

changes per machuga comments

changes per machuga comments

remove separate supported-versions list for saml

* restore test we lost in bad squash

* change some vars to lets and camel to snake case in tests

* add test to ensure we can allow expired certs if we choose to

* remove unneeded comment from tests

.travis.yml

@@ -1,5 +1,4 @@
 language: node_js
 before_install: npm i -g npm@1.4.28
 node_js:
-  - 4
   - 6

README.md

@@ -122,7 +122,7 @@ If you have found a bug or if you have a feature request, please report them at
 
 ## Security Notice
 
-The [Security Notice](SECURITY-NOTICE.md) lists the version that is vulnerable and the actions that are required to upgrade to the lastest version.
+The [Security Notice](SECURITY-NOTICE.md) lists the version that is vulnerable and the actions that are required to upgrade to the latest version.
 
 ## Author
 

lib/passport-wsfed-saml2/saml.js

@@ -5,6 +5,7 @@ var cryptiles = require('cryptiles');
 var xpath     = require('xpath');
 var xmlCrypto = require('xml-crypto');
 var EventEmitter = require('events');
+const x509 = require('x509');
 
 var utils = require('./utils');
 var ELEMENT_NODE = 1;
@@ -21,20 +22,15 @@ var SAML = function (options) {
   }
 
   this.options.checkExpiration = (typeof this.options.checkExpiration !== 'undefined') ? this.options.checkExpiration : true;
+  // Note! It would be best to set this to true. But it's defaulting to false so as not to break login for expired certs.
+  this.options.checkCertExpiration = (typeof this.options.checkCertExpiration !== 'undefined') ? this.options.checkCertExpiration : false;
   this.options.checkAudience = (typeof this.options.checkAudience !== 'undefined') ? this.options.checkAudience : true;
   this.options.checkRecipient = (typeof this.options.checkRecipient !== 'undefined') ? this.options.checkRecipient : true;
   this.options.checkNameQualifier = (typeof this.options.checkNameQualifier !== 'undefined') ? this.options.checkNameQualifier : true;
   this.options.checkSPNameQualifier = (typeof this.options.checkSPNameQualifier !== 'undefined') ? this.options.checkSPNameQualifier : true;
   this.eventEmitter = this.options.eventEmitter || new EventEmitter();
 };
 
-SAML.prototype.certToPEM = function (cert) {
-  cert = cert.match(/.{1,64}/g).join('\n');
-  cert = "-----BEGIN CERTIFICATE-----\n" + cert;
-  cert = cert + "\n-----END CERTIFICATE-----\n";
-  return cert;
-};
-
 SAML.prototype.validateSignature = function (xml, options, callback) {
   var self = this;
   xml = utils.parseSamlResponse(xml);
@@ -58,7 +54,7 @@ SAML.prototype.validateSignature = function (xml, options, callback) {
       //If there's no embedded signing cert, use the configured cert through options
       if(!keyInfo || keyInfo.length===0){
         if(!options.cert) throw new Error('options.cert must be specified for SAMLResponses with no embedded signing certificate');
-        return self.certToPEM(options.cert);
+        return utils.certToPEM(options.cert);
       }
 
       //If there's an embedded signature and thumprints are provided check that
@@ -73,14 +69,14 @@ SAML.prototype.validateSignature = function (xml, options, callback) {
 
           // using embedded cert, so options.cert is not used anymore
           delete options.cert;
-          return self.certToPEM(base64cer);
+          return utils.certToPEM(base64cer);
         }
       }
 
       // If there's an embedded signature, but no thumprints are supplied, use options.cert
       // either options.cert or options.thumbprints must be specified so at this point there
       // must be an options.cert
-      return self.certToPEM(options.cert);
+      return utils.certToPEM(options.cert);
     }
   };
 
@@ -97,6 +93,10 @@ SAML.prototype.validateSignature = function (xml, options, callback) {
     return callback(e);
   }
 
+  if (self.options.checkCertExpiration && !self.validateCertExpiration(xml)) {
+    return callback(new Error('The signing certificate is not currently valid.'), null);
+  }
+
   if (!valid) {
     return callback(new Error('Signature check errors: ' + sig.validationErrors));
   }
@@ -119,7 +119,21 @@ SAML.prototype.validateSignature = function (xml, options, callback) {
   }
 };
 
+SAML.prototype.validateCertExpiration = function (validatedSamlAssertion) {
+  // This accepts a validated SAML assertion and checks current time against the valid cert dates
+  const certNodes = validatedSamlAssertion.getElementsByTagName("X509Certificate");
+  // Use cert embedded in the assertion if available. If not, use cert included in options passed to SAML constructor
+  const cert = certNodes.length > 0 ? certNodes[0].textContent : this.options.cert;
+  const parsedCert = x509.parseCert(utils.certToPEM(cert));
+
+  const nowDate = new Date();
+
+  // true if current date is before expiry AND after cert start date
+  return (nowDate > parsedCert.notBefore && nowDate < parsedCert.notAfter);
+};
+
 SAML.prototype.validateExpiration = function (samlAssertion, version) {
+
   var conditions = xpath.select(".//*[local-name(.)='Conditions']", samlAssertion);
   if (!conditions || conditions.length === 0) return true;
 
@@ -322,14 +336,11 @@ SAML.prototype.parseAssertion = function(samlAssertion, callback) {
   if (!samlAssertion.getAttribute)
     samlAssertion = samlAssertion.documentElement;
 
-  var version;
-  if (samlAssertion.getAttribute('MajorVersion') === '1')
-    version = '1.1';
-  else if (samlAssertion.getAttribute('Version') === '2.0')
-    version = '2.0';
-  else
-    return callback(new Error('SAML Assertion version not supported'), null);
-
+  const version = utils.getSamlAssertionVersion(samlAssertion);
+  if (!version){
+    // Note that this assumes any version returned by getSamlAssertionVersion is supported.
+    return callback(new Error('SAML Assertion version not supported, or not defined'), null);
+  }
 
   if (self.options.checkExpiration && !self.validateExpiration(samlAssertion, version)) {
     return callback(new Error('assertion has expired.'), null);

lib/passport-wsfed-saml2/utils.js

@@ -4,6 +4,23 @@ var SamlAssertionParserError = require('./errors/SamlAssertionParserError');
 var SamlResponseParserError = require('./errors/SamlResponseParserError');
 var WSFederationResultParserError = require('./errors/WSFederationResultParserError');
 
+const CERT_START = "-----BEGIN CERTIFICATE-----\n";
+const CERT_END = "\n-----END CERTIFICATE-----\n";
+
+exports.certToPEM = (cert) => CERT_START + cert.match(/.{1,64}/g).join('\n') + CERT_END;
+
+exports.getSamlAssertionVersion = function(samlAssertion){
+  if (samlAssertion.getAttribute('MajorVersion') === '1') {
+    return '1.1';
+  } else if (samlAssertion.getAttribute('Version') === '2.0'){
+    return '2.0';
+  } else {
+    // In this case the version is undefined, or we weren't able to determine it.
+    return undefined;
+  }
+
+}
+
 exports.parseSamlAssertion = function(xml) {
   if (typeof xml === 'string') {
     try {