CAST5 file decryption hangs indefinitely

[T-parrish]

• OpenPGP.js version: 5.9.0
• Affected platform (Browser or Node.js version): 18.17.0

When attempting to decrypt a 512kb csv file, the decryption process hangs indefinitely and the CPU spins up to 100%.

const fixturePath = `util/fixtures`;
const private_key = fs.readFileSync('util/fixtures/test_priv_key.txt', 'utf-8');

const file_stream = fs.createReadStream(`${fixturePath}/test_encrypted.csv`);

const message = await openpgp.readMessage({ binaryMessage: file_stream });

const pk = await openpgp.decryptKey({
  privateKey: await openpgp.readKey({
    armoredKey: private_key.toString(),
  }),
  passphrase: 'password',
});

const { data } = await openpgp.decrypt({
  message, 
  verificationKeys: [], 
  decryptionKeys: pk, 
});

A few other details of note:

• Sometimes it will complete when using the debugger, but never when calling the decryption script directly
• It works just fine on Node 16.16.0
• If I decrypt the file using Node 16.16.0 and re-encrypt the file using OpenPGP 5.9.0, I can decrypt the newly encrypted file without issue using node 18.17.0
• There are no issues with OpenPGP V4
• Unfortunately, I cannot share the file in question since it contains sensitive information

This seems like a race condition of some sort, but I could be wrong. Any help or suggestions to debug this would be greatly appreciated - we would love to upgrade our codebase to OpenPGP V5 but without understanding the root cause of this issue and building tests to catch this moving forward, we are unable to do so.

[twiss]

Hey 👋 Could you perhaps generate a new test key and file using V4, in a similar setup as these files were generated in, then check if they also reproduce the issue, and if so share them here, to make it easier to reproduce?

[larabr]

When you say "the decryption process hangs indefinitely", does it mean that the code after decrypt is not run at all?
If the call returns, are you pulling the decrypted data stream afterwards? That's required for decryption to actually proceed.

If it's decrypt() that hangs, sharing the encrypted file (test_encrypted.csv) could also be helpful, to check that everything looks good with it. Would sending it via email to contact@openpgpjs.org work for you?

[T-parrish]

Yes, the call to decrypt() never returns, but only when using node >= 18 and openPGP.js 5.10.1. The file decrypts successfully using the exact same code using node 16 and openPGP.js > 5.10.1.

After debugging a bit more, we have a few more insights to share - the following is sanitized output from using GPG directly:

% gpg --list-packets 20230724_xxxx_encrypted.csv
gpg: encrypted with rsa4096 key, ID xx, created 2020-09-23
      "xxxx"
gpg: using "xxxx" as default secret key for signing
gpg: used key is not marked for encryption use.
gpg: WARNING: cipher algorithm CAST5 not found in recipient preferences
gpg: WARNING: message was not integrity protected
gpg: Hint: If this message was created before the year 2003 it is
     likely that this message is legitimate.  This is because back
     then integrity protection was not widely used.
gpg: Use the option '--ignore-mdc-error' to decrypt anyway.
gpg: decryption forced to fail!
# off=0 ctb=85 tag=1 hlen=3 plen=524
:pubkey enc packet: version 3, algo 1, keyid xxxxx
    data: [4095 bits]
# off=527 ctb=c9 tag=9 hlen=6 plen=522568 new-ctb
:encrypted data packet:
    length: 522568
# off=543 ctb=a3 tag=8 hlen=1 plen=0 indeterminate
:compressed packet: algo=1
# off=545 ctb=cb tag=11 hlen=6 plen=1544009 new-ctb
:literal data packet:
    mode b (62), created 1690196205, name="20230724_xxxx.csv",
    raw data: 1543971 bytes

These are the config flags we're using for decryption:

openpgp.config.allowInsecureDecryptionWithSigningKeys = true;
openpgp.config.allowUnauthenticatedMessages = true;
openpgp.config.allowUnauthenticatedStream = true;

Since the encrypted file contains sensitive information, we'd like to exhaust all potential avenues for debugging before sending it over - are there any other suggestions you can offer?

[T-parrish]

Hey 👋 Could you perhaps generate a new test key and file using V4, in a similar setup as these files were generated in, then check if they also reproduce the issue, and if so share them here, to make it easier to reproduce?

@twiss - unfortunately, the files sent to us are encrypted by our clients using a variety of different mechanisms - it is unclear what method our client used to encrypt this file, but we have been unable to reproduce the issue outside of this file. (We have tried OpenPGP V4, V5, and GPG)

We have no issues decrypting this file using GPG, OpenPGP V4, or OpenPGP V5 and Node 16. The only time it breaks is with OpenPGP V5 and Node 18+. This feels like a race condition somewhere; if the file were corrupted, I don't imagine we'd be able to decrypt it at all - but that is not the case. Do you have any suggestions for ways we can continue to debug this and/or reproduce this issue?

[larabr]

I don't think we can do much without the file itself. Based on the GPG output, it's a very old & messy file, with a deprecated encryption mechanism. The reason why re-encrypting it works, is simply because it gets re-encrypted using a modern construction.

Yes, the call to decrypt() never returns

Can you try and spot where it hangs? One way to do this, without a debugger, is to edit the OpenPGP.js package.json in the node_modules, to point to the non-minified OpenPGP.js dist for Node. Then you can try and change the code in the target openpgpjs bundle under dist, by adding console.logs in decrypt() and inner calls. We could have a live debugging session if it helps.

I think the issue might be related to #1449 , even tho it's unclear to me why the Node version would make a difference .

Edit: just to be sure, are you testing on Node 16 and 18 on the same machine, or different ones?

[T-parrish]

Thank you for the guidance @larabr - confirming that the tests for Node 16 and Node 18 are being run on the same machine using NVM to handle swapping between runtimes.

I'll try your suggestion for adding logs to the non-minified dist and see if I can pinpoint where this is breaking - if that doesn't work I'll reach out to see if we can dig into this together. Really appreciate your help here!

[T-parrish]

I have a few updates to share after debugging against the non-minified dist:

Reader.prototype.readToEnd = async function(join=concat) {
  const result = [];
  while (true) {
    // hangs here
    const { done, value } = await this.read();
    if (done) break;
    result.push(value);
  }
  return join(result);
};

Reader.prototype.read = async function() {
  if (this[externalBuffer] && this[externalBuffer].length) {
    const value = this[externalBuffer].shift();
    return { done: false, value };
  }
  // Never returns
  return this._read();
};

From stepping through the code, the readToEnd method for the stream doesn't get a return value from the final call to this._read() and hangs indefinitely.

[larabr]

Thanks for the info @T-parrish , we'll look into what might be going wrong with the streaming.
We think it might have to do with internal conversions from Node to WebStreams.
Could you try converting the stream manually using the stream.Readable.toWeb helper in Node 18, before passing it to readMessage?

BTW for this specific file, due to its legacy construction (require passing config.allowUnauthenticatedMessages), streaming is not useful, in the sense that the data is buffered in memory regardless as part of decryption (see #1512).
So to go around the problem you are facing, you could also try and read the whole file in advance and pass a Uint8Array to readMessage instead of the stream. Could you check if this works ?

[T-parrish]

Attempted to pass a manually converted Web Stream to the readMessage() method - decrypt() still hangs indefinitely:

const file_stream = stream.Readable.toWeb(fs.createReadStream(`${fixturePath}/test_encrypted.csv`));
const message = await openpgp.readMessage({ binaryMessage: file_stream }); 
...

const { data } = await openpgp.decrypt({
  message, 
  verificationKeys: [], 
  decryptionKeys: pk, 
});

I also tried reading the entire file into a Uint8Array - in this case, it seems like it may finish decrypting over the span of several hours (file size ~520kb) :

let buffs = [];
const file_obj = fs.createReadStream(`${fixturePath}/enc_test_2.csv`);

for await (const data of file_obj) {
  buffs.push(data)
}

buffs = Buffer.concat(buffs)

const message = await openpgp.readMessage({ binaryMessage: buffs }); 

Stepping through the decrypt() method, things get tied up in this loop where the size of the decblock container shrinks by 8 every iteration:

let ct = new Uint8Array();
const process = chunk => {
  if (chunk) {
    ct = util.concatUint8Array([ct, chunk]);
  }
  const plaintext = new Uint8Array(ct.length);
  let i;
  let j = 0;
  while (chunk ? ct.length >= block_size : ct.length) {
    const decblock = cipherfn.encrypt(blockp);
    blockp = ct;
    for (i = 0; i < block_size; i++) {
      plaintext[j++] = blockp[i] ^ decblock[i];
    }
    ct = ct.subarray(block_size);
  }
  return plaintext.subarray(0, j);
};

It's unclear if the file will actually finish decrypting once the size of the decblock container reaches 0 or if this is unexpected behavior, but this seems like it could be an important detail to share. Hope this helps, but let me know if I can provide any more context / information.

[twiss]

Ah, you're reaching the JS implementation of CFB. That's a bit surprising as normally it's built-in to Node Crypto, and we use the native implementation if it's available. However, it seems that Cast5-CFB was removed in Node 18:

$ nvm use 16
Now using node v16.20.2 (npm v8.19.4)
$ node -e 'console.log(require("crypto").getCiphers().includes("cast5-cfb"))'
true
$ nvm use 18
Now using node v18.17.1 (npm v9.6.7)
$ node -e 'console.log(require("crypto").getCiphers().includes("cast5-cfb"))'
false

(while e.g. AES-*-CFB is still there).

The behavior you mentioned is expected, I believe, so this would point to it being more of a performance issue rather than an infinite loop, for example. (And, thanks for the additional details!)

Though, it's still a bit surprising to me that decrypting 520kb would take several hours. I'm also not sure why it'd be different in OpenPGP.js v4, I don't think we changed much here. Perhaps, could you step through the decryption using v4 as well and see if there's any observable difference?

[larabr]

Thanks for digging into this @T-parrish , there is actually a CFB-decryption performance issue for legacy (non-AES) ciphers, I could repro in the browser even for files with the MDC.
Could you pull this patch and see if it fixes the problem? #1679 .

The affected code should be the same as v4 though. Did you test v4 on Node 18 too?

[T-parrish]

@twiss - Confirming that the issue is present when using OpenPGP V4 and Node 18.
@larabr - the patch seems to fix the issue, really appreciate your help here. Will this change be merged into 5.10.2? If so, what is the timeline for the next release?

[larabr]

@T-parrish we just released v5.10.2 🙂