Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unknown ciphers: aes-256-cfb, aes-128-cfb #16195

Closed
ghost opened this issue Dec 26, 2018 · 31 comments
Closed

Unknown ciphers: aes-256-cfb, aes-128-cfb #16195

ghost opened this issue Dec 26, 2018 · 31 comments
Labels
4-2-x bug 🪲 bug/regression ↩️ A new version of Electron broke something

Comments

@ghost
Copy link

ghost commented Dec 26, 2018

  • Output of node_modules/.bin/electron --version: 4.0.0
  • Operating System (Platform and Version): Windows 10
  • Output of node_modules/.bin/electron --version on last known working Electron version (if applicable): v3.x.x

Expected Behavior
Generally , invoke crypto.createCipheriv () that should return a Cipheriv Object.

Actual behavior
When I call crypto.createCipheriv() , it will response me a error that is unknown cipher .
But If I run this program(test code ) alone by node cmd. (ex. node test.js ) that it work.
So I don't think it is a node10 problem.

To Reproduce
sample code
in main.js

const crypto = require('crypto')
const methodName = 'aes-256-cfb'
const key =  'nMvTdb7VXMPudFWH'
const iv = crypto.randomBytes(16)
const cipher = crypto.createCipheriv(methodName, key, iv)
console.log(cipher)

Response:
in Electron 4 or more newer

unknown cipher

in Electron 3.x , more older or run alone by node cmd

 Cipheriv {
  _handle: {},
  _decoder: null,
  _options: undefined,
  writable: true,
  readable: true }  
@welcome
Copy link

welcome bot commented Dec 26, 2018

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

To help make it easier for us to investigate your issue, please follow the contributing guidelines.

@shiftkey
Copy link
Contributor

@DessertBoy which version of Node 10 are you testing on locally that works with your sample code? I ask because Electron 3 is based on Node 10.2 and Electron 4 is based on Node 10.11, so having more data points here about what works and what doesn't will help us troubleshoot the issue better...

@ghost
Copy link
Author

ghost commented Dec 27, 2018

@DessertBoy which version of Node 10 are you testing on locally that works with your sample code? I ask because Electron 3 is based on Node 10.2 and Electron 4 is based on Node 10.11, so having more data points here about what works and what doesn't will help us troubleshoot the issue better...

@shiftkey I have tried v10.2.0 and v10.11.0 on locally with sample code in test.js , and execute the sample code via node cmd. As a result both versions can succeed
If I run sample code in main.js via Electron 4 that will fail. but Electron 3 is ok.
so I don't think that is Node10 version problem.

@pbca26
Copy link

pbca26 commented Dec 27, 2018

It seems that the problem is far bigger than just missing cipher

I compared Electron 3, Electron 4 and NodeJS 10.11.0

Electron 3/NodeJS 10.11.0

const crypto = require('crypto')
const hashes = crypto.getHashes();
console.log(hashes);

[ 'RSA-MD4',
'RSA-MD5',
'RSA-MDC2',
'RSA-RIPEMD160',
'RSA-SHA1',
'RSA-SHA1-2',
'RSA-SHA224',
'RSA-SHA256',
'RSA-SHA384',
'RSA-SHA512',
'blake2b512',
'blake2s256',
'md4',
'md4WithRSAEncryption',
'md5',
'md5-sha1',
'md5WithRSAEncryption',
'mdc2',
'mdc2WithRSA',
'ripemd',
'ripemd160',
'ripemd160WithRSA',
'rmd160',
'sha1',
'sha1WithRSAEncryption',
'sha224',
'sha224WithRSAEncryption',
'sha256',
'sha256WithRSAEncryption',
'sha384',
'sha384WithRSAEncryption',
'sha512',
'sha512WithRSAEncryption',
'ssl3-md5',
'ssl3-sha1',
'whirlpool' ]

Electron 4

var crypto = require('crypto')
const hashes = crypto.getHashes();
console.log(hashes);

[ 'md4', 'md5', 'sha1', 'sha224', 'sha256', 'sha384', 'sha512' ]

quite a lot of hash functions just disappeared in Electron 4

@shiftkey
Copy link
Contributor

For reference, I checked the similar crypto.getCiphers() function on macOS (not Windows like the original reporter) in Electron 3.0.13 (135 listed):

And then compared it to what's now in 4.0.0 (16 listed):

@ca333
Copy link

ca333 commented Dec 27, 2018

confirm the above issue - seems the v4 incl. crypto module got "rid of" most hash funcs which breaks backwards compatiblity in our software.

@ghost
Copy link
Author

ghost commented Dec 27, 2018

@ca333 yes. That's it.

@ghost
Copy link
Author

ghost commented Dec 27, 2018

@shiftkey @pbca26 Thank you. It seems that the problem is this. But I want to know why is there such a difference ? or Is this just a bug in Electron 4 ? and Can I think this is the meaning ?

@shiftkey
Copy link
Contributor

@DessertBoy I'm not sure offhand. You'd have to look into how the Electron team are building and packaging Node into Electron - https://github.com/electron/node is the repository where that happens

@nornagon
Copy link
Member

nornagon commented Jan 2, 2019

The root cause here is that we switched from OpenSSL in Electron 3 to BoringSSL in Electron 4.

In Electron 3, Chromium uses BoringSSL and Node uses OpenSSL. In Electron 4, we only ship one SSL library, and that's BoringSSL. The main reason for this is that Electron 4 is a single static binary, and BoringSSL and OpenSSL cannot coexist in the same binary because their symbols conflict (e.g. both of them define a function called SSL_CTX_use_certificate).

In general, BoringSSL is more focused on providing up-to-date and secure ciphers than it is concerned with backwards compatibility, so if you really need OpenSSL specifically, I think the best path forward would be to have OpenSSL available as a native node module. That way the vast majority of folks that don't need access to obscure cipher suites can use the smaller, already-bundled BoringSSL, and anyone who needs OpenSSL specifically can install it from npm.

That said, there is some precedent for BoringSSL adding support for less-commonly-used ciphers and cipher options via the decrepit module, for example aes-128-cfb. Electron does not compile all the decrepit sources by default—only just enough to get Node to build—but we could add more relatively easily. We're generally happy to support and expose what BoringSSL already supports but doesn't necessarily expose to the EVP APIs that Node calls (i.e. EVP_CIPHER_do_all_sorted and EVP_get_cipherbyname). However, there are some cipher suites that BoringSSL doesn't support, for instance Camellia. See decrepit/ in the BoringSSL source for features that BoringSSL supports but doesn't expose by default (usually because they're underused or insecure).

Here's an example of a patch that would add support for the aes-{128,256}-cfb cipher suites to BoringSSL's compatibility layer. (Would also need to add cfb.c to the list of decrepit modules that Electron includes.)

@nornagon
Copy link
Member

nornagon commented Jan 2, 2019

Also, as for the list of hashes, OpenSSL adds a lot of aliases to that list, so e.g. RSA-SHA1, RSA-SHA1-2, sha1WithRSAEncryption and ssl3-sha1 are all aliases of sha1.

Digest algorithms present in OpenSSL that are not present in BoringSSL (decrepit or otherwise):

  • mdc2 (aka RSA-MDC2, mdc2WithRSA)
  • blake2 (its variants blake2b512, blake2s256)
  • whirlpool

All the other elements in the list produced by crypto.getHashes() are just aliases.

ripemd160 (aka RSA-RIPEMD160, ripemd160, ripemd160WithRSA, rmd160) is present in BoringSSL's decrepit module and not currently built in Electron.

@jasontlouro
Copy link

jasontlouro commented Jan 10, 2019

@nornagon With Electron 3 (and OpenSSL rather than BoringSSL), 'aes256' was an alias for which cipher? In Electron 4, I have tried changing to aes-256-cbc, aes-256-ctr, aes-256-ecb, aes-256-ofb, and aes-256-xts. None of these have allowed me to achieve backwards compatibility with the version of my software that ran on Electron 3 (and used cipher aes256).

@jasontlouro
Copy link

Never mind. Was able to achieve compatibility using aes-256-cbc.

@nornagon
Copy link
Member

Glad you found it! If you want to check the source, here's the list of aliases in the OpenSSL source: https://github.com/nodejs/node/blob/master/deps/openssl/openssl/crypto/evp/c_allc.c

@sirasistant
Copy link

Is it planned to add ripemd160 to electron from boringSSL? it is needed for some cryptocurrency-related libraries, like https://github.com/cryptocoinjs/hdkey/blob/master/lib/hdkey.js#L238

@pazaan
Copy link

pazaan commented Jan 18, 2019

Thanks @nornagon.

That said, there is some precedent for BoringSSL adding support for less-commonly-used ciphers and cipher options via the decrepit module, for example aes-128-cfb.

Does this mean that these decrepit ciphers will be make available in a 4.x.y version of Electron? We use crypto's aes-128-cfb cipher to communicate with a medical device.
So we'll either have to wait to upgrade to 4 until it's included, or include a aes-128-cfb module ourselves, which apparently gets really nasty in Windows.

@andreieftimie
Copy link

Can someone change the label? This doesn't only affect windows.

On macOS I see the same limited ciphers available as @pbca26 mentioned above:

> require('crypto').getHashes()
[ 'md4', 'md5', 'sha1', 'sha224', 'sha256', 'sha384', 'sha512' ]

@nornagon
Copy link
Member

ripemd160 is a digest, not a cipher. It's listed in crypto.getHashes() on versions >= 4.0.4.

@ontheroad1992
Copy link

ripemd160是一个摘要,而不是一个密码。它列在crypto.getHashes()版本> = 4.0.4上。

Excuse me, will you support 'rmd160' in the follow-up?

@nornagon
Copy link
Member

rmd160 is an alias for ripemd160. We won't be adding additional aliases for ciphers that are already available.

@junderw
Copy link

junderw commented Mar 28, 2019

@nornagon So every single app that uses rmd160 (keeping in mind that NodeJS crypto uses aliases) will have to dance around with try catches every time a user tries to use their library in Electron?

Just making sure.

$ node -e "console.log(require('crypto').getHashes().filter(v => v.indexOf('160') > -1))"
[ 'RSA-RIPEMD160', 'ripemd160', 'ripemd160WithRSA', 'rmd160' ]

Edit: I have asked some coworkers using various versions of NodeJS, Linux, MacOS, Windows 10 etc. and everyone gets the same results as me above...

Which comes down to two choices:

  1. The world changes for Electron. Every library who uses rmd160 must change. Best to get the word out to the entire open source world.
  2. Electron maintains backwards compatibility for NodeJS libraries and adds aliases.

Don't read into my words here. I am not implying anything.

Just want to clarify that 1 is the proper way forward? If so I will go ahead and add a try catch to our library.

@nornagon
Copy link
Member

I'd recommend you just use "ripemd160" which is the canonical name and is supported on both Node.js and Electron. No need for a try/catch.

@junderw
Copy link

junderw commented Mar 28, 2019

Ok, I'll use a try catch then.

Wouldn't want someone's app to break because of it, and bumping major version for a string change is silly.

@peterremote1980
Copy link

Is there any fix to this problem? thanks

@sschiessl-bcp
Copy link

sschiessl-bcp commented Oct 10, 2019

Is there any fix to this problem? thanks

Anything? We would also be very interested in a fix, especially for Electron 6.

@nornagon nornagon changed the title Electron 4.0.0 'unknown cipher' appears in Crypto module Unknown ciphers: aes-256-cfb, aes-128-cfb Oct 10, 2019
@nornagon
Copy link
Member

nornagon commented Oct 10, 2019

I've updated the title of this issue to refer to the initially-requested ciphers aes-256-cfb and aes-128-cfb, which are available & working in current versions of Electron. If you're having trouble with a different cipher, please open a new issue with the details of the problem you're experiencing.

@franzwilhelm
Copy link

@nornagon Thank you for some very clear answers in this thread! I'm in need of aes-128-ccm, not cfb for a project I'm currently working on. I don't really rely on having that installed into the electron binaries, if there's an easy way to polyfill the cipher into the crypto library included with the latest electron versions. How would I do such a thing? I'm currently relying on a package importing crypto through require('crypto'), and I need the cipher to be available through that import

@nornagon
Copy link
Member

@franzwilhelm as requested in my previous comment, please open a new issue for new ciphers.

jonbarrow added a commit to jonbarrow/node-aes-cmac that referenced this issue Jan 6, 2020
Removed the deprecated Buffer constructor calls and replaced with `Buffer.from()` and `Buffer.alloc()`

Removed unused `toBinaryString` from buffer-tools

Changed from cipher algorithm alias to real names (`aes128`, `aes192`, and `aes256` to `aes-128-cbc`, `aes-192-cbc`, and `aes-256-cbc`) to improve compatibility with Electron, see electron/electron#16195 for more details

Now allows users to provide their own cipher algorithm name. To keep backwards compatibility, if an algorithm is not given then defaults to the old lookup table method
@PeterRao
Copy link

PeterRao commented Feb 5, 2020

Never mind. Was able to achieve compatibility using aes-256-cbc.

How did you solve it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4-2-x bug 🪲 bug/regression ↩️ A new version of Electron broke something
Projects
None yet
Development

No branches or pull requests