FAB-1263 ECDSA signature malleability resistance

This change-set introduces ECDSA Signature malleability resistance.
ECDSA signatures do not have unique representation and this can facilitate
replay attacks and more. In order to have a unique representation,
this change-set forses BCCSP to generate and accept only signatures
with low-S.

Bitcoin has also addressed this issue with the following BIP:
https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki
Before merging this change-set, we need to ensure that client-sdks
generates signatures properly in order to avoid massive rejection
of transactions.

This is a port of the GO implementation here:
https://gerrit.hyperledger.org/r/#/c/2983

This changeset has been successfully tested with 2983.

Change-Id: Iee78ee93f83ddfdd99526ea3cca9c11b33af8318
Signed-off-by: Jim Zhang <jzhang@us.ibm.com>

Author: jimthematrix

hfc/lib/impl/CryptoSuite_ECDSA_AES.js

@@ -22,19 +22,14 @@ var api = require('../api.js');
 var crypto = require('crypto');
 var elliptic = require('elliptic');
 var EC = elliptic.ec;
-var sjcl = require('sjcl');
 var jsrsa = require('jsrsasign');
 var KEYUTIL = jsrsa.KEYUTIL;
 var util = require('util');
 var hashPrimitives = require('../hash.js');
 var utils = require('../utils');
 var ECDSAKey = require('./ecdsa/key.js');
-
-// constants
-const AESKeyLength = 32;
-const HMACKeyLength = 32;
-const ECIESKDFOutput = 512; // bits
-const IVLength = 16; // bytes
+var BN = require('bn.js');
+var Signature = require('elliptic/lib/elliptic/ec/signature.js');
 
 var logger = utils.getLogger('crypto_ecdsa_aes');
 
@@ -236,6 +231,7 @@ var CryptoSuite_ECDSA_AES = class extends api.CryptoSuite {
 		// module './ecdsa/key.js'
 		var signKey = this._ecdsa.keyFromPrivate(key._key.prvKeyHex, 'hex');
 		var sig = this._ecdsa.sign(digest, signKey);
+		sig = _preventMalleability(sig, key._key.ecparams);
 		logger.debug('ecdsa signature: ', sig);
 		return sig.toDER();
 	}
@@ -258,6 +254,11 @@ var CryptoSuite_ECDSA_AES = class extends api.CryptoSuite {
 			throw new Error('A valid message is required to verify');
 		}
 
+		if (!_checkMalleability(signature, key._key.ecparams)) {
+			logger.error(new Error('Invalid S value in signature. Must be smaller than half of the order.').stack);
+			return false;
+		}
+
 		var pubKey = this._ecdsa.keyFromPublic(key.getPublicKey()._key.pubKeyHex, 'hex');
 		// note that the signature is generated on the hash of the message, not the message itself
 		return pubKey.verify(this.hash(digest), signature);
@@ -282,10 +283,58 @@ var CryptoSuite_ECDSA_AES = class extends api.CryptoSuite {
 	}
 };
 
-function _zeroBuffer(length) {
-	var buf = new Buffer(length);
-	buf.fill(0);
-	return buf;
+// [Angelo De Caro] ECDSA signatures do not have unique representation and this can facilitate
+// replay attacks and more. In order to have a unique representation,
+// this change-set forses BCCSP to generate and accept only signatures
+// with low-S.
+// Bitcoin has also addressed this issue with the following BIP:
+// https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki
+// Before merging this change-set, we need to ensure that client-sdks
+// generates signatures properly in order to avoid massive rejection
+// of transactions.
+
+// map for easy lookup of the "N/2" value per elliptic curve
+const halfOrdersForCurve = {
+	'secp256r1': elliptic.curves['p256'].n.shrn(1),
+	'secp384r1': elliptic.curves['p384'].n.shrn(1)
+};
+
+function _preventMalleability(sig, curveParams) {
+	var halfOrder = halfOrdersForCurve[curveParams.name];
+	if (!halfOrder) {
+		throw new Error('Can not find the half order needed to calculate "s" value for immalleable signatures. Unsupported curve name: ' + curve);
+	}
+
+	// in order to guarantee 's' falls in the lower range of the order, as explained in the above link,
+	// first see if 's' is larger than half of the order, if so, it needs to be specially treated
+	if (sig.s.cmp(halfOrder) == 1) { // module 'bn.js', file lib/bn.js, method cmp()
+		// convert from BigInteger used by jsrsasign Key objects and bn.js used by elliptic Signature objects
+		var bigNum = new BN(curveParams.n.toString(16), 16);
+		sig.s = bigNum.sub(sig.s);
+	}
+
+	return sig;
+}
+
+function _checkMalleability(sig, curveParams) {
+	var halfOrder = halfOrdersForCurve[curveParams.name];
+	if (!halfOrder) {
+		throw new Error('Can not find the half order needed to calculate "s" value for immalleable signatures. Unsupported curve name: ' + curve);
+	}
+
+	// first need to unmarshall the signature bytes into the object with r and s values
+	var sigObject = new Signature(sig, 'hex');
+	if (!sigObject.r || !sigObject.s) {
+		throw new Error('Failed to load the signature object from the bytes.');
+	}
+
+	// in order to guarantee 's' falls in the lower range of the order, as explained in the above link,
+	// first see if 's' is larger than half of the order, if so, it is considered invalid in this context
+	if (sigObject.s.cmp(halfOrder) == 1) { // module 'bn.js', file lib/bn.js, method cmp()
+		return false;
+	}
+
+	return true;
 }
 
 module.exports = CryptoSuite_ECDSA_AES;

package.json

@@ -16,7 +16,9 @@
   "engine-strict": true,
   "engineStrict": true,
   "devDependencies": {
+    "bn.js": "^4.11.6",
     "bunyan": "^1.8.1",
+    "elliptic": "^6.3.2",
     "gulp": "^3.9.1",
     "gulp-debug": "^3.0.0",
     "gulp-eslint": "^3.0.1",

test/unit/headless-tests.js

@@ -1243,6 +1243,14 @@ var asn1 = jsrsa.asn1;
 
 var ecdsaKey = require('hfc/lib/impl/ecdsa/key.js');
 var api = require('hfc/lib/api.js');
+var elliptic = require('elliptic');
+var BN = require('bn.js');
+var Signature = require('elliptic/lib/elliptic/ec/signature.js');
+
+const halfOrdersForCurve = {
+	'secp256r1': elliptic.curves['p256'].n.shrn(1),
+	'secp384r1': elliptic.curves['p384'].n.shrn(1)
+};
 
 test('\n\n ** CryptoSuite_ECDSA_AES - function tests **\n\n', function (t) {
 	resetDefaults();
@@ -1369,7 +1377,14 @@ test('\n\n ** CryptoSuite_ECDSA_AES - function tests **\n\n', function (t) {
 			var testSignature = function (msg) {
 				var sig = cryptoUtils.sign(key, cryptoUtils.hash(msg));
 				if (sig) {
-					t.pass('Valid signature object generated from sign()');
+					// test that signatures have low-S
+					var halfOrder = halfOrdersForCurve[key._key.ecparams.name];
+					var sigObject = new Signature(sig);
+					if (sigObject.s.cmp(halfOrder) == 1) {
+						t.fail('Invalid signature object: S value larger than N/2');
+					} else {
+						t.pass('Valid signature object generated from sign()');
+					}
 
 					// using internal calls to verify the signature
 					var pubKey = cryptoUtils._ecdsa.keyFromPublic(key.getPublicKey()._key.pubKeyHex, 'hex');
@@ -1412,20 +1427,21 @@ test('\n\n ** CryptoSuite_ECDSA_AES - function tests **\n\n', function (t) {
 			utils.setConfigSetting('crypto-hash-algo', 'SHA2');
 			cryptoUtils = utils.getCryptoSuite();
 
-			var testVerify = function (sig, msg) {
+			var testVerify = function (sig, msg, expected) {
 				// manually construct a key based on the saved privKeyHex and pubKeyHex
 				var f = new ECDSA({ curve: 'secp256r1' });
 				f.setPrivateKeyHex(TEST_KEY_PRIVATE);
 				f.setPublicKeyHex(TEST_KEY_PUBLIC);
 				f.isPrivate = true;
 				f.isPublic = false;
 
-				t.equal(cryptoUtils.verify(new ecdsaKey(f, 256), sig, msg), true,
+				t.equal(cryptoUtils.verify(new ecdsaKey(f, 256), sig, msg), expected,
 					'CryptoSuite_ECDSA_AES function tests: verify() method');
 			};
 
-			testVerify(TEST_MSG_SIGNATURE_SHA2_256, TEST_MSG);
-			testVerify(TEST_LONG_MSG_SIGNATURE_SHA2_256, TEST_LONG_MSG);
+			// these signatures have S values larger than N/2
+			testVerify(TEST_MSG_SIGNATURE_SHA2_256, TEST_MSG, false);
+			testVerify(TEST_LONG_MSG_SIGNATURE_SHA2_256, TEST_LONG_MSG, false);
 
 			// test importKey()
 			var pubKey = cryptoUtils.importKey(TEST_CERT_PEM, { algorithm: api.CryptoAlgorithms.X509Certificate });