Skip to content

Commit

Permalink
Support multiple and dynamic signing certificates (#218)
Browse files Browse the repository at this point in the history
Some organizations roll the signing certificate every year.  During the roll over period the server provides two certificates which may be valid, so when a SAML response is provided it is valid if it is signed with either of those signatures.

This change allows the `cert` to ban an array or a async function which returns a single cert or an array of certs.
  • Loading branch information
richjharris authored and markstos committed Nov 1, 2017
1 parent da829fc commit 631aaa7
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 11 deletions.
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,18 @@ If you have a certificate in the binary DER encoding, you can convert it to the
openssl x509 -inform der -in my_certificate.cer -out my_certificate.pem
````

If the Identity Provider has multiple signing certificates that are valid (such as during the rolling from an old key to a new key and responses signed with either key are valid) then the `cert` configuration key can be an array:

```javascript
cert: [ 'MIICizCCAfQCCQCY8tKaMc0BMjANBgkqh ... W==', 'MIIEOTCCAyGgAwIBAgIJAKZgJdKdCdL6M ... g=' ]
```

The `cert` configuration key can also be a function that receives a callback as argument calls back a possible error and a certificate or array of certificates. This allows the Identity Provider to be polled for valid certificates and the new certificate can be used if it is changed:

```javascript
cert: function(callback) { callback(null,polledCertificates); }
```

## Usage with Active Directory Federation Services

Here is a configuration that has been proven to work with ADFS:
Expand Down
56 changes: 47 additions & 9 deletions lib/passport-saml/saml.js
Original file line number Diff line number Diff line change
Expand Up @@ -461,12 +461,33 @@ SAML.prototype.certToPEM = function (cert) {
return cert;
};

SAML.prototype.certsToCheck = function () {
var self = this;
if (!self.options.cert) {
return Q();
}
if (typeof(self.options.cert) === 'function') {
return Q.nfcall(self.options.cert)
.then(function(certs) {
if (!Array.isArray(certs)) {
certs = [certs];
}
return Q(certs);
});
}
var certs = self.options.cert;
if (!Array.isArray(certs)) {
certs = [certs];
}
return Q(certs);
};

// This function checks that the |currentNode| in the |fullXml| document contains exactly 1 valid
// signature of the |currentNode|.
//
// See https://github.com/bergie/passport-saml/issues/19 for references to some of the attack
// vectors against SAML signature verification.
SAML.prototype.validateSignature = function (fullXml, currentNode, cert) {
SAML.prototype.validateSignature = function (fullXml, currentNode, certs) {
var self = this;
var xpathSigQuery = ".//*[local-name(.)='Signature' and " +
"namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']";
Expand All @@ -476,6 +497,14 @@ SAML.prototype.validateSignature = function (fullXml, currentNode, cert) {
if (signatures.length != 1)
return false;
var signature = signatures[0];
return certs.some(function (certToCheck) {
return self.validateSignatureForCert(signature, certToCheck, fullXml, currentNode);
});
};

// This function checks that the |signature| is signed with a given |cert|.
SAML.prototype.validateSignatureForCert = function (signature, cert, fullXml, currentNode) {
var self = this;
var sig = new xmlCrypto.SignedXml();
sig.keyInfoProvider = {
getKeyInfo: function (key) {
Expand Down Expand Up @@ -538,9 +567,12 @@ SAML.prototype.validatePostResponse = function (container, callback) {
}
})
.then(function() {
return self.certsToCheck();
})
.then(function(certs) {
// Check if this document has a valid top-level signature
var validSignature = false;
if (self.options.cert && self.validateSignature(xml, doc.documentElement, self.options.cert)) {
if (self.options.cert && self.validateSignature(xml, doc.documentElement, certs)) {
validSignature = true;
}

Expand All @@ -557,7 +589,7 @@ SAML.prototype.validatePostResponse = function (container, callback) {
if (assertions.length == 1) {
if (self.options.cert &&
!validSignature &&
!self.validateSignature(xml, assertions[0], self.options.cert)) {
!self.validateSignature(xml, assertions[0], certs)) {
throw new Error('Invalid signature');
}
return self.processValidlySignedAssertion(assertions[0].toString(), inResponseTo, callback);
Expand All @@ -579,7 +611,7 @@ SAML.prototype.validatePostResponse = function (container, callback) {

if (self.options.cert &&
!validSignature &&
!self.validateSignature(decryptedXml, decryptedAssertions[0], self.options.cert))
!self.validateSignature(decryptedXml, decryptedAssertions[0], certs))
throw new Error('Invalid signature');

self.processValidlySignedAssertion(decryptedAssertions[0].toString(), inResponseTo, callback);
Expand Down Expand Up @@ -851,12 +883,18 @@ SAML.prototype.validatePostRequest = function (container, callback) {
return callback(err);
}

// Check if this document has a valid top-level signature
if (self.options.cert && !self.validateSignature(xml, dom.documentElement, self.options.cert)) {
return callback(new Error('Invalid signature'));
}
self.certsToCheck()
.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'));
}

processValidlySignedPostRequest(self, doc, callback);
processValidlySignedPostRequest(self, doc, callback);
})
.fail(function(err) {
callback(err);
})
});
};

Expand Down
Loading

0 comments on commit 631aaa7

Please sign in to comment.