lib: add diagnostics_channel events to cipher and hashing algorithms

[uurien]

Adding crypto.cipher.new, crypto.decipher.new, crypto.cipheriv.new, crypto.decipheriv.new, crypto.hash.new, crypto.hash.copy, crypto.hmac.new, crypto.sign.new and crypto.verify.new diagnostics_channel events so APMs don't need to patch the crypto module to know when a particular crypto algorithm is used.

[nodejs-github-bot]

Review requested:

• @nodejs/crypto

[tniessen]

so APMs don't need to patch the crypto module to know when a particular crypto algorithm is used

Is this a common thing?

[tniessen]

Cipher and Decipher should not be used; I don't think it is beneficial to introduce any sort of monitoring for these APIs.

[uurien]

I wasn't very sure about it.
But as cipher and decipher are deprecated, createCipher and createDecipher methods are deprecated and defined like that, it is not easy to patch.

ObjectDefineProperties(module.exports, {
  createCipher: {
    __proto__: null,
    enumerable: false,
    value: deprecate(createCipher,
                     'crypto.createCipher is deprecated.', 'DEP0106')
  }

But, if the rule is "not modify deprecated methods", I can delete this part of the code

[tniessen]

But, if the rule is "not modify deprecated methods", I can delete this part of the code

There is no such rule; I just think adding complexity to a feature that nobody should be using might be the wrong approach.

[simon-id]

Don't forget to add it to the docs here: https://nodejs.org/api/diagnostics_channel.html#built-in-channels

[uurien]

@tniessen

so APMs don't need to patch the crypto module to know when a particular crypto algorithm is used

Is this a common thing?

For example, if the product want to detect the used cypher/hashing algorithms, for example, to detect the uses of the insecure sha1 algorithm.

[tniessen]

For example, if the product want to detect the used cypher/hashing algorithms, for example, to detect the uses of the insecure sha1 algorithm.

That seems difficult; createHash is just one of many APIs that allow specifying hash algorithms. For example, that "product" you are describing would also have to check OAEP hash algorithms etc. Is the goal to add such events to all cryptographic operations, and to provide all user inputs to the observers?

[Qard]

Some lint errors due to missing semicolons, but other than that LGTM.

[uurien]

@tniessen yes, you are right, thanks
I'm going to review again the crypto module to find all ways that the user have to hash or cipher and reopen this PR (I'm moving it to draft now)

[tniessen]

Just to be clear, I am not saying that adding such events to all crypto operations is a good idea. I am just wondering what the goal is here.

I assume that most cryptographic vulnerabilities arise from design issues or misuse of cryptographic APIs or mechanisms, not from selecting weak algorithms.