Skip to content

Commit

Permalink
[SECURITY] Disable HMAC sig methods by default due to key confusion
Browse files Browse the repository at this point in the history
Both HMAC and public key digital signature methods use the same
variable to provide the key, and the choice of algorithm is provided
by user input. This can result in a security vulnerability. Since
HMAC-SHA1 is less commonly use, disable it by default to prevent
this issue. If you use it, you can re-enable it by calling
SignedXml.enableHMAC().

Since this is a breaking change to the API, update major version
number to 2.0.0
  • Loading branch information
bawolff committed Oct 5, 2020
1 parent d295ecc commit 3d9db71
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 3 deletions.
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,18 @@ A pre requisite it to have [openssl](http://www.openssl.org/) installed and its
* RSA-SHA1 http://www.w3.org/2000/09/xmldsig#rsa-sha1
* RSA-SHA256 http://www.w3.org/2001/04/xmldsig-more#rsa-sha256
* RSA-SHA512 http://www.w3.org/2001/04/xmldsig-more#rsa-sha512

HMAC-SHA1 is also available but it is disabled by default
* HMAC-SHA1 http://www.w3.org/2000/09/xmldsig#hmac-sha1

to enable HMAC-SHA1, do:
```javascript
require( 'xml-crypto' ).SignedXml.enableHMAC();
```
This will enable HMAC and disable digital signature algorithms. Due to key
confusion issues, it is risky to have both HMAC-based and public key digital
signature algorithms enabled at same time.

by default the following algorithms are used:

*Canonicalization/Transformation Algorithm:* Exclusive Canonicalization http://www.w3.org/2001/10/xml-exc-c14n#
Expand Down
14 changes: 13 additions & 1 deletion lib/signed-xml.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,19 @@ SignedXml.SignatureAlgorithms = {
'http://www.w3.org/2000/09/xmldsig#rsa-sha1': RSASHA1,
'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256': RSASHA256,
'http://www.w3.org/2001/04/xmldsig-more#rsa-sha512': RSASHA512,
'http://www.w3.org/2000/09/xmldsig#hmac-sha1': HMACSHA1
// Disabled by default due to key confusion concerns.
// 'http://www.w3.org/2000/09/xmldsig#hmac-sha1': HMACSHA1
}

/**
* Due to key-confusion issues, its risky to have both hmac
* and digital signature algos enabled at the same time.
* This enables HMAC and disables other signing algos.
*/
SignedXml.enableHMAC = function () {
SignedXml.SignatureAlgorithms = {
'http://www.w3.org/2000/09/xmldsig#hmac-sha1': HMACSHA1
}
}

SignedXml.defaultNsForPrefix = {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "xml-crypto",
"version": "1.5.3",
"version": "2.0.0",
"description": "Xml digital signature and encryption library for Node.js",
"engines": {
"node": ">=0.4.0"
Expand Down
4 changes: 3 additions & 1 deletion test/hmac-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ var xpath = require('xpath');
var xmldom = require('xmldom');
var fs = require('fs');

crypto.SignedXml.enableHMAC()

exports['test validating HMAC signature'] = function (test) {
var xml = fs.readFileSync('./test/static/hmac_signature.xml', 'utf-8');
var doc = new xmldom.DOMParser().parseFromString(xml);
Expand Down Expand Up @@ -49,4 +51,4 @@ exports['test create and validate HMAC signature'] = function (test) {
test.equal(result, true);

test.done();
};
};

0 comments on commit 3d9db71

Please sign in to comment.