Am I doing AES 256 encryption and decryption Node.js correctly?
Asked Answered
N

1

9

I need to encrypt a chat message that will be stored a database. The data is a string of characters of various lengths. I want to use the native node.js crypto library and use a symmetric encryption protocol such as AES 256. I have concerns are the following:

  1. Is CBC the correct AES mode for this use case for this type of field stored in a TEXT field in MySQL?
  2. Does the key look like it is generated correctly?
  3. Is the IV correct? Is prepending the IV to the encrypted text a proper way to do it or should it be a separate field?
// AES RFC - https://tools.ietf.org/html/rfc3602
const crypto = require('crypto');

const algorithm = 'aes-256-cbc';
// generate key with crypto.randomBytes(256/8).toString('hex')
const key = '6d858102402dbbeb0f9bb711e3d13a1229684792db4940db0d0e71c08ca602e1';
const IV_LENGTH = 16;

const encrypt = (text) => {
  const iv = crypto.randomBytes(IV_LENGTH);
  const cipher = crypto.createCipheriv(algorithm, Buffer.from(key, 'hex'), iv);
  let encrypted = cipher.update(text);
  encrypted = Buffer.concat([encrypted, cipher.final()]);
  return `${iv.toString('hex')}:${encrypted.toString('hex')}`;
};

const decrypt = (text) => {
  const [iv, encryptedText] = text.split(':').map(part => Buffer.from(part, 'hex'));
  const decipher = crypto.createDecipheriv(algorithm, Buffer.from(key, 'hex'), iv);
  let decrypted = decipher.update(encryptedText);
  decrypted = Buffer.concat([decrypted, decipher.final()]);
  return decrypted.toString();
};

exports.encrypt = encrypt;
exports.decrypt = decrypt;
Nahshun answered 6/9, 2018 at 21:33 Comment(8)
Possible duplicate of Node.js encrypts large file using AESOverwhelm
@MattClark I believe this is not a duplicate because it provides an example that uses an IV and demonstrates decryption.Nahshun
@MattClark does this seem better? This is really my concern. Also saying someone has a "terrible question" and shows "no effort" doesn't seem to match the StackExchange Code of Conduct. meta.stackexchange.com/conductNahshun
what are your concerns? This looks good to me on first look.Pentomic
does this code work for you?Plasticize
@Plasticize the code is working correctly.Nahshun
@Pentomic I have improved my questions to more clearly state my concerns.Nahshun
It seems that Code Review would be a better place for this question as it looks like you just want some opinions on working code and the answer does not provide any code, just a review on it.Gadid
P
4

Is CBC the correct AES mode for this use case for this type of field stored in a TEXT field in MySQL?

Well, this depends a bit on your text. But probably yes.

Does the key look like it is generated correctly?

yeah, looks good to me. It should look random and it looks random. Not sure what your concern is here.

Is the IV correct? Is prepending the IV to the encrypted text a proper way to do it or should it be a separate field?

The IV looks good to me. I don't see many reasons why you shouldn't do it this way except one: its not very storage efficient. It would be far more efficient to store the data not as hex string but as binary data! And then you can't just use a colon to seperate the data. So either you know that its the first n bytes or you do a seperate field. Both has upsides and downsides, but both is ok. It's primary a question about style.

Pentomic answered 6/9, 2018 at 22:46 Comment(4)
The text in this application are chat messages.Nahshun
maybe consider using streams tooPlasticize
@Plasticize what benefit would that give me?Nahshun
@Nahshun it can allow you more stable memory usage & prevent you from blocking the event loop when a big payload needs to be processed, because it only blocks for a chunk at a time. With a chat app it probably will not buy you much.Plasticize

© 2022 - 2024 — McMap. All rights reserved.