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

X509 swap + dependency upgrades #144

Closed
wants to merge 13 commits into from
38 changes: 38 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
name: Run Test

on:
push:
pull_request:

jobs:
mocha-test:
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [10.x, 12.x, 14.x]
steps:
- name: Use Node Version ${{ matrix.node-version }}
uses: actions/setup-node@v2
with:
node-version: ${{ matrix.node-version }}

- name: Checkout repo
uses: actions/checkout@v2

- name: Get yarn cache directory path
id: yarn-cache-dir-path
run: echo "::set-output name=dir::$(yarn cache dir)"

- uses: actions/cache@v2
with:
path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
key: ${{ runner.os }}-node-${{ hashFiles('**/yarn.lock') }}
restore-keys: |
${{ runner.os }}-node-
- name: Install Dependencies
run: yarn

- name: Run Test
run: yarn test
env:
CI: true
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ node_modules
.DS_Store
.vscode
package-lock.json
yarn.lock
*.log
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
language: node_js
node_js:
- 6
- 8
- 10
- 12
- 14
7 changes: 3 additions & 4 deletions SECURITY-NOTICE.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ maintaining the signature valid. This could allow an authenticated attacker to "

Updated packages are available on npm. To ensure delivery of additional bug fixes moving forward, please make sure your `package.json` file is updated to take patch and minor level updates of our libraries. See below:

```
```json
{
"dependencies": {
"passport-wsfed-saml2": "^3.0.10"
Expand All @@ -20,9 +20,8 @@ This fix patches the library that your application runs, but will not impact you

You can read more details regarding the vulnerability [here](https://auth0.com/docs/security/bulletins/cve-2018-8085).



Security vulnerability details for passport-wsfed-saml2 < 3.0.5

===============================================================

A vulnerability has been discovered in the passport-wsfed-saml2 library affecting versions < 3.0.5. This vulnerability allows an attacker to impersonate another user and potentially elevate their privileges if the SAML identity provider:
Expand All @@ -34,7 +33,7 @@ Developers using the passport-wsfed-saml2 Passport Strategy need to upgrade to t

Updated packages are available on npm. To ensure delivery of additional bug fixes moving forward, please make sure your `package.json` file is updated to take patch and minor level updates of our libraries. See below:

```
```json
{
"dependencies": {
"passport-wsfed-saml2": "^3.0.5"
Expand Down
24 changes: 11 additions & 13 deletions examples/auth0/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,17 @@ passport.use(new Strategy(
var app = express();

// configure Express
app.configure(function() {
app.set('views', __dirname + '/views');
app.set('view engine', 'ejs');
app.use(express.logger());
app.use(express.cookieParser());
app.use(express.bodyParser());
app.use(express.methodOverride());
app.use(express.session({ secret: 'keyboard cat' }));
app.use(passport.initialize());
app.use(passport.session());
app.use(app.router);
app.use(express.static(__dirname + '/../../public'));
});
app.set('views', __dirname + '/views');
app.set('view engine', 'ejs');
app.use(express.logger());
app.use(express.cookieParser());
app.use(express.bodyParser());
app.use(express.methodOverride());
app.use(express.session({ secret: 'keyboard cat' }));
app.use(passport.initialize());
app.use(passport.session());
app.use(app.router);
app.use(express.static(__dirname + '/../../public'));


app.get('/', function(req, res){
Expand Down
8 changes: 4 additions & 4 deletions lib/passport-wsfed-saml2/saml.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ var crypto = require('crypto');
var xpath = require('xpath');
var xmlCrypto = require('xml-crypto');
var EventEmitter = require('events');
const x509 = require('x509');
const forge = require('node-forge');
const utils = require('./utils');

var ELEMENT_NODE = 1;
Expand Down Expand Up @@ -64,7 +64,7 @@ SAML.prototype.validateSignature = function (xml, options, callback) {
if (embeddedSignature.length > 0) {
var base64cer = embeddedSignature[0].firstChild.toString();
var shasum = crypto.createHash('sha1');
var der = new Buffer(base64cer, 'base64').toString('binary');
var der = new Buffer.from(base64cer, 'base64').toString('binary');
shasum.update(der, 'latin1');
self.calculatedThumbprint = shasum.digest('hex');

Expand Down Expand Up @@ -134,12 +134,12 @@ SAML.prototype.extractAndValidateCertExpiration = function (validatedSamlAsserti

if (!cert) { return false; }

const parsedCert = x509.parseCert(utils.certToPEM(cert));
const parsedCert = forge.pki.certificateFromPem(utils.certToPEM(cert));

const nowDate = new Date();

// true if current date is before expiry AND after cert start date
if ( ! (nowDate > parsedCert.notBefore && nowDate < parsedCert.notAfter)) {
if ( ! (nowDate > parsedCert.validity.notBefore && nowDate < parsedCert.validity.notAfter)) {
this.eventEmitter.emit('certificateExpirationValidationFailed', {});
return false;
}
Expand Down
8 changes: 4 additions & 4 deletions lib/passport-wsfed-saml2/samlp.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,12 @@ Samlp.prototype = {
SAMLRequest = trimXml(sig.getSignedXml());
}

params.SAMLRequest = new Buffer(SAMLRequest).toString('base64');
params.SAMLRequest = new Buffer.from(SAMLRequest).toString('base64');
return callback(null, params);
}

// HTTP-Redirect with deflate encoding (http://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf - section 3.4.4.1)
zlib.deflateRaw(new Buffer(SAMLRequest), function (err, buffer) {
zlib.deflateRaw(new Buffer.from(SAMLRequest), function (err, buffer) {
if (err) return callback(err);

params.SAMLRequest = buffer.toString('base64');
Expand Down Expand Up @@ -263,12 +263,12 @@ Samlp.prototype = {
},

decodeResponse: function(req) {
var decoded = new Buffer(req.body['SAMLResponse'], 'base64').toString(this.default_encoding);
var decoded = new Buffer.from(req.body['SAMLResponse'], 'base64').toString(this.default_encoding);

const encoding = utils.getEncoding(decoded);
if (encoding && encodingMappings[encoding] && encodingMappings[encoding] !== this.default_encoding){
// Encoding defers from the one configured, decode again with the correct value
decoded = new Buffer(req.body['SAMLResponse'], 'base64').toString(encodingMappings[encoding]);
decoded = new Buffer.from(req.body['SAMLResponse'], 'base64').toString(encodingMappings[encoding]);
}

return decoded;
Expand Down
41 changes: 21 additions & 20 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"name": "passport-wsfed-saml2",
"version": "4.1.0",
"version": "4.1.1",
"description": "SAML2 Protocol and WS-Fed library",
"scripts": {
"test": "mocha --reporter spec --recursive"
"test": "mocha --reporter spec --recursive --exit"
},
"author": {
"name": "Matias Woloski",
Expand All @@ -19,34 +19,35 @@
},
"main": "./lib/passport-wsfed-saml2",
"dependencies": {
"ejs": "2.5.5",
"jsonwebtoken": "~5.0.4",
"ejs": "3.1.5",
"jsonwebtoken": "~8.5.1",
"node-forge": "^0.10.0",
"passport-strategy": "^1.0.0",
"uid2": "0.0.x",
"valid-url": "^1.0.9",
"x509": "^0.3.4",
"xml-crypto": "auth0/xml-crypto#v1.4.1-auth0.2",
"xml-encryption": "auth0/node-xml-encryption#v0.12.0",
"xml2js": "0.1.x",
"xml-crypto": "2.0.0",
"xml-encryption": "1.2.1",
"xml2js": "0.4.23",
"xmldom": "auth0/xmldom#v0.1.19-auth0.2",
"xpath": "0.0.5",
"xtend": "~2.0.3"
"xpath": "0.0.32",
"xtend": "~4.0.2"
},
"devDependencies": {
"chai": "2.x.x",
"body-parser": "^1.19.0",
"chai": "4.x.x",
"chai-passport-strategy": "1.x.x",
"cheerio": "~0.19.0",
"express": "~3.11.0",
"mocha": "~1.8.1",
"passport": "^0.3.2",
"cheerio": "~1.0.0-rc.5",
"express": "~4.17.1",
"mocha": "~8.2.0",
"passport": "^0.4.1",
"request": "~2.88.0",
"saml": "~0.4.4",
"samlp": "~0.4.3",
"should": "~1.1.0",
"wsfed": "~0.3.5"
"saml": "~1.0.0",
"samlp": "~3.4.2",
"should": "~13.2.3",
"wsfed": "~6.0.0"
},
"engines": {
"node": ">= 4"
"node": ">= 10"
},
"licenses": [
{
Expand Down
17 changes: 8 additions & 9 deletions test/fixture/samlp-server.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var bodyParser = require('body-parser');
var express = require('express');
var http = require('http');
var samlp = require('samlp');
Expand Down Expand Up @@ -295,14 +296,12 @@ module.exports.start = function(options, callback){

var app = express();

app.configure(function(){
this.use(express.bodyParser());
this.use(passport.initialize());
this.use(passport.session());
this.use(function(req,res,next){
req.user = fakeUser;
next();
});
app.use(bodyParser.urlencoded({ extended: false }));
app.use(passport.initialize());
app.use(passport.session());
app.use(function(req,res,next){
req.user = fakeUser;
next();
});

function getPostURL (audience, samlRequestDom, req, callback) {
Expand Down Expand Up @@ -407,7 +406,7 @@ module.exports.start = function(options, callback){
app.post('/callback/samlp-with-invalid-xml',
function (req, res, next) {
passport.authenticate('samlp-with-utf8', { protocol: 'samlp' }, function(err, user, info) {
res.send(400, { message: err.message });
res.status(400).json({ message: err.message });
})(req, res, next);
},
function(req, res) {
Expand Down
22 changes: 12 additions & 10 deletions test/fixture/wsfed-server.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var bodyParser = require('body-parser');
var express = require('express');
var http = require('http');
var wsfed = require('wsfed');
Expand Down Expand Up @@ -59,14 +60,12 @@ module.exports.start = function(options, callback){

var app = express();

app.configure(function(){
this.use(express.bodyParser());
this.use(passport.initialize());
this.use(passport.session());
this.use(function(req,res,next){
req.user = fakeUser;
next();
});
app.use(bodyParser.urlencoded({ extended: false }));
app.use(passport.initialize());
app.use(passport.session());
app.use(function(req,res,next){
req.user = fakeUser;
next();
});

function getPostURL (wtrealm, wreply, req, callback) {
Expand All @@ -84,7 +83,7 @@ module.exports.start = function(options, callback){
app.post('/callback/wresult-with-invalid-xml',
function (req, res, next) {
passport.authenticate('wsfed-saml2', function(err, user, info) {
res.send(400, { message: err.message });
res.status(400).json({ message: err.message });
})(req, res, next);
},
function(req, res) {
Expand All @@ -99,7 +98,10 @@ module.exports.start = function(options, callback){
});

var server = http.createServer(app).listen(5050, callback);
module.exports.close = server.close.bind(server);
module.exports.close = function(callback) {
server.close.bind(server);
callback();
}
};

module.exports.fakeUser = fakeUser;
Expand Down
10 changes: 5 additions & 5 deletions test/interop.tests.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading