Skip to content

Commit 02c6c5a

Browse files
mhassan1markstos
authored andcommitted
normalize line endings before signature validation
1 parent b349e4b commit 02c6c5a

File tree

2 files changed

+24
-3
lines changed

2 files changed

+24
-3
lines changed

src/passport-saml/saml.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -752,6 +752,7 @@ class SAML {
752752
if (totalReferencedNodes.length > 1) {
753753
return false;
754754
}
755+
fullXml = fullXml.replace(/\r\n?/g, '\n');
755756
return sig.checkSignature(fullXml);
756757
}
757758

test/test-signatures.js

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,26 @@ describe('Signatures', function() {
1717
done(ex);
1818
}
1919
},
20-
testOneResponse = ( pathToXml, shouldErrorWith, amountOfSignatureChecks = 1 ) => {
20+
testOneResponseBody = ( samlResponseBody, shouldErrorWith, amountOfSignatureChecks = 1 ) => {
2121
return done => {
2222
//== Instantiate new instance before every test
2323
const samlObj = new SAML({ cert });
2424
//== Spy on `validateSignature` to be able to count how many times it has been called
2525
const validateSignatureSpy = sinon.spy(samlObj, 'validateSignature');
2626

27-
//== Create a body bases on an XML an run the test in `func`
28-
samlObj.validatePostResponse(createBody(pathToXml), tryCatchTest(done, function( error ) {
27+
//== Run the test in `func`
28+
samlObj.validatePostResponse(samlResponseBody, tryCatchTest(done, function( error ) {
2929
//== Assert error. If the error is `SAML assertion expired` we made it past the certificate validation
3030
shouldErrorWith ? error.should.eql(new Error(shouldErrorWith)) : error.should.eql(new Error('SAML assertion expired'));
3131
//== Assert times `validateSignature` was called
3232
validateSignatureSpy.callCount.should.eql(amountOfSignatureChecks);
3333
done();
3434
}));
3535
};
36+
},
37+
testOneResponse = ( pathToXml, ...args ) => {
38+
//== Create a body based on an XML and run the test
39+
return testOneResponseBody(createBody(pathToXml), ...args);
3640
};
3741

3842
describe('Signatures on saml:Response - Only 1 saml:Assertion', () => {
@@ -80,4 +84,20 @@ describe('Signatures', function() {
8084

8185
});
8286

87+
describe('Signature on saml:Response with non-LF line endings', () => {
88+
const samlResponseXml = fs.readFileSync(__dirname + '/static/signatures/valid/response.root-signed.assertion-signed.xml').toString();
89+
const makeBody = str => ({ SAMLResponse: Buffer.from(str).toString('base64') });
90+
91+
it('CRLF line endings', done => {
92+
const body = makeBody(samlResponseXml.replace(/\n/g, '\r\n'));
93+
testOneResponseBody(body, false, 1)(done);
94+
});
95+
96+
it('CR line endings', done => {
97+
const body = makeBody(samlResponseXml.replace(/\n/g, '\r'));
98+
testOneResponseBody(body, false, 1)(done);
99+
});
100+
101+
});
102+
83103
});

0 commit comments

Comments
 (0)