Return to Answer

added 39 characters in body

Source Link

edited Mar 1, 2020 at 12:09

6.8k
21
54

Comments are below the code fragments.

const ENC_IN = "utf8";
const ENC_OUT = "base64";

With the choice of encryption and encoding, I'd write out encodinginclude the "encoding" part fully (e.g. ENCODING_IN and ENCODING_OUT).

const KEY = Buffer.from("…", ENC_IN);

No, a key should not be a constant, and a key should certainly not be a string, even if it has a specific encoding. A key should be 16, 24 or 32 fully unpredictable bytes (for 128, 192 or 256 bit keys respectively). With UTF-8 you only have some 108 possible printable characters for each byte. Besides that, many libraries will not handle partial or overly long keys well.

If you want to use a password, then use a password based key derivation function (PBKDF) and then createCipheriv / createDecipheriv. Support for that is build in (note that createCipher / createDecipher is deprecated).

Encryption

const encrypt = async function encrypt(data) {

At this point you may want to document the character encoding and the base 64 encoding that is being applied, the mode of operation and that an IV and authentication tag is added. Document or point to the protocol in other words.

const IV = Buffer.from(CRYPTO.randomBytes(BUFFER_SIZE));

Good, random IV. For GCM though I'd use 12 bytes or the mode has to perform additional operations for no security benefit.

const ENCRYPTED = Object.freeze({

Even though you froze the variable, it's still a local variable and therefore you should not be using all uppercase.

data: enc,
iv: IV,
authTag: CIPHER.getAuthTag()

Now this is a bit weird. Unless I'm mistaken enc is now base 64 encoded, but iv and authTag are just plain buffers of binary data.

Decryption

 * @param {string} iv - initialisation vector

Strange, your iv of type Buffer seems to have magically become a string...

Otherwise the decrypt function is nicely symmetrical to the encrypt function, which is good.

Beware of overly stringifying your code. Beware of wrapper classes; don't start using above class for all your code: use user specific encryption routines insetead, and clearly specify your protocol.

When using GCM, you may want to allow additional authenticated data (AAD) to be included in the calculation of the tag.

Comments are below the code fragments.

const ENC_IN = "utf8";
const ENC_OUT = "base64";

With the choice of encryption and encoding, I'd write out encoding.

const KEY = Buffer.from("…", ENC_IN);

Encryption

const encrypt = async function encrypt(data) {

const IV = Buffer.from(CRYPTO.randomBytes(BUFFER_SIZE));

Good, random IV. For GCM though I'd use 12 bytes or the mode has to perform additional operations for no security benefit.

const ENCRYPTED = Object.freeze({

Even though you froze the variable, it's still a local variable and therefore you should not be using all uppercase.

data: enc,
iv: IV,
authTag: CIPHER.getAuthTag()

Now this is a bit weird. Unless I'm mistaken enc is now base 64 encoded, but iv and authTag are just plain buffers of binary data.

Decryption

 * @param {string} iv - initialisation vector

Strange, your iv of type Buffer seems to have magically become a string...

Otherwise the decrypt function is nicely symmetrical to the encrypt function, which is good.

When using GCM, you may want to allow additional authenticated data (AAD) to be included in the calculation of the tag.

Comments are below the code fragments.

const ENC_IN = "utf8";
const ENC_OUT = "base64";

With the choice of encryption and encoding, I'd include the "encoding" part fully (e.g. ENCODING_IN and ENCODING_OUT).

const KEY = Buffer.from("…", ENC_IN);

Encryption

const encrypt = async function encrypt(data) {

const IV = Buffer.from(CRYPTO.randomBytes(BUFFER_SIZE));

Good, random IV. For GCM though I'd use 12 bytes or the mode has to perform additional operations for no security benefit.

const ENCRYPTED = Object.freeze({

Even though you froze the variable, it's still a local variable and therefore you should not be using all uppercase.

data: enc,
iv: IV,
authTag: CIPHER.getAuthTag()

Now this is a bit weird. Unless I'm mistaken enc is now base 64 encoded, but iv and authTag are just plain buffers of binary data.

Decryption

 * @param {string} iv - initialisation vector

Strange, your iv of type Buffer seems to have magically become a string...

Otherwise the decrypt function is nicely symmetrical to the encrypt function, which is good.

When using GCM, you may want to allow additional authenticated data (AAD) to be included in the calculation of the tag.

Source Link

answered Mar 1, 2020 at 0:49

Maarten Bodewes

6.8k
21
54

Comments are below the code fragments.

const ENC_IN = "utf8";
const ENC_OUT = "base64";

With the choice of encryption and encoding, I'd write out encoding.

const KEY = Buffer.from("…", ENC_IN);

Encryption

const encrypt = async function encrypt(data) {

const IV = Buffer.from(CRYPTO.randomBytes(BUFFER_SIZE));

Good, random IV. For GCM though I'd use 12 bytes or the mode has to perform additional operations for no security benefit.

const ENCRYPTED = Object.freeze({

Even though you froze the variable, it's still a local variable and therefore you should not be using all uppercase.

data: enc,
iv: IV,
authTag: CIPHER.getAuthTag()

Now this is a bit weird. Unless I'm mistaken enc is now base 64 encoded, but iv and authTag are just plain buffers of binary data.

Decryption

 * @param {string} iv - initialisation vector

Strange, your iv of type Buffer seems to have magically become a string...

Otherwise the decrypt function is nicely symmetrical to the encrypt function, which is good.

When using GCM, you may want to allow additional authenticated data (AAD) to be included in the calculation of the tag.