deno icon indicating copy to clipboard operation
deno copied to clipboard

Deno decrypt AES-CBC, OperationError: Decryption failed

Open iugo opened this issue 3 years ago • 10 comments

node code:

"use strict";

const Crypto = require("crypto");

function decrypt(encrypted, aesKey) {
  const iv = aesKey.slice(0, 16);
  const crypto = Crypto.createDecipheriv("AES-256-CBC", aesKey, iv);
  const decrypt = Buffer.concat([crypto.update(encrypted)]);
  return decrypt.toString();
}

const ENCODING_AES_KEY = new Uint8Array([
  83, 155, 46, 22, 48, 5, 120, 88, 32, 156, 75, 248, 201, 177, 241, 32, 208, 64,
  69, 118, 7, 66, 162, 28, 181, 231, 68, 120, 206, 29, 8, 44,
]);

const decryptRes = decrypt(
  new Uint8Array([
    237, 20, 86, 9, 112, 185, 229, 119, 195, 63, 122, 156, 59, 82, 210, 255, 75,
    80, 90, 154, 34, 237, 222, 91, 111, 160, 218, 14, 224, 204, 104, 23, 31,
    204, 221, 68, 207, 200, 161, 31, 207, 182, 191, 105, 235, 207, 251, 6, 222,
    228, 139, 184, 43, 165, 190, 67, 57, 136, 55, 110, 122, 130, 182, 31, 204,
    102, 177, 35, 177, 77, 163, 133, 154, 183, 52, 95, 154, 53, 114, 138, 138,
    224, 190, 63, 247, 222, 181, 170, 23, 97, 78, 172, 206, 35, 146, 47,
  ]),
  ENCODING_AES_KEY
);

console.log(decryptRes);

deno code:

Deno.test('decrypt-aescbc-deno', async () => {
  const u8aKey = new Uint8Array([
    83, 155, 46, 22, 48, 5, 120, 88, 32, 156, 75, 248, 201, 177, 241, 32, 208,
    64, 69, 118, 7, 66, 162, 28, 181, 231, 68, 120, 206, 29, 8, 44,
  ]);

  const key = await crypto.subtle.importKey(
    'raw',
    u8aKey,
    {
      name: 'AES-CBC',
    },
    false,
    ['encrypt', 'decrypt']
  );

  const res = new Uint8Array([
    237, 20, 86, 9, 112, 185, 229, 119, 195, 63, 122, 156, 59, 82, 210, 255, 75,
    80, 90, 154, 34, 237, 222, 91, 111, 160, 218, 14, 224, 204, 104, 23, 31,
    204, 221, 68, 207, 200, 161, 31, 207, 182, 191, 105, 235, 207, 251, 6, 222,
    228, 139, 184, 43, 165, 190, 67, 57, 136, 55, 110, 122, 130, 182, 31, 204,
    102, 177, 35, 177, 77, 163, 133, 154, 183, 52, 95, 154, 53, 114, 138, 138,
    224, 190, 63, 247, 222, 181, 170, 23, 97, 78, 172, 206, 35, 146, 47,
  ]);

  const iv = u8aKey.slice(0, 16);
  const d = await crypto.subtle.decrypt(
    {
      name: 'AES-CBC',
      iv,
    },
    key,
    res
  );

  const str = new TextDecoder().decode(d);
  console.log('str', str);
});

Node.js res is enngahLCFzK8tHkl{"EventType":"check_url"}dingbecac0crlgnsqv8k.

But Deno cannot be executed, and the error message is:

error: OperationError: Decryption failed
  const d = await crypto.subtle.decrypt(
            ^
    at async SubtleCrypto.decrypt (deno:ext/crypto/00_crypto.js:598:29)
  • deno 1.24.2 (release, x86_64-apple-darwin)
  • v8 10.4.132.20
  • typescript 4.7.4

iugo avatar Aug 08 '22 08:08 iugo

As a temporary workaround, don't use the standard library, but use another JS implementation.

import { Aes } from 'https://deno.land/x/[email protected]/aes.ts';
import { Cbc } from 'https://deno.land/x/[email protected]/block-modes.ts';
Deno.test('test2', () => {
  const key = new Uint8Array([
    83, 155, 46, 22, 48, 5, 120, 88, 32, 156, 75, 248, 201, 177, 241, 32, 208,
    64, 69, 118, 7, 66, 162, 28, 181, 231, 68, 120, 206, 29, 8, 44,
  ]);
  const iv = key.slice(0, 16);

  const decipher = new Cbc(Aes, key, iv);

  const decrypted = decipher.decrypt(
    new Uint8Array([
      237, 20, 86, 9, 112, 185, 229, 119, 195, 63, 122, 156, 59, 82, 210, 255,
      75, 80, 90, 154, 34, 237, 222, 91, 111, 160, 218, 14, 224, 204, 104, 23,
      31, 204, 221, 68, 207, 200, 161, 31, 207, 182, 191, 105, 235, 207, 251, 6,
      222, 228, 139, 184, 43, 165, 190, 67, 57, 136, 55, 110, 122, 130, 182, 31,
      204, 102, 177, 35, 177, 77, 163, 133, 154, 183, 52, 95, 154, 53, 114, 138,
      138, 224, 190, 63, 247, 222, 181, 170, 23, 97, 78, 172, 206, 35, 146, 47,
    ])
  );

  console.log(decrypted, new TextDecoder().decode(decrypted));
});

iugo avatar Aug 08 '22 09:08 iugo

This isn't a problem with Deno. Your res value does not match the expected encrypted value, the following only fails on comparing the expected res to the actual encrypted value:

import { assertEquals } from "https://deno.land/[email protected]/testing/asserts.ts";

Deno.test("decrypt-aescbc-deno", async () => {
  // deno-fmt-ignore
  const u8aKey = new Uint8Array([
    83, 155, 46, 22, 48, 5, 120, 88, 32, 156, 75, 248, 201, 177, 241, 32, 208,
    64, 69, 118, 7, 66, 162, 28, 181, 231, 68, 120, 206, 29, 8, 44,
  ]);

  const key = await crypto.subtle.importKey(
    "raw",
    u8aKey,
    {
      name: "AES-CBC",
    },
    false,
    ["encrypt", "decrypt"],
  );

  const data = new TextEncoder().encode(
    `enngahLCFzK8tHkl{"EventType":"check_url"}dingbecac0crlgnsqv8k`,
  );

  // deno-fmt-ignore
  const res = new Uint8Array([
    237, 20, 86, 9, 112, 185, 229, 119, 195, 63, 122, 156, 59, 82, 210, 255, 75,
    80, 90, 154, 34, 237, 222, 91, 111, 160, 218, 14, 224, 204, 104, 23, 31,
    204, 221, 68, 207, 200, 161, 31, 207, 182, 191, 105, 235, 207, 251, 6, 222,
    228, 139, 184, 43, 165, 190, 67, 57, 136, 55, 110, 122, 130, 182, 31, 204,
    102, 177, 35, 177, 77, 163, 133, 154, 183, 52, 95, 154, 53, 114, 138, 138,
    224, 190, 63, 247, 222, 181, 170, 23, 97, 78, 172, 206, 35, 146, 47,
  ]);

  const iv = u8aKey.slice(0, 16);
  const ab = await crypto.subtle.encrypt({ name: "AES-CBC", iv }, key, data);

  const d = await crypto.subtle.decrypt(
    {
      name: "AES-CBC",
      iv,
    },
    key,
    ab,
  );

  const str = new TextDecoder().decode(d);
  console.log("str", str);
  assertEquals(res, new Uint8Array(ab));
});

kitsonk avatar Aug 08 '22 09:08 kitsonk

And running your test case through Chrome shows a very similar error:

Cursor_and_New_Tab

Your "fixture" is not encrypted properly, and failures of decryption are intentionally vague to ensure that details of "what went wrong" are not leaked and become an attack vector.

kitsonk avatar Aug 08 '22 09:08 kitsonk

@kitsonk With the same key and encrypted data, Node.js will execute successfully. And https://deno.land/x/[email protected] will also get the correct result.

Kind of weird.

iugo avatar Aug 08 '22 10:08 iugo

What happens when you use Node.js web crypto instead?

kitsonk avatar Aug 08 '22 10:08 kitsonk

The problem seems to be with the encoding.

"use strict";

const Crypto = require("crypto");

const ENCODING_AES_KEY = new Uint8Array([
  83, 155, 46, 22, 48, 5, 120, 88, 32, 156, 75, 248, 201, 177, 241, 32, 208, 64,
  69, 118, 7, 66, 162, 28, 181, 231, 68, 120, 206, 29, 8, 44,
]);

// return like deno
function encrypt(str, aesKey) {
  const iv = aesKey.slice(0, 16);
  let cipher = Crypto.createCipheriv("aes-256-cbc", aesKey, iv);
  let encrypted = cipher.update(str, "utf8", "base64");
  encrypted += cipher.final("base64");
  return encrypted;
}

const encrypted = encrypt(
  'enngahLCFzK8tHkl{"EventType":"check_url"}dingbecac0crlgnsqv8k',
  ENCODING_AES_KEY
);

console.log("encrypt", encrypted);

function decryptWithFinal(encrypted, aesKey) {
  const iv = aesKey.slice(0, 16);
  const crypto = Crypto.createDecipheriv("AES-256-CBC", aesKey, iv);
  let decrypt = crypto.update(encrypted, "base64");
  decrypt += crypto.final("utf8");
  return decrypt.toString();
}

console.log("decryptWithFinal", decryptWithFinal(encrypted, ENCODING_AES_KEY));

function decryptWithoutFinal(encrypted, aesKey) {
  const iv = aesKey.slice(0, 16);
  const crypto = Crypto.createDecipheriv("AES-256-CBC", aesKey, iv);
  let decrypt = crypto.update(encrypted, "base64");
  return decrypt.toString();
}

console.log(
  "decryptWithoutFinal",
  decryptWithoutFinal(
    "7RRWCXC55XfDP3qcO1LS/0tQWpoi7d5bb6DaDuDMaBcfzN1Ez8ihH8+2v2nrz/sG3uSLuCulvkM5iDdueoK2H8xmsSOxTaOFmrc0X5o1coqK4L4/9961qhdhTqzOI5Iv",
    ENCODING_AES_KEY
  )
);

iugo avatar Aug 08 '22 12:08 iugo

Seems like encoding differences between web crypto and Node.js crypto. Is there anything actionable here?

kitsonk avatar Aug 08 '22 22:08 kitsonk

I don't know the real difference between decipher.update() and decipher.final() in Node.js. But the problem here seems to be that when someone else encrypts using a specific method. Use if decrypt without using decipher.final() can get the data.

Can Deno provide some low-level function implementations to implement update() and final() respectively?

iugo avatar Aug 09 '22 08:08 iugo

Deno adheres to web crypto which is what browsers implement and Node.js also supports. Those expose the APIs they expose.

If the W3C were to implement lower level APIs in the standard, we would follow suit. Implementing APIs directly in Deno outside of the standards is not desirable, as it may very well expose security vulnerabilities, but also be incompatible with browsers and other implementors of web crypto.

kitsonk avatar Aug 09 '22 09:08 kitsonk

I had a similar issue while encrypting some data around server and client and could not rely on the TextDecoder class for the same reason:

error: OperationError: Decryption failed

After some hours I played around and found a solution, a bit tricky, but is dependency-free:

const textEncoder = new TextEncoder();
const textDecoder = new TextDecoder();

//  we import the key that we have previously generated with the same algorithm
const cryptoKey = await crypto.subtle.importKey(
    "raw",
    textEncoder.encode("R�{���M��~`R�@☺h"),
    "AES-CBC",
    true,
    ["encrypt", "decrypt"],
);

// we generate the IV
const iv = crypto.getRandomValues(new Uint8Array(16));

// here is the string we want to encrypt
const stringToEncrypt = "foobar"

// we encrypt
const encryptedString = await crypto.subtle.encrypt(
    { name: "AES-CBC", iv: iv },
    cryptoKey,
    textEncoder.encode(stringToEncrypt),
);

// we transform the encrypted string to an UInt8Array
const uint8ArrayEncryptedString = new Uint8Array(encryptedString);

// we transform the Array to a String so we have a representation we can carry around
const stringifiedEncryption = String.fromCharCode(...uint8ArrayEncryptedString);

/* now is time to decrypt again the message, so we transform the string into a char array and for every iteration we transform 
 the char into a byte, so in the end we have a byte array
*/
const stringByteArray = [...stringifiedEncryption].map((v) => v.charCodeAt(0))

// we transform the byte array into a Uint8Array buffer
const stringBuffer = new Uint8Array(stringByteArray.length);

// we load the buffer
stringByteArray.forEach((v, i) => stringBuffer[i] = v)

// we decrypt again
const againDecrString = await crypto.subtle.decrypt(
    { name: "AES-CBC", iv: iv },
    cryptoKey,
    stringBuffer,
);

// enjoy
console.log(textDecoder.decode(againDecrString))

So in the end I believe that somewhere there is a loss of bytes and maybe is a bug. So we are waiting for the W3C @kitsonk ?

starrematte avatar Sep 12 '22 16:09 starrematte

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Nov 19 '22 15:11 stale[bot]

@iugo There are a number of issues with your code that should be checked.

  1. AES-256-CBC adds (encryption) and automatically removes (decryption) padding bytes, that MUST obey the relevant standard (called PKCS#7). This is because AES is a block cipher, that will only encrypt/decrypt blocks of 16 bytes. So, if your data is not a multiple of 16 bytes, padding will need to be inserted at the end (and even if the data IS a multiple of 16 bytes, padding will still be added, so that the 'unpadding' algorithm does not get lost).

I manually decrypted your 'res', to show the raw encrypted data with padding:

0000 65 6e 6e 67 61 68 4c 43 46 7a 4b 38 74 48 6b 6c
0010 00 00 00 19 7b 22 45 76 65 6e 74 54 79 70 65 22
0020 3a 22 63 68 65 63 6b 5f 75 72 6c 22 7d 64 69 6e
0030 67 62 65 63 61 63 30 63 72 6c 67 6e 73 71 76 38
0040 6b 1f 1f 1f 1f 1f 1f 1f 1f 1f 1f 1f 1f 1f 1f 1f
0050 1f 1f 1f 1f 1f 1f 1f 1f 1f 1f 1f 1f 1f 1f 1f 1f

Here we can see that the block is padded using 31 x '1F' bytes. This is not to spec (RFC2315) where padding is limited to 'k' the block size, in this case 16. 0x1F is 31, above this limit.

See this and RFC2315

  1. TextEncoder Note that in the middle of the data (offset 0x10) there are 4 non-text bytes (00 00 00 19). This is not represented in the above issue and examples, and thus your example code that encrypts from enngahLCFzK8tHkl{"EventType":"check_url"}dingbecac0crlgnsqv8k is not correct.

As for errors from TextDecoder, maybe you should check if these 'non-text' bytes are valid input for UTF-8 encoding.

  1. Node Cipher/Decipher methods. Due to the padding requirements, also implemented by Node, you MUST call .final after calls to .update, in order to include or verify/remove the padding. If you just call .update(), as in your 1st example, node will just decrypt the data without checking the padding, and will "hold" back at least the block at 0x50 for pad checking, which does not happen. You MUST call . final() to check the padding, then you will find that Node will also reject your data.

So, conclusions -

  • The example CIPHERTEXT is INVALID. Can you check what/who generated it? How did they achieve the invalid 0x1F padding? Maybe used key-size (32 for AES-256) instead of block-size (16).

  • The example PLAINTEXT is INCORRECT, missing the 'non-text' bytes.

  • Deno's implementation of WebCrypto is CORRECT, is verifying the padding after internally decrypting your ciphertext, and is throwing an error due to INVALID PADDING.

  • Node's implementation is also CORRECT, the example uses an invalid call sequence and, purely as a side-effect, the decryption is working

cryptographix avatar Dec 05 '22 14:12 cryptographix

@cryptographix Thank you very much. However, the data that needs to be decrypted was sent to me by DingTalk, and I just want to read it. I know that DingTalk's encryption is wrong, but I can't ask DingTalk to fix this problem, I can only find a way to make it compatible.

Thank you for taking the time to explain. Your comment has given me a deeper understanding of encryption and decryption, thank you.

iugo avatar Dec 09 '22 10:12 iugo

@iugo Here's part of my code that I used for checking. If you want to use Deno WebCrypto, there is a workaround you can use, which is to add a new block of correct padding (ByteArray of 16 x 0x16, XOR with last block encrypted), something like:

const AES_BLOCK_SIZE = 16;

const AES_ECB_ENCRYPT_BLOCK_WITH_IV = async (data: ArrayBuffer, key: CryptoKey, iv: ArrayBuffer) => {
  return new Uint8Array(await crypto.subtle.encrypt(
    {
      name: 'AES-CBC',
      iv,
    },
    key,
    data
  )).slice(0, AES_BLOCK_SIZE);
};

const AES_CBC_DECRYPT_NO_PADDING = async (data: ArrayBuffer, key: CryptoKey, iv: ArrayBuffer) => {
  const paddedData = new Uint8Array(new Uint8Array(data).length + AES_BLOCK_SIZE);
  paddedData.set(new Uint8Array(data), 0);

  // Fabricate a correct padding block and concatenate, this making sure webcrypto CBC decryption does not complain
  const padding = new Uint8Array(16)
  padding.fill(0x10);

  const padBlock = await AES_ECB_ENCRYPT_BLOCK_WITH_IV(padding, key, data.slice(-AES_BLOCK_SIZE));
  paddedData.set(padBlock, AES_BLOCK_SIZE);

  return new Uint8Array(await crypto.subtle.decrypt(
    {
      name: 'AES-CBC',
      iv,
    },
    key,
    paddedData
  )).slice(0, AES_BLOCK_SIZE);
}

You can use AES_CBC_DECRYPT_NO_PADDING() to decrypt your data, then manually remove the padding.

Best wishes.

@iugo please close this issue as "resolved - not a problem with DENO", or ask @kitsonk.

cryptographix avatar Dec 09 '22 13:12 cryptographix