calcom / cal.com

Scheduling infrastructure for absolutely everyone.
https://cal.com
Other
32.45k stars 8.01k forks source link

[CAL-2901] Symmetric encryption keys are weaker than they should be #10805

Open nicktrn opened 1 year ago

nicktrn commented 1 year ago

Issue Summary

https://github.com/calcom/cal.com/blob/7c237736365c188e1762691fa4376a5e71daeee5/.env.example#L88-L91

Currently, 24 random bytes are expanded via base64-encoding to match the required AES-256 key length of 32 bytes.

This is effectively AES-192 security (with two additional rounds).

Recommendations

diff --git a/.env.example b/.env.example
index 20bef8f8a..209740148 100644
--- a/.env.example
+++ b/.env.example
@@ -87,7 +87,7 @@ CRON_ENABLE_APP_SYNC=false

 # Application Key for symmetric encryption and decryption
 # must be 32 bytes for AES256 encryption algorithm
-# You can use: `openssl rand -base64 24` to generate one
+# You can use: `openssl rand -base64 32` to generate one
 CALENDSO_ENCRYPTION_KEY=

 # Intercom Config
diff --git a/packages/lib/crypto.ts b/packages/lib/crypto.ts
index d33f0c8f4..f662f22cc 100644
--- a/packages/lib/crypto.ts
+++ b/packages/lib/crypto.ts
@@ -13,7 +13,7 @@ const IV_LENGTH = 16; // AES blocksize
  * @returns Encrypted value using key
  */
 export const symmetricEncrypt = function (text: string, key: string) {
-  const _key = Buffer.from(key, "latin1");
+  const _key = Buffer.from(key, "base64");
   const iv = crypto.randomBytes(IV_LENGTH);

   const cipher = crypto.createCipheriv(ALGORITHM, _key, iv);
@@ -30,7 +30,7 @@ export const symmetricEncrypt = function (text: string, key: string) {
  * @param key Key used to decrypt value must be 32 bytes for AES256 encryption algorithm
  */
 export const symmetricDecrypt = function (text: string, key: string) {
-  const _key = Buffer.from(key, "latin1");
+  const _key = Buffer.from(key, "base64");

   const components = text.split(":");
   const iv_from_ciphertext = Buffer.from(components.shift() || "", OUTPUT_ENCODING);

The key encoding could also be chosen dynamically based on its length and an additional env var OLD_ENCRYPTION_KEY provided to migrate old secrets on-demand.

As part of the migration, I would recommend switching over to aes-256-gcm. Using non-authenticated AES in CBC mode is not recommended when GCM is available.

Could also take care of this at the same time:

From SyncLinear.com | CAL-2901

MeenuyD commented 1 year ago

/attempt #10805

nicktrn commented 1 year ago

Sorry, there's nothing to attempt here, the labeling bot is just having fun.

PeerRich commented 1 year ago

Maige update instructions to avoid bounties

maige-app[bot] commented 1 year ago

Here are your new instructions:

"Update instructions to avoid bounties"

Feel free to provide feedback.

PeerRich commented 1 year ago

Maige update instructions to avoid "šŸ’Ž Bounty"

maige-app[bot] commented 1 year ago

Here are your new instructions:

"Update instructions to avoid bounties and šŸ’Ž Bounty"

Feel free to provide feedback.

nicktrn commented 1 year ago

Not stale.