fix: refactor crypto.ts to migrate to more secure encryption keys

[stabildev]

What does this PR do?

• Defaults to CALENDSO_ENCRYPTION_KEY for encrypting new secrets. Encoding is automatically inferred based on key length
• Adds a new CALENDSO_OLD_ENCRYPTION_KEY environment variable to enable on-demand migration to new encryption key
• Defaults to CALENDSO_ENCRYPTION_KEY for decrypting (symmetricDecrypt). If decryption fails and CALENDSO_OLD_ENCRYPTION_KEY is set, decryption usingCALENDSO_OLD_ENCRYPTION_KEY is attempted
• If decryption with the old key is successful, a callback onShouldUpdate: (result) => void is fired to facilitate on demand migration to the new key

Because decryption happens in multiple places with multiple different expected result types, there needs to be a mechanism to detect whether decryption was successful. To achieve this, zod is being used (similar to how TRPC works). When attempting to decrypt a value, a schema has to be supplied. In many places the result of symmetricDecrypt was already being parsed using zod, so this addition can simplify the code in some places.

export const symmetricDecrypt = function <TOut>(
  text: string,
  opts: {
    schema: z.ZodSchema<TOut>;
    key?: string; // now optional, defaulting to CALENDSO_ENCRYPTION_KEY
    encoding?: BufferEncoding; // now optional, defaulting to "base64"
    onShouldUpdate?: (result: TOut) => void;
  }
): TOut {
  const { key = CALENDSO_ENCRYPTION_KEY, encoding = "base64", schema } = opts;

  const _key = Buffer.from(key, encoding);

  const components = text.split(":");
  const iv_from_ciphertext = Buffer.from(components.shift() || "", OUTPUT_ENCODING);
  const decipher = crypto.createDecipheriv(ALGORITHM, _key, iv_from_ciphertext);
  let deciphered = decipher.update(components.join(":"), OUTPUT_ENCODING, INPUT_ENCODING);
  deciphered += decipher.final(INPUT_ENCODING);

  // Parsing the deciphered result using the specified schema
  const result = schema.safeParse(
    schema instanceof z.ZodObject || schema instanceof z.ZodArray ? JSON.parse(deciphered) : deciphered
  );

  // If parsing fails and the new encryption key has been used
  // and there is an old encryption key, automatically retry with the old one
  if (!result.success && key === CALENDSO_ENCRYPTION_KEY && CALENDSO_OLD_ENCRYPTION_KEY) {
    return symmetricDecrypt(text, {
      schema: schema,
      key: CALENDSO_OLD_ENCRYPTION_KEY,
      encoding: "latin1",
    });
  }

  // If parsing fails (after trying the old key as well), throw
  if (!result.success) {
    throw new Error(result.error.issues.map((issue) => issue.message).join(", "));
  }

  // Convenience callback if the value needs to be re-encrypted using the new key
  if (key === CALENDSO_OLD_ENCRYPTION_KEY && opts.onShouldUpdate) {
    opts.onShouldUpdate(result.data);
  }

  // Finally return result
  return result.data;
};

In many instances this function can be dropped in and even simplify the existing code:

Example usage

This function call returns an array of strings and updates values encrypted using the old key on demand

const backupCodes = symmetricDecrypt(user.backupCodes, {
  schema: z.array(z.string()),
  onShouldUpdate: async (result) =>
    await prisma.user.update({
      where: { id: user.id },
      data: { backupCodes: symmetricEncrypt(JSON.stringify(result)) },
    }),
});

Decryption, parsing and destructuring in one call

const { username, url, password } = symmetricDecrypt(credential.key?.toString() || "", {
  schema: z.object({
    username: z.string(),
    url: z.string(),
    password: z.string(),
  }),
});

Background

README.md suggests to generate the CALENDSO_ENCRYPTION_KEY via openssl rand -base64.

However, packages/lib/crypto.ts expects a latin1 encoded key.

With the current configuration (openssl rand -base64 + latin1 encoding in crypto.ts) setting up 2FA fails.

Replacing latin1 with base64 fixes the issue.

Furthermore, the instructions in README.md are not congruent with those in .env.example. AES256 requires a 32 byte key so we should fix that in readme as well.

Backward compatibility

To support legacy latin1 encoded keys I added a function that detects whether a key is latin1 or base64 encoded (always under the assumption of 32 byte keys!):

const getEncoding = (key: string) => {
  if (key.length === 32) {
    return "latin1" as const;
  }
  return "base64" as const;
};

This should ensure that both the recommended 32 byte base64 keys and the apparently currently in use 32 byte latin1 keys work.

Related:

• https://github.com/calcom/cal.com/issues/10805
• https://github.com/calcom/cal.com/pull/12043

Type of change

• [x] Bug fix (non-breaking change which fixes an issue)

[vercel[bot]]

@stabildev is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions[bot]]

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link.

[github-actions[bot]]

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

[stabildev]

@nicktrn what are your thoughts on this, regarding https://github.com/calcom/cal.com/pull/12043#issuecomment-1786784680

[nicktrn]

Hi @stabildev

As far as I can tell, the function symmetricEncrypt is used only in apps/web/pages/api/auth/two-factor/totp/setup.ts to set up 2FA.

It's used in a few different places, mainly for integration secrets and 2FA like you mentioned, including backup codes.

Looking back on some of the changes I proposed:

The key encoding could also be chosen dynamically based on its length

Your PR certainly ticks that box!

an additional env var OLD_ENCRYPTION_KEY provided to migrate old secrets on-demand

That's really where the issue is. There needs to be a mechanism for decrypting old secrets. Automatically re-encrypting them with the new key would be great.

Would also be a good time to use a more secure, authenticated encryption scheme - I suggested aes-256-gcm.

Currently, your PR will break existing secrets.

[stabildev]

Hi @nicktrn

thanks for checking in!

As far as I can tell, the function symmetricEncrypt is used only in apps/web/pages/api/auth/two-factor/totp/setup.ts to set up 2FA.

I meant to edit this out. Wrote it in the initial draft and dug a lot deeper afterwards. Edited it out now.

an additional env var OLD_ENCRYPTION_KEY provided to migrate old secrets on-demand

That's really where the issue is. There needs to be a mechanism for decrypting old secrets. Automatically re-encrypting them with the new key would be great.

My PR checks the key length of CALENDSO_ENCRYPTION_SECRET. If it is base64 encoded with 32 bytes (as is suggested in .env) the key will always be longer than 32 characters. So this is a reliable way to tell old and new keys apart.

You will find that the code with the included check works exactly as before with existing (old) keys while also supporting new base64 keys.

const getEncoding = (key: string) => {
  if (key.length === 32) {
    return "latin1" as const;
  }
  return "base64" as const;
};

export const symmetricEncrypt = function (text: string, key: string) {
  const _key = Buffer.from(key, getEncoding(key));
  ...

Unless I am missing something?

Would also be a good time to use a more secure, authenticated encryption scheme - I suggested aes-256-gcm.

I agree, but unless I am mistaken this would be a quick and easy fix for this issue allowing the use of base64 secrets as well as legacy keys in the meantime

[nicktrn]

Just re-read my comment and I should have defined these terms:

• Key - the encryption key, i.e. CALENDSO_ENCRYPTION_SECRET
• Secret - the encrypted data, i.e. AES encrypt output

Yes, you'll be able to use both old latin1 encoded keys, and new base64 encoded keys.

But what happens to old secrets after replacing your encryption key? How would you decrypt them?

For this to work, the new key would have to be the old one, re-encoded in base64. Surely that defeats the purpose?

It would be perfectly fine for fresh deploys with empty databases. My concern is with migrations.

Hope that was clearer!

[stabildev]

@nicktrn I have implemented some of your ideas.

Unfortunately, detecting whether a secret is encrypted using the old or a new key is not straight forward. Especially, since the initial setup can be different, e.g. all existing keys could be either encrypted using the latin1 key or the base64 key, depending on when and how the project was initially set up.

Generally, the only way to find out which key was used is to try both and see which one works. The difficulty here is the definition of "works".

There are currently three kinds of data structures encrypted using the key:

• user.backupCodes: an array of strings, referenced in setup.ts and next-auth-options.ts
• credential.key: a JSON object, referenced in add.ts and _postAdd.ts of various integrations
• user.twoFactorSecret: a string with a length of 32 characters

To avoid repeating the logic (trying one key, then trying the other key, finally updating the secret using the new key) in many places, I suggest passing the expected data type as a zod schema to symmetricDecrypt, e.g.

• z.array(z.string())
• z.object(...)
• z.string().length(32)

This provides the possibility to detect whether decryption was successful, provides the opportunity to retry immediately before returning a value and providing a convenient callback for updating the secret.

I added some info to the PR description.

What are your thoughts on this approach?

[stabildev]

@PeerRich What are your thoughts on this approach?

[PeerRich]

i have very little knowledge about encryption, will loop in some people. i dont understand what is wrong with our current encryption tho

[nicktrn]

@PeerRich Essentially, we're trying to do AES-256 which requires a key length of 32 bytes, but the current way of generating and using those keys limits their randomness to 24 bytes. This results in security closer to AES-192.

As @stabildev has outlined in https://github.com/calcom/cal.com/pull/12698#issuecomment-1848804581, migration is non-trivial to solve as there is no central secret store. Secrets are handled differently in different places.

[stabildev]

@PeerRich @nicktrn

This PR doesn't change the way crypto works in Cal.com. It does not change the hashing / decrypting algorithms.

It enables the transition from the old "legacy" key format (24byte latin1) to the new, recommended format (32byte base64).

There are two ways to achieve this transition: A) Decrypt all stored hashes using the old key and re-encrypt them with the new key (Does not work for hashes used as url params in pending links)

B) Re-encrypt hashes on demand during next use – as recommended by @nicktrn

This PR uses the second approach.

This leads to a situation where hashes throughout Cal.com can be either encrypted using the old or the new key. Therefore the upated algorithm tries the NEW key first. If decryption fails* it uses the OLD key. If encryption using the old key is successful*, the "should update" callback is fired, enabling instant update of the hash wherever it was retrieved from.

Over time this leads to successive replacement of all old hashes with new, more secure hashes.

• to determine whether encryption was successful or not, a zod schema is introduced. Wherever decryption is requested, a schema is supplied that specifies the required format. E.g. "decrypt this hash. the result should be an object" or "decrypt this hash. the result should be a string with 12 characters"

I hope this clears things up!

[PeerRich]

/bonus 50 to fix type checks and unit tests

[algora-pbc[bot]]

A bonus of $50 has been added by PeerRich. @stabildev: You will receive $50 once you implement the requested changes and your PR is merged.

[stabildev]

All test are passing now

[keithwillcode]

Moving this to 4.0 milestone because we need more time based on current priorities

[zomars]

Blocking to make some changes:

Renaming CALENDSO_ENCRYPTION_KEY to CALENDSO_V2_ENCRYPTION_KEY

Renaming CALENDSO_OLD_ENCRYPTION_KEY to CALENDSO_ENCRYPTION_KEY

The reason behind this is because we don't want to have to touch all current CALENDSO_ENCRYPTION_KEY values. It's prone to a lot of human error.

Instead we want this to be opt-in. So only if CALENDSO_V2_ENCRYPTION_KEY is set. We'll use the new encryption key. Otherwise we fallback to the old encryption model.

This PR is more of a transition state until we can get rid of CALENDSO_ENCRYPTION_KEY in a later update.

I take it back. Seems like it's going to be way more effort to make this opt-in. Will merge into a local branch to make some tests before merging to main. Thanks again! @stabildev

[algora-pbc[bot]]

@stabildev: You've been awarded a $50 bounty by cal! 👉 Complete your Algora onboarding to collect the bounty.