benchmark: replace deprecated cipher in cipher-stream.js

[gengjiawen]

I am not quite confident in my crypto knowledge. @BridgeAR Can you help to review this ? Thanks.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

[mscdex]

Be aware that any (valid) cipher name can be passed in, not just the two predefined ones currently in the benchmark file.

[gengjiawen]

Be aware that any (valid) cipher name can be passed in, not just the two predefined ones currently in the benchmark file.

I am not fully understand what's your meaning.Is this PR okay or should I do something ? Can you elaborate on this, thanks.

[mscdex]

@gengjiawen What happens when I pass in cipher=aes128, cipher=blowfish, cipher=rc4, etc.? I think we will first need to expose key and IV length on KeyObject instances to avoid having a massive lookup table in this benchmark file to know the correct values for each possible cipher.

[gengjiawen]

@gengjiawen What happens when I pass in cipher=aes128, cipher=blowfish, cipher=rc4, etc.? I think we will first need to expose key and IV length on KeyObject instances to avoid having a massive lookup table in this benchmark file to know the correct values for each possible cipher.

Can you be more specific how do I improve current code ? Or this change bring too much side effect ?

[mscdex]

I think we probably need some new crypto API exposed that gives useful cipher information like key and block sizes before we can easily modify this test.

[tniessen]

I think we will first need to expose key and IV length on KeyObject instances

You can already access the key length of symmetric keys when wrapped into a KeyObject. However, a KeyObject contains no information about the algorithm, so there is no way to tell what the IV size is.

I think we probably need some new crypto API exposed that gives useful cipher information like key and block sizes before we can easily modify this test.

See e.g., #22304. We have had brief discussions about this before. Such APIs are difficult to design and there is very little need for them outside of our own tests. There is also a security risk involved: Adding such APIs allows people to use arbitrary ciphers interchangeably, at least that's what many will think. In fact, even minor changes can nullify the security of a cryptosystem. aes-256-cbc has completely different security characteristics from, e.g, aes-256-ecb and aes-256-ctr. Currently, people are forced to manually select an appropriate algorithm and parameters. Some parameters have no single correct choice: Some ciphers permit multiple IV sizes (those IVs are usually called nonces), and the security heavily depends on the choice of that parameter. We could return a default value, but how are we going to chose that? Sometimes, the "most secure" parameters have other downsides, e.g. by restricting the maximal permitted size of a message, and defaulting to those parameters would likely cause unexpected Errors at runtime when users unknowingly violate those restrictions.

We could simply return whatever OpenSSL decided to use as default values. But even then, we will have to carefully design an API. Adding more and more functions to crypto doesn't seem like the right approach. If we really want to have a crypto API with all those fancy features, we will need abstractions such as representations of algorithms at runtime. And it won't stop at ciphers, the same applies to hash algorithms, signature algorithms, asymmetric encryption algorithms etc.

[mscdex]

there is very little need for them outside of our own tests

I disagree. Again, there are plenty of use cases where ciphers are variable/user-provided and needing to know technical details like key sizes and block lengths are important, especially since you need to know that information to avoid exceptions being thrown by node. If you get an invalid IV length exception for example for some arbitrary, valid cipher name provided by the user, what do you do? Keep passing different-sized IVs until the error goes away to find an acceptable length? That's not particularly user friendly.

For example, in ssh2-streams I've had to maintain my own lookup table that stores this and other kinds of information about each cipher. It would be better if I didn't have to do this and I could just query OpenSSL for this same information instead. Others have pointed out other use cases as well in the issue you linked to.

This has nothing at all to do with reduced security, it's merely about exposing information about ciphers that OpenSSL already knows.

[sam-github]

I agree with @mscdex 's comments above, and I have also built lookup tables, as he has, for crypto APIs that don't provide the info (albeit in other languages). Providing info on expected lengths doesn't prevent errors in API usage, but forcing people to build their own tables of cryptogragraphic constants sounds even more fraught with possibilities to make mistakes.

[tniessen]

@mscdex @sam-github I get your point. Let's carefully consider options to make this happen or at least easier. Here are some of my concerns:

If you get an invalid IV length exception for example for some arbitrary, valid cipher name provided by the user, what do you do?

I honestly don't know. If there is a single valid IV length, you are right, it should be determined automatically. But as I said before, sometimes, there is no single correct choice, many ciphers accept multiple IV lengths. In that case, I'd probably ask the user for the IV size; if they know enough about crypto to pick a cipher for a specific application, then they should also be able to supply its parameters. What would you do? For example, what should the IV size for AES-CCM be? pycryptodome, a popular crypto library, says 11 bytes, a reasonably strong choice. OpenSSL only uses 7 bytes, a much weaker choice, which has the advantage of permitting larger messages to be encrypted. Similar problems arise with authentication tag lengths etc. We would have to decide without knowing what the user is going to do with the cipher.

Also, there are API differences for different ciphers. All of the following statements are true for some supported ciphers but not for all:

• supports streaming behavior
• has a unique valid IV length
• supports AEAD
• has a maximum message size
• is resistant against padding oracle attacks
• its output length is a multiple of the block size
• only works if message integrity is guaranteed by using an authentication tag
• needs to know the length of the message in advance
• designed for message encryption
• designed for symmetric key wrapping

If you let users pick any valid cipher, you end up with an arbitrary subset of these attributes, and your application needs to somehow consider all of these points and their combinations.

[mscdex]

if they know enough about crypto to pick a cipher for a specific application, then they should also be able to supply its parameters

Not necessarily. I've never seen an end user program that allows you to specify ciphers also require the end user to specify an IV.

[github-actions]

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

[tniessen]

This might benefit from #35368.

[bnb]

given the discussion and time this has been open, is this still something we'd want to consider pursuing? @tniessen @gengjiawen