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

Equivalent node:crypto commands for CryptoJS (to migrate after deprecation) #468

Open
cannikin opened this issue Oct 26, 2023 · 13 comments
Open

Comments

@cannikin
Copy link

cannikin commented Oct 26, 2023

We are using CryptoJS in our code but now that 4.2.0 comes with a big deprecation notice we plan on moving off. We'd like to eliminate the CryptoJS lib completely, and just use node:crypto going forward, but I can't figure out the proper incantation of node:crypto commands to decrypt something that was encrypted with CryptoJS:

// Current encryption method
// `secret` is a 64 byte string
CryptoJS.AES.encrypt(text, secret).toString() // => base64 string

// Current decryption method
// `encText` is the base64 string derived from above
CryptoJS.AES.decrypt(encText, secret).toString(CryptoJS.enc.Utf8)

We'd like to be able to decrypt using node:crypto and then re-encrypt using a more secure version, with an iv and all that good stuff. I've tried several different iterations of this code, with 6 different algorithms, but none of them are able to decrypt:

// Proposed decryption method for existing CryptoJS-encrypted text
// `algorithm` is one of aes128 | aes192 | aes256 | aes-128-cbc | aes-192-cbc | aes-256-cbc
// `secret` is same 64 byte string used to encrypt
// `encText` is the base64 string from `CryptoJS.AES.encrypt()`

const decipher = crypto.createDecipher(algorithm, secret);
let decryptedText = decipher.update(encText, "base64", "utf-8")
decryptedText += decipher.final("utf-8")

The node:crypto docs state:

The implementation of crypto.createDecipher() derives keys using the OpenSSL function EVP_BytesToKey with the digest algorithm set to MD5, one iteration, and no salt.

Which sounds similar to what CryptoJS is doing, but maybe not similar enough. :(

I've tried truncating the secret to only 32 characters (tried both first 32 and last 32) but it didn't help.

Any ideas of what else I could try? For testing, here's the encText and secret that should decrypt to Hello, world:

const CryptoJS = require('crypto-js')

const encText = 'U2FsdGVkX18ZLpNMrgcEPbbEfE2c6h3E9kc0GRLE4pU='
const secret = 'V7gRKWw4uz6QVH7cGHqcUEPHpr8CfqD4LTckiTpmdeeDzS423Zc7zaBngvpwBv6Y'

CryptoJS.AES.decrypt(encText.secret).toString(CryptoJS.enc.Utf8) // => 'Hello, world'
@a-tortevois
Copy link

a-tortevois commented Oct 27, 2023

I found this example:

'use strict';

const assert = require('node:assert');
const crypto = require('node:crypto');

const CryptoJS = require('crypto-js');

const secret = 'secret';
const plainText = 'message';

class CryptoHelper {
  decryptAES(encryptedText, secret) {
    // From https://gist.github.com/schakko/2628689?permalink_comment_id=3321113#gistcomment-3321113
    // From https://gist.github.com/chengen/450129cb95c7159cb05001cc6bdbf6a1
    const cypher = Buffer.from(encryptedText, 'base64');
    const salt = cypher.slice(8, 16);
    const password = Buffer.concat([Buffer.from(secret, 'binary'), salt]);
    const md5Hashes = [];
    let digest = password;
    for (let i = 0; i < 3; i++) {
      md5Hashes[i] = crypto.createHash('md5').update(digest).digest();
      digest = Buffer.concat([md5Hashes[i], password]);
    }
    const key = Buffer.concat([md5Hashes[0], md5Hashes[1]]);
    const iv = md5Hashes[2];
    const contents = cypher.slice(16);
    const decipher = crypto.createDecipheriv('aes-256-cbc', key, iv);

    return decipher.update(contents) + decipher.final();
  }
}

const cryptoHelper = new CryptoHelper;

const encrypted = CryptoJS.AES.encrypt(plainText, secret, { mode: CryptoJS.mode.CBC, padding: CryptoJS.pad.Pkcs7 });
const encryptedText = encrypted.toString();
console.log('encrypted', encryptedText);

const decryptedText = cryptoHelper.decryptAES(encryptedText, secret);
console.log('decrypted', decryptedText);

assert.equal(plainText, decryptedText);

@cannikin
Copy link
Author

cannikin commented Oct 27, 2023

@a-tortevois Thank you for the example!

I'm going to test it later this afternoon, but from a quick glance I'm a little worried about the padding: CryptoJS.pad.Pkcs7 There's no mention of PKCS7 in any of the node:crypto docs, only PKCS1. CryptoJS says that it uses PKCS7 by default if you don't tell it to use something different, and I didn't specify the padding in my existing code, so I assume it's using PKCS7 as well.

I'm wondering if that difference will make my encrypted strings non-decryptable by node:crypto...

@a-tortevois
Copy link

a-tortevois commented Oct 27, 2023

It work fine with your string, secret :

console.log('decrypted:', cryptoHelper.decryptAES('U2FsdGVkX18ZLpNMrgcEPbbEfE2c6h3E9kc0GRLE4pU=', 'V7gRKWw4uz6QVH7cGHqcUEPHpr8CfqD4LTckiTpmdeeDzS423Zc7zaBngvpwBv6Y'));

decrypted: Hello, world

@cannikin
Copy link
Author

This worked perfectly, thank you so much @a-tortevois!!

jtoar added a commit to redwoodjs/redwood that referenced this issue Nov 4, 2023
So CryptoJS just dropped a bomb: everything they do by default is not as
strong as it could be. Oh and by the way, the entire library is now
deprecated. :(
GHSA-xwcq-pm8m-c4vf
Unfortunately we can't just upgrade to the latest release 4.2.0 because
the hashing algorithm has changed, and a user would no longer be able to
login: the default hash generated by CryptoJS 4.2.0 won't match the hash
generated by CryptoJS 4.1.0.

Note that this is only an issue if someone got the contents of your
database and wanted to figure out user passwords (but it still cost
[$45,000 per password](https://eprint.iacr.org/2020/014.pdf)
apparently?).

In the wake of this CVE we're going to convert dbAuth to use the
built-in `node:crypto` library instead, with more sensible default
configuration.

There are two areas where we use the crypto libs:

1. Hashing the user's password to store in the DB and compare on login
2. Encrypting/decrypting the session data in a cookie

We're going to do this in a non-breaking way by supporting *both* the
original CryptoJS-derived values, and the new `node:crypto` ones. The
alternative would be to require every user to change their password,
which seems like a non-starter.

1. On signup, store the hashedPassword using the new `node:crypto`
algorithm
2. On login, compare the user's hashedPassword using the `node:crypto`
algorithm:
  * If a match is found, user is logged in
* If a match fails, fall back to the original CryptoJS algorithm (but
using the `node:crypto` implementation)
* If a match is found, update the `hashedPassword` in the database to
the new algorithm, user is logged in
* If a match is still not found, the user entered the wrong password.

Likewise for cookies and login:

1. When encrypting the user's session, always use the new `node:crypto`
algorithm
2. When decrypting the user's session, first try with `node:crypto`
  * If decrypting works, user is logged in
* If decrypting fails, try the older CryptoJS algorithm ([I haven't
figured how](brix/crypto-js#468) to use
`node:crypto` to decrypt something that was encrypted with CryptoJS yet,
so we'll need to keep the dependency on CryptoJS around for now)
* If decrypting works, re-encrypt the cookie using the new `node:crypto`
algorithm, user is logged in
* If decrypting still fails, the session is invalid (someone tampered
with the cookie) so log them out

## Notifying Users

We could announce in the Release Notes that if a platform wants the
absolute safest route, they should change their `SESSION_SECRET` *and*
have users change their password if, for example, they suspect that
their database may have been compromised before our release.

The next most secure thing would be to just change `SESSION_SECRET`
which would log everyone out, and on next login their password will get
re-hashed with the new algorithm.

But the default for most people will probably be to just go about
business as usual, and as time goes by more and more users' passwords
will be re-hashed on login.

Related to #9337 #9338 #9339 #9340

---------

Co-authored-by: Dominic Saadi <[email protected]>
jtoar added a commit to redwoodjs/redwood that referenced this issue Nov 4, 2023
So CryptoJS just dropped a bomb: everything they do by default is not as
strong as it could be. Oh and by the way, the entire library is now
deprecated. :(
GHSA-xwcq-pm8m-c4vf
Unfortunately we can't just upgrade to the latest release 4.2.0 because
the hashing algorithm has changed, and a user would no longer be able to
login: the default hash generated by CryptoJS 4.2.0 won't match the hash
generated by CryptoJS 4.1.0.

Note that this is only an issue if someone got the contents of your
database and wanted to figure out user passwords (but it still cost
[$45,000 per password](https://eprint.iacr.org/2020/014.pdf)
apparently?).

In the wake of this CVE we're going to convert dbAuth to use the
built-in `node:crypto` library instead, with more sensible default
configuration.

There are two areas where we use the crypto libs:

1. Hashing the user's password to store in the DB and compare on login
2. Encrypting/decrypting the session data in a cookie

We're going to do this in a non-breaking way by supporting *both* the
original CryptoJS-derived values, and the new `node:crypto` ones. The
alternative would be to require every user to change their password,
which seems like a non-starter.

1. On signup, store the hashedPassword using the new `node:crypto`
algorithm
2. On login, compare the user's hashedPassword using the `node:crypto`
algorithm:
  * If a match is found, user is logged in
* If a match fails, fall back to the original CryptoJS algorithm (but
using the `node:crypto` implementation)
* If a match is found, update the `hashedPassword` in the database to
the new algorithm, user is logged in
* If a match is still not found, the user entered the wrong password.

Likewise for cookies and login:

1. When encrypting the user's session, always use the new `node:crypto`
algorithm
2. When decrypting the user's session, first try with `node:crypto`
  * If decrypting works, user is logged in
* If decrypting fails, try the older CryptoJS algorithm ([I haven't
figured how](brix/crypto-js#468) to use
`node:crypto` to decrypt something that was encrypted with CryptoJS yet,
so we'll need to keep the dependency on CryptoJS around for now)
* If decrypting works, re-encrypt the cookie using the new `node:crypto`
algorithm, user is logged in
* If decrypting still fails, the session is invalid (someone tampered
with the cookie) so log them out

We could announce in the Release Notes that if a platform wants the
absolute safest route, they should change their `SESSION_SECRET` *and*
have users change their password if, for example, they suspect that
their database may have been compromised before our release.

The next most secure thing would be to just change `SESSION_SECRET`
which would log everyone out, and on next login their password will get
re-hashed with the new algorithm.

But the default for most people will probably be to just go about
business as usual, and as time goes by more and more users' passwords
will be re-hashed on login.

Related to #9337 #9338 #9339 #9340

---------

Co-authored-by: Dominic Saadi <[email protected]>
jtoar added a commit to redwoodjs/redwood that referenced this issue Nov 4, 2023
So CryptoJS just dropped a bomb: everything they do by default is not as
strong as it could be. Oh and by the way, the entire library is now
deprecated. :(
GHSA-xwcq-pm8m-c4vf
Unfortunately we can't just upgrade to the latest release 4.2.0 because
the hashing algorithm has changed, and a user would no longer be able to
login: the default hash generated by CryptoJS 4.2.0 won't match the hash
generated by CryptoJS 4.1.0.

Note that this is only an issue if someone got the contents of your
database and wanted to figure out user passwords (but it still cost
[$45,000 per password](https://eprint.iacr.org/2020/014.pdf)
apparently?).

In the wake of this CVE we're going to convert dbAuth to use the
built-in `node:crypto` library instead, with more sensible default
configuration.

There are two areas where we use the crypto libs:

1. Hashing the user's password to store in the DB and compare on login
2. Encrypting/decrypting the session data in a cookie

We're going to do this in a non-breaking way by supporting *both* the
original CryptoJS-derived values, and the new `node:crypto` ones. The
alternative would be to require every user to change their password,
which seems like a non-starter.

1. On signup, store the hashedPassword using the new `node:crypto`
algorithm
2. On login, compare the user's hashedPassword using the `node:crypto`
algorithm:
  * If a match is found, user is logged in
* If a match fails, fall back to the original CryptoJS algorithm (but
using the `node:crypto` implementation)
* If a match is found, update the `hashedPassword` in the database to
the new algorithm, user is logged in
* If a match is still not found, the user entered the wrong password.

Likewise for cookies and login:

1. When encrypting the user's session, always use the new `node:crypto`
algorithm
2. When decrypting the user's session, first try with `node:crypto`
  * If decrypting works, user is logged in
* If decrypting fails, try the older CryptoJS algorithm ([I haven't
figured how](brix/crypto-js#468) to use
`node:crypto` to decrypt something that was encrypted with CryptoJS yet,
so we'll need to keep the dependency on CryptoJS around for now)
* If decrypting works, re-encrypt the cookie using the new `node:crypto`
algorithm, user is logged in
* If decrypting still fails, the session is invalid (someone tampered
with the cookie) so log them out

We could announce in the Release Notes that if a platform wants the
absolute safest route, they should change their `SESSION_SECRET` *and*
have users change their password if, for example, they suspect that
their database may have been compromised before our release.

The next most secure thing would be to just change `SESSION_SECRET`
which would log everyone out, and on next login their password will get
re-hashed with the new algorithm.

But the default for most people will probably be to just go about
business as usual, and as time goes by more and more users' passwords
will be re-hashed on login.

Related to #9337 #9338 #9339 #9340

---------

Co-authored-by: Dominic Saadi <[email protected]>
@mustanish
Copy link

To add to this we are also using CryptoJs and we would like to replace existing cryptoJs implementation with web crypto APIs. For text and objects, it is working fine but with files, we are first converting into wordArray using

cryptoJs.lib.WordArray.create(fileObject)

what is something equivalent we can use to achieve the same result so that hash generated by web crypto API is same

@ppasler
Copy link

ppasler commented Mar 5, 2024

Hi there,
thanks for this!
Is there an equivalent encryptAES(plainText, secret) that will be backwards compatible (which could be decrypted with decryptAES())?
Cheers,
paul

@C0D3O
Copy link

C0D3O commented Apr 17, 2024

what about encryption?.......... chatgpt can't do it.......

@a-tortevois
Copy link

Of course, encryption is simply the reverse operation
Something like this:

  encryptAES(plainText, secret) {
    const salt = crypto.randomBytes(8);
    const password = Buffer.concat([Buffer.from(secret, 'binary'), salt]);
    const hash = [];
    let digest = password;
    for (let i = 0; i < 3; i++) {
      hash[i] = crypto.createHash('md5').update(digest).digest();
      digest = Buffer.concat([hash[i], password]);
    }
    const keyDerivation = Buffer.concat(hash);
    const key = keyDerivation.subarray(0, 32);
    const iv = keyDerivation.subarray(32);
    const cipher = crypto.createCipheriv('aes-256-cbc', key, iv);
    return Buffer.concat([
      Buffer.from('Salted__', 'utf8'),
      salt,
      cipher.update(plainText),
      cipher.final()
    ]).toString('base64');
  }

@C0D3O
Copy link

C0D3O commented Apr 17, 2024

@a-tortevois Thanks... but it doesn't work.. I guess my cryptojs ecnryption is different or sth....
could you please look at it and give a small hint? thank you

function checkAndConvertToHex(str) {
   if (/^\d+$/.test(str)) {
      // eslint-disable-next-line no-undef
      let hex = BigInt(str).toString(16).padStart(64, '0');
      return '0x' + hex;
   }
   else {
      return str;
   }
}

function is_odd(num) {
   return num % 2 !== 0;
}


export const encryptPrivateKey2 = (privateKey: string, password: string) => {
	const padded = checkAndConvertToHex(privateKey);

	let privateKeyBytes;
	if (is_odd(padded.length)) {
		privateKeyBytes = CryptoJS.enc.Utf8.parse(padded);
	} else {
		privateKeyBytes = CryptoJS.enc.Hex.parse(padded);
	}

	const key = CryptoJS.PBKDF2(password, CryptoJS.SHA256(password), { keySize: 256 / 32 });
	const ciphertext = CryptoJS.AES.encrypt(privateKeyBytes, key, { mode: CryptoJS.mode.ECB });

	return (is_odd(padded.length) ? 'UTF8' : '') + ciphertext.toString();
};

@a-tortevois
Copy link

Be careful, you use CryptoJS.mode.ECB, mine is with aes-256-cbc algorithm

@C0D3O
Copy link

C0D3O commented Apr 17, 2024

Be careful, you use CryptoJS.mode.ECB, mine is with aes-256-cbc algorithm

could you say what's more secured? if it's even correct to compare them?

@mohamedgomran
Copy link

@a-tortevois Thank you! both encrypt and decrypt work fine as expected 🚀

@RaisinTen
Copy link

Hi folks! I have decided to create this NPM module with a similar API: https://github.com/RaisinTen/aes-crypto-js to make it easier to migrate away from this deprecated library. Thanks to the conversation above, the library uses Node.js' builtin crypto module internally and is far easier to maintain. Feel free to give it a try, post issues and submit PRs, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants