Fix AES key wear-out #5724

X-Cli · 2025-04-16T15:44:14Z

Hey,

Here is a fix proposal for the AES key wear-out issue.

This fix is stacked upon the fix for #5706 because they both modify the same lines and separate PRs would have conflicted uselessly. Also I feel like #5723 as better chances of being merged as is than this one. This one may require some changes depending on whether you are open to adding some more bytes to the token or not.

The commit message contains some additional information, but feel free to ask any question.

fixes #5706, fixes #5705, closes #5723

Closes transloadit#5705 AES-CBC is malleable and its use is dangereous without using a encrypt-then-mac approach. AES-CCM is an AEAD that is widely available and free to use (as opposed to OCB which was patented until 2021). AES-CCM is also stronger against nonce misuse than GCM. Until transloadit#5706 is fixed, nonce misuses are possible, so CCM seems preferable, even if it is slower (2-passes).

Custom encoding functions can be removed in favor the native base64url encoding scheme.

soatok · 2025-04-17T01:03:23Z

packages/@uppy/companion/src/server/helpers/utils.js

@@ -1,5 +1,6 @@
 const crypto = require('node:crypto')

+const ivLength = 12


Since you're using this with HKDF, you can actually use a larger nonce here. (For example, 256 bits or 32 bytes.)

You can then synthetically derive the GCM nonce from the HKDF output. This is what PASETO v3/v4 do, if you need an incumbent design.

https://github.com/paseto-standard/paseto-spec/blob/master/docs/01-Protocol-Versions/Version3.md#encrypt
(See n2).

The only difference is PASETO is CTR+HMAC, so they use the full 48 bytes of HKDF output. You would only need 44 bytes.

Also, as I wrote here, this yields a 2^{-85.3} probability of a collision after 2^{85.3} encryptions with the same input key.

(a.k.a. not going to happen in practice, ever)

Thank you! This makes a lot of sense. I should have thought about that 😅 I'll amend the PR.

github-actions · 2025-04-17T07:59:04Z

Diff output files

diff --git a/packages/@uppy/companion/lib/server/helpers/utils.js b/packages/@uppy/companion/lib/server/helpers/utils.js
index 490e448..f88f6d5 100644
--- a/packages/@uppy/companion/lib/server/helpers/utils.js
+++ b/packages/@uppy/companion/lib/server/helpers/utils.js
@@ -1,6 +1,11 @@
 "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
 const crypto = require("node:crypto");
+const logger = require("../logger.js");
+const authTagLength = 16;
+const nonceLength = 16;
+const encryptionKeyLength = 32;
+const ivLength = 12;
 /**
  * @param {string} value
  * @param {string[]} criteria
@@ -59,29 +64,19 @@ module.exports.getURLBuilder = (options) => {
   return buildURL;
 };
 /**
- * Ensure that a user-provided `secret` is 32 bytes long (the length required
- * for an AES256 key) by hashing it with SHA256.
+ * Create an AES-CCM encryption key and initialization vector from the provided secret
+ * and a random nonce.
  *
  * @param {string|Buffer} secret
+ * @param {Buffer|undefined} nonce
  */
-f
8000
unction createSecret(secret) {
-  const hash = crypto.createHash("sha256");
-  hash.update(secret);
-  return hash.digest();
-}
-/**
- * Create an initialization vector for AES256.
- *
- * @returns {Buffer}
- */
-function createIv() {
-  return crypto.randomBytes(16);
-}
-function urlEncode(unencoded) {
-  return unencoded.replace(/\+/g, "-").replace(/\//g, "_").replace(/=/g, "~");
-}
-function urlDecode(encoded) {
-  return encoded.replace(/-/g, "+").replace(/_/g, "/").replace(/~/g, "=");
+function createSecrets(secret, nonce) {
+  const key = crypto.hkdfSync("sha256", secret, new Uint8Array(32), nonce, encryptionKeyLength + ivLength);
+  const buf = Buffer.from(key);
+  return {
+    key: buf.subarray(0, encryptionKeyLength),
+    iv: buf.subarray(encryptionKeyLength, encryptionKeyLength + ivLength),
+  };
 }
 /**
  * Encrypt a buffer or string with AES256 and a random iv.
@@ -91,21 +86,19 @@ function urlDecode(encoded) {
  * @returns {string} Ciphertext as a hex string, prefixed with 32 hex characters containing the iv.
  */
 module.exports.encrypt = (input, secret) => {
-  const iv = createIv();
-  const cipher = crypto.createCipheriv("aes256", createSecret(secret), iv);
-  let encrypted = cipher.update(input, "utf8", "base64");
-  encrypted += cipher.final("base64");
-  // add iv to encrypted string to use for decryption
-  return iv.toString("hex") + urlEncode(encrypted);
+  const nonce = crypto.randomBytes(nonceLength);
+  const { key, iv } = createSecrets(secret, nonce);
+  const cipher = crypto.createCipheriv("aes-256-ccm", key, iv, { authTagLength });
+  const encrypted = Buffer.concat([
+    cipher.update(input, "utf8"),
+    cipher.final(),
+    cipher.getAuthTag(),
+  ]);
+  // add nonce to encrypted string to use for decryption
+  return `${nonce.toString("hex")}${encrypted.toString("base64url")}`;
 };
-/**
- * Decrypt an iv-prefixed or string with AES256. The iv should be in the first 32 hex characters.
- *
- * @param {string} encrypted
- * @param {string|Buffer} secret
- * @returns {string} Decrypted value.
- */
-module.exports.decrypt = (encrypted, secret) => {
+// todo backwards compat for old tokens - remove in the future
+function compatDecrypt(encrypted, secret) {
   // Need at least 32 chars for the iv
   if (encrypted.length < 32) {
     throw new Error("Invalid encrypted value. Maybe it was generated with an old Companion version?");
@@ -115,7 +108,9 @@ module.exports.decrypt = (encrypted, secret) => {
   const encryptionWithoutIv = encrypted.slice(32);
   let decipher;
   try {
-    decipher = crypto.createDecipheriv("aes256", createSecret(secret), iv);
+    const secretHashed = crypto.createHash("sha256");
+    secretHashed.update(secret);
+    decipher = crypto.createDecipheriv("aes256", secretHashed.digest(), iv);
   } catch (err) {
     if (err.code === "ERR_CRYPTO_INVALID_IV") {
       throw new Error("Invalid initialization vector");
@@ -123,9 +118,45 @@ module.exports.decrypt = (encrypted, secret) => {
       throw err;
     }
   }
+  const urlDecode = (encoded) => encoded.replace(/-/g, "+").replace(/_/g, "/").replace(/~/g, "=");
   let decrypted = decipher.update(urlDecode(encryptionWithoutIv), "base64", "utf8");
   decrypted += decipher.final("utf8");
   return decrypted;
+}
+/**
+ * Decrypt an iv-prefixed or string with AES256. The iv should be in the first 32 hex characters.
+ *
+ * @param {string} encrypted hex encoded string of encrypted data
+ * @param {string|Buffer} secret
+ * @returns {string} Decrypted value.
+ */
+module.exports.decrypt = (encrypted, secret) => {
+  try {
+    const nonceHexLength = nonceLength * 2; // because hex encoding uses 2 bytes per byte
+    // NOTE: The first 32 characters are the nonce, in hex format.
+    const nonce = Buffer.from(encrypted.slice(0, nonceHexLength), "hex");
+    // The rest is the encrypted string, in base64url format.
+    const encryptionWithoutNonce = Buffer.from(encrypted.slice(nonceHexLength), "base64url");
+    // The last 16 bytes of the rest is the authentication tag
+    const authTag = encryptionWithoutNonce.subarray(-authTagLength);
+    // and the rest (from beginning) is the encrypted data
+    const encryptionWithoutNonceAndTag = encryptionWithoutNonce.subarray(0, -authTagLength);
+    if (nonce.length < nonceLength) {
+      throw new Error("Invalid encrypted value. Maybe it was generated with an old Companion version?");
+    }
+    const { key, iv } = createSecrets(secret, nonce);
+    const decipher = crypto.createDecipheriv("aes-256-ccm", key, iv, { authTagLength });
+    decipher.setAuthTag(authTag);
+    const decrypted = Buffer.concat([
+      decipher.update(encryptionWithoutNonceAndTag),
+      decipher.final(),
+    ]);
+    return decrypted.toString("utf8");
+  } catch (err) {
+    // todo backwards compat for old tokens - remove in the future
+    logger.info("Failed to decrypt - trying old encryption format instead", err);
+    return compatDecrypt(encrypted, secret);
+  }
 };
 module.exports.defaultGetKey = ({ filename }) => `${crypto.randomUUID()}-${filename}`;
 /**

packages/@uppy/companion/src/server/helpers/utils.js

Closes transloadit#5706 Using the same encryption key to encrypt an unlimited amount of data/documents results in key wear-out, in particular when using AES-CBC or other cipher modes that are not very resistant to nonce misuse. This commit derives from the application secret a fresh encryption key and a fresh initialization vector for every document to encrypt. HKDF is used to derive these values and the context info parameter is set to a fresh random value called nonce. This nonce replaces the IV in the token. Using a random value as the context info instead of using it as the salt parameter is a recommendation from https://soatok.blog/2021/11/17/understanding-hkdf/

and simplify code a bit

mifi · 2025-05-01T10:16:13Z

Thanks a lot for this! I've added backwards compatibility (so that for decrypts that fail it will fallback to the old decryption method) which we can remove in the future. I also did some touchups to the code. I tested this and it seems to be working well. I did look at the encrypted data length and it's only longer by ~20 base64 characters compared to the old output, so I don't think that's a big problem, unless I'm missing something.

| Package | Version | Package | Version | | -------------------------- | ------- | -------------------------- | ------- | | @uppy/audio | 2.1.3 | @uppy/image-editor | 3.3.3 | | @uppy/box | 3.2.3 | @uppy/instagram | 4.2.3 | | @uppy/companion | 5.7.0 | @uppy/onedrive | 4.2.4 | | @uppy/companion-client | 4.4.2 | @uppy/remote-sources | 2.3.3 | | @uppy/core | 4.4.5 | @uppy/screen-capture | 4.2.3 | | @uppy/dashboard | 4.3.4 | @uppy/unsplash | 4.3.4 | | @uppy/drag-drop | 4.1.3 | @uppy/url | 4.2.4 | | @uppy/dropbox | 4.2.3 | @uppy/utils | 6.1.4 | | @uppy/facebook | 4.2.3 | @uppy/webcam | 4.1.3 | | @uppy/file-input | 4.1.3 | @uppy/webdav | 0.3.3 | | @uppy/google-drive | 4.3.3 | @uppy/zoom | 3.2.3 | | @uppy/google-drive-picker | 0.3.5 | uppy | 4.16.0 | | @uppy/google-photos-picker | 0.3.5 | | | - @uppy/companion-client: don't reject on incorrect origin (Mikael Finstad / #5736) - @uppy/companion: implement credentials param `transloadit_gateway` (Mikael Finstad / #5725) - @uppy/companion: Fix AES key wear-out (Florian Maury / #5724) - @uppy/core: fix undefined reference when cancelling an upload (Prakash / #5730) - @uppy/audio,@uppy/box,@uppy/core,@uppy/dashboard,@uppy/drag-drop,@uppy/dropbox,@uppy/facebook,@uppy/file-input,@uppy/google-drive-picker,@uppy/google-drive,@uppy/google-photos-picker,@uppy/image-editor,@uppy/instagram,@uppy/onedrive,@uppy/remote-sources,@uppy/screen-capture,@uppy/unsplash,@uppy/url,@uppy/utils,@uppy/webcam,@uppy/webdav,@uppy/zoom: ts: make locale strings optional (Merlijn Vos / #5728)

| Package | Version | Package | Version | | -------------------------- | ------- | -------------------------- | ------- | | @uppy/audio | 2.1.3 | @uppy/image-editor | 3.3.3 | | @uppy/box | 3.2.3 | @uppy/instagram | 4.2.3 | | @uppy/companion | 5.7.0 | @uppy/onedrive | 4.2.4 | | @uppy/companion-client | 4.4.2 | @uppy/remote-sources | 2.3.3 | | @uppy/core | 4.4.5 | @uppy/screen-capture | 4.2.3 | | @uppy/dashboard | 4.3.4 | @uppy/unsplash | 4.3.4 | | @uppy/drag-drop | 4.1.3 | @uppy/url | 4.2.4 | | @uppy/dropbox | 4.2.3 | @uppy/utils | 6.1.4 | | @uppy/facebook | 4.2.3 | @uppy/webcam | 4.1.3 | | @uppy/file-input | 4.1.3 | @uppy/webdav | 0.3.3 | | @uppy/google-drive | 4.3.3 | @uppy/zoom | 3.2.3 | | @uppy/google-drive-picker | 0.3.5 | uppy | 4.16.0 | | @uppy/google-photos-picker | 0.3.5 | | | - meta: Revert "Release: uppy@4.16.0 (#5750)" (Mikael Finstad) - meta: force cdn upload (Mikael Finstad) - meta: fix invalid brach option (now ref) (Mikael Finstad) - meta: improve release script output (Mikael Finstad) - meta: fix error (Mikael Finstad) - meta: Release: uppy@4.16.0 (github-actions[bot] / #5750) - meta: Fix node versions (Mikael Finstad / #5740) - @uppy/companion-client: don't reject on incorrect origin (Mikael Finstad / #5736) - @uppy/companion: implement credentials param `transloadit_gateway` (Mikael Finstad / #5725) - @uppy/companion: Fix AES key wear-out (Florian Maury / #5724) - @uppy/core: fix undefined reference when cancelling an upload (Prakash / #5730) - @uppy/audio,@uppy/box,@uppy/core,@uppy/dashboard,@uppy/drag-drop,@uppy/dropbox,@uppy/facebook,@uppy/file-input,@uppy/google-drive-picker,@uppy/google-drive,@uppy/google-photos-picker,@uppy/image-editor,@uppy/instagram,@uppy/onedrive,@uppy/remote-sources,@uppy/screen-capture,@uppy/unsplash,@uppy/url,@uppy/utils,@uppy/webcam,@uppy/webdav,@uppy/zoom: ts: make locale strings optional (Merlijn Vos / #5728)

* main: Release: uppy@4.16.0 (#5754) Revert "Release: uppy@4.16.0 (#5753)" fix botched prettier formatting Release: uppy@4.16.0 (#5753) Revert "Release: uppy@4.16.0 (#5752)" make release job less prone to crash Release: uppy@4.16.0 (#5752) Revert "Release: uppy@4.16.0 (#5751)" fix release script Release: uppy@4.16.0 (#5751) Revert "Release: uppy@4.16.0 (#5750)" force cdn upload fix invalid brach option (now ref) improve release script output fix error Release: uppy@4.16.0 (#5750) Fix node versions (#5740) don't reject on incorrect origin (#5736) implement credentials param `transloadit_gateway` (#5725) Fix AES key wear-out (#5724)

X-Cli added 2 commits April 16, 2025 17:13

refactor: uses base64url encoding

131f4ad

Custom encoding functions can be removed in favor the native base64url encoding scheme.

soatok approved these changes Apr 17, 2025

View reviewed changes

soatok reviewed Apr 17, 2025

View reviewed changes

X-Cli marked this pull request as draft April 17, 2025 06:45

X-Cli force-pushed the fix-5706-key-wear-out branch from 3050eec to 54c3531 Compare April 17, 2025 07:57

X-Cli marked this pull request as ready for review April 17, 2025 10:33

X-Cli commented Apr 21, 2025

View reviewed changes

packages/@uppy/companion/src/server/helpers/utils.js Outdated Show resolved Hide resolved

X-Cli force-pushed the fix-5706-key-wear-out branch from 54c3531 to 58575c2 Compare April 21, 2025 14:11

Murderlon requested a review from mifi April 28, 2025 13:11

make decrypt backwards compat

2505016

and simplify code a bit

mifi approved these changes May 1, 2025

View reviewed changes

mifi merged commit 06d9a7c into transloadit:main May 1, 2025
20 checks passed

github-actions bot mentioned this pull request May 18, 2025

Release: uppy@4.16.0 #5750

Merged

github-actions bot mentioned this pull request May 18, 2025

Release: uppy@4.16.0 #5751

Merged

github-actions bot mentioned this pull request May 18, 2025

Release: uppy@4.16.0 #5752

Merged

github-actions bot mentioned this pull request May 18, 2025

Release: uppy@4.16.0 #5753

Merged

github-actions bot mentioned this pull request May 18, 2025

Release: uppy@4.16.0 #5754

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix AES key wear-out #5724

Fix AES key wear-out #5724

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

		@@ -1,5 +1,6 @@
		const crypto = require('node:crypto')

		const ivLength = 12

Fix AES key wear-out #5724

Fix AES key wear-out #5724

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!