Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs,crypto: AAD decryption of fs stream > 32768 bytes fails #31733

Closed
fenying opened this issue Feb 9, 2020 · 6 comments
Closed

fs,crypto: AAD decryption of fs stream > 32768 bytes fails #31733

fenying opened this issue Feb 9, 2020 · 6 comments
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem. fs Issues and PRs related to the fs subsystem / file system. stream Issues and PRs related to the stream subsystem.

Comments

@fenying
Copy link

fenying commented Feb 9, 2020

  • Node.js Version: v12.15.0
  • OS: Linux 5.5.2-arch1-1
  • Scope (install, code, runtime, meta, other?): code
  • Module (and version) (if relevant): crypto (openssl: '1.1.1d')

With a 32768 bytes message, the AES-128-CCM cipher and decipher both work well.
With a 32769 bytes message, the AES-128-CCM cipher works well, but the decipher failed with a message:

Error: Unsupported state or unable to authenticate data
    at Decipheriv._flush (internal/crypto/cipher.js:139:29)
    at Decipheriv.prefinish (_stream_transform.js:140:10)
    at Decipheriv.emit (events.js:223:5)
    at prefinish (_stream_writable.js:670:14)
    at finishMaybe (_stream_writable.js:678:5)
    at endWritable (_stream_writable.js:698:3)
    at Decipheriv.Writable.end (_stream_writable.js:627:5)
    at ReadStream.onend (_stream_readable.js:693:10)
    at Object.onceWrapper (events.js:312:28)
    at ReadStream.emit (events.js:228:7)

I can't understand why. Whatever I change the AAD/IV/authTagLength, it can‘t work.

Here is my code:

// execute after: dd if=/dev/random of=random.bin count=1 bs=32769
const $Crypto = require("crypto");
const $fs = require("fs");

const key = $Crypto.randomBytes(16);

const iv = $Crypto.randomBytes(8);

const aad = $Crypto.randomBytes(1);

const stream = $Crypto.createCipheriv("aes-128-ccm", key, iv, {
    authTagLength: 16
});

stream.setAAD(aad, {
    plaintextLength: 32769
});

$fs.createReadStream("./random.bin").pipe(stream).pipe(
    $fs.createWriteStream("./random.bin.ciphertext")
).on("finish", function() {

    console.info("encrypted");

    const destream = $Crypto.createDecipheriv("aes-128-ccm", key, iv, {
        authTagLength: 16
    });
    
    destream.setAAD(aad, {
        plaintextLength: 32769
    });

    destream.setAuthTag(stream.getAuthTag());
    
    $fs.createReadStream("./random.bin.ciphertext").pipe(destream).pipe(
        $fs.createWriteStream("./random.bin.plaintext")
    );
});
@bnoordhuis
Copy link
Member

See here - the maximum message size is a function of the IV size. Make it bigger and it'll work.

@fenying
Copy link
Author

fenying commented Feb 11, 2020

@bnoordhuis Thx guy, but I have read this article and tried it before. And It doesn't work even if I increase the length of IV to 13 bytes, which is the maximum length of CCM IV.

const iv = $Crypto.randomBytes(13);

@bnoordhuis bnoordhuis transferred this issue from nodejs/help Feb 11, 2020
@bnoordhuis bnoordhuis added crypto Issues and PRs related to the crypto subsystem. fs Issues and PRs related to the fs subsystem / file system. stream Issues and PRs related to the stream subsystem. labels Feb 11, 2020
@bnoordhuis
Copy link
Member

I'm moving this to nodejs/node because this looks like a timing related bug with fs streams. I can reproduce what you're describing but also observe that AAD decryption works with other stream types.

@bnoordhuis
Copy link
Member

I don't have time to investigate right now but I've opened #31734 with a known issues test as a starting point for others.

@bnoordhuis bnoordhuis added the confirmed-bug Issues with confirmed bugs. label Feb 11, 2020
@bnoordhuis bnoordhuis changed the title Why AES-128-CCM decipher failed when message larger than 32768 bytes fs,crypto: AAD decryption of fs stream > 32768 bytes fails Feb 11, 2020
@fenying
Copy link
Author

fenying commented Feb 11, 2020

@bnoordhuis Okay, Thanks.

bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Feb 18, 2020
Authenticated decryption works for file streams up to 32768 bytes but
not beyond. Other streams and direct decryption are not affected.

Refs: nodejs#31733
addaleax pushed a commit that referenced this issue Apr 2, 2020
Authenticated decryption works for file streams up to 32768 bytes but
not beyond. Other streams and direct decryption are not affected.

Refs: #31733

PR-URL: #31734
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
BethGriggs pushed a commit that referenced this issue Apr 7, 2020
Authenticated decryption works for file streams up to 32768 bytes but
not beyond. Other streams and direct decryption are not affected.

Refs: #31733

PR-URL: #31734
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this issue Apr 12, 2020
Authenticated decryption works for file streams up to 32768 bytes but
not beyond. Other streams and direct decryption are not affected.

Refs: #31733

PR-URL: #31734
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this issue Apr 22, 2020
Authenticated decryption works for file streams up to 32768 bytes but
not beyond. Other streams and direct decryption are not affected.

Refs: #31733

PR-URL: #31734
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@ronag
Copy link
Member

ronag commented Jun 20, 2020

This might be fixed through #33981

ronag added a commit to nxtedition/node that referenced this issue Jun 20, 2020
The performance benefit of using a custom pool are negligable.
Furthermore, it causes problems with Workers and transferrable.
Rather than further adding complexity for compat with Workers,
just remove the pooling logic.

Refs: nodejs#33880 (comment)
Fixes: nodejs#31733
@ronag ronag closed this as completed in db0e991 Jun 23, 2020
targos pushed a commit to targos/node that referenced this issue Apr 25, 2021
The performance benefit of using a custom pool are negligable.
Furthermore, it causes problems with Workers and transferrable.
Rather than further adding complexity for compat with Workers,
just remove the pooling logic.

Refs: nodejs#33880 (comment)
Fixes: nodejs#31733

PR-URL: nodejs#33981
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
targos pushed a commit that referenced this issue Apr 26, 2021
The performance benefit of using a custom pool are negligable.
Furthermore, it causes problems with Workers and transferrable.
Rather than further adding complexity for compat with Workers,
just remove the pooling logic.

Refs: #33880 (comment)
Fixes: #31733

PR-URL: #33981
Backport-PR-URL: #38397
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem. fs Issues and PRs related to the fs subsystem / file system. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants