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

Support multiple and dynamic signing certificates #218

Merged
merged 4 commits into from
Nov 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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