hyperledger
Type mismatch between params in the SigningIdentity sign and CryptoSuite hash.
The SigningIdentity has the following code and documentation:
/**
* Signs digest with the private key contained inside the signer.
*
* @param {byte[]} msg The message to sign
* @param {Object} opts Options object for the signing, contains one field 'hashFunction' that allows
* different hashing algorithms to be used. If not present, will default to the hash function
* configured for the identity's own crypto suite object
*/
sign(msg, opts) {
// calculate the hash for the message before signing
let hashFunction;
if (opts && opts.hashFunction) {
if (typeof opts.hashFunction !== 'function') {
throw new Error('The "hashFunction" field must be a function');
}
hashFunction = opts.hashFunction;
} else {
hashFunction = this._cryptoSuite.hash.bind(this._cryptoSuite);
}
const digest = hashFunction(msg);
return this._signer.sign(Buffer.from(digest, 'hex'), null);
}
I think that there are two main problems here related to the documentation. It says that it signs the digest with the private key, actually, the function signs the message (calculating the digest inside). Another issue here is the typing in the msg param, the function uses a hashFunction (defaults to the CryptoSuite) but the CryptoSuite hash function requires a string msg instead of a Buffer. This causes two main issues:
- Mislead the developer of thinking the msg should be a digest due to the type and text.
- Make it harder to use for typescript users.
If I am not looking this wrong, I can open the PR to update the documentation. Thanks!
You are correct, there do look to be some issues here!
I think that there are two main problems here related to the documentation. It says that it signs the digest with the private key, actually, the function signs the message (calculating the digest inside).
Correct. This function signs the message with the private key.
Another issue here is the typing in the msg param, the function uses a hashFunction (defaults to the CryptoSuite) but the CryptoSuite hash function requires a string msg instead of a Buffer.
The JSDoc for SigningIdentity says the message is a byte[]. This isn't really a Type/JavaScript type. The TypeScript definition in fabric-common/type/index.d.ts says it's a Buffer. Probably the JSDoc and typings should be consistent.
As you say, whatever the message is, it's going to be hashed by the CryptoSuite, and the typings and JSDoc for that say it's a string.
Digging down to the default hash implementation in fabric-common/lib/hash/hash_sha2_256.js, it uses Node crypto Hash to hash the message, so it will actually work for a UTF-8 string, Buffer, TypedArray or DataView. And it really does return a hex-encoded string. I think Buffer is a pretty reasonable type for representing arbitrary byte data, and it's consistent with the rest of the API, but changing the type definition for CryptoSuite.hash() from string (which is valid) to Buffer (which is also valid for the default implementation) does run the risk of breaking existing code that passes strings. I actually see some code in FabricCAClient that does pass a string to the SigningIdentity.sign() function, although there are other cases where it is looks to be passed a Uint8Array!
If I am not looking this wrong, I can open the PR to update the documentation. Thanks!
There is definitely some tidy-up that could be done safely, but changing the typings seems to be a compatibility minefield here. I don't mind standardising on a sensible type (like Buffer) in the main branch. This would probably need to be done consistently at all the points that use these functions throughout the codebase, and might require some code changes to make sure all the code really is doing the right thing.
Making changes in the release-2.2 branch seems too risky as the current typings are valid, and changes might break existing client code.
@bestbeforetoday BTW, when ca, peer and orderer use sha3-384, how can I set for ca client using node sdk?
@bh4rtp I think you would need to either:
- Set the key size and hash algorithm using the
crypto-keysizeandcrypto-hash-algoconfig properties respectively; or - Explicitly create the CryptoSuite implementation (passing appropriate key size and hash algorithm) and pass that to the FabricCAServices constructor.
@bestbeforetoday Does sdk support SHA3-384 when using softhsm2?
const hsmOptions = {
lib: await findSoftHSMPKCS11Lib(),
pin: process.env.PKCS11_PIN || pkcs11Pin,
label: process.env.PKCS11_LABEL || pkcs11Label,
keysize: 384,
algorithm: 'SHA3',
};
const hsmProvider = new HsmX509Provider(hsmOptions);
I got this error: Failed to get HSM provider : Error: Desired CryptoSuite module not found supporting algorithm "SHA3"
When using SW BCCSP with SHA3 384, everything runs ok. If set
"crypto-hash-algo": "SHA3",
"crypto-keysize": 384,
client will fail: 2023-04-21T01:49:37.431Z - error: [DiscoveryResultsProcessor]: parseDiscoveryResults[democh] - Channel:democh received discovery error:access denied Error in transaction: Error: DiscoveryService: democh error: access denied
If you are passing explicit options to the HsmX509Provider, the algorithm option should be EC. A hash option should be SHA3. I think if the options are omitted then it should use whatever is configured in the config.
@bestbeforetoday I wonder why node sdk app runs ok without any option configuration if peer and orderer using SHA3 384. But if I change options to SHA3 384, it will fail.
It sounds like one end is not using the expected hash and, since when the client is using the default SHA-256 things seems to work, that suggests the issue might be at the peer end. It might be worth capturing the message before the point of failure and verifying the signature yourself to confirm which end the problem is occurring.
@bestbeforetoday I have reported an issue in fabric: hyperledger/fabric#4179
Since this package is now deprecated in favour of @hyperledger/fabric-gateway, I do not plan to make any enhancements to function signatures / typing, particularly in the low-level APIs.