-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
chacha20-poly1305 decryption works without setting authTag on decypher in v16 #45874
Comments
We experience the same. We use similar file encryption/decryption, and what we observed is that with node 16 it just works as it is, but after upgrading to node 18, it still does work, but we do get an error at decryption, which does not affect the decrypted result. See below. The error I get:
CodeClass const iv = '123456789012'; // for testing
export class FileEncrypter {
constructor(
private readonly nonceLength: number,
private readonly algorithm: CipherCCMTypes,
private readonly encryptionKey: string,
) {}
encrypt(file: Buffer): Promise<Buffer> {
return new Promise<Buffer>((res, rej) => {
const cipher = createCipheriv(this.algorithm, this.encryptionKey, iv, { authTagLength: 16 });
const input = Readable.from(file);
const output = new PassThrough();
const outputData: Buffer[] = [];
output.write(iv);
output.on('readable', () => {
const data = output.read();
if (data) {
outputData.push(data);
}
});
output.on('end', () => {
const encrypted = Buffer.concat(outputData);
res(encrypted);
});
pipeline(input, cipher, output, (err) => {
if (err) {
rej(err);
}
});
});
}
decrypt(file: StreamableFile): Promise<Buffer> {
return new Promise<Buffer>((res, rej) => {
const input = file.getStream();
const output = new PassThrough();
input.once('readable', () => {
const iv = input.read(12);
const cipher = createDecipheriv(this.algorithm, this.encryptionKey, iv, { authTagLength: 16 });
output.once('readable', () => {
const encrypted = output.read();
res(encrypted);
});
pipeline(input, cipher, output, (err) => {
if (err) {
rej(err);
}
});
});
});
}
} Test const testImage = readFileSync(`${E2E_TESTS_ASSETS_DIR}/simonstalenhag.jpg`);
const encrypter = new FileEncrypter(12, 'chacha20-poly1305', ENCRYPTION_KEY);
const encryptedFile = await encrypter.encrypt(testImage);
const decryptedFile = await encrypter.decrypt(new StreamableFile(encryptedFile));
console.log('decrypted');
console.log(decryptedFile);
console.log('original');
console.log(testImage);
expect(Buffer.compare(decryptedFile, testImage)).toStrictEqual(0); AND here |
cc @tniessen @nodejs/crypto |
The "unable to authenticate data" error is expected; its absence in v16 isn't. I speculate openssl 1.x didn't enforce it but openssl 3.x does. (There is some special handling for chacha20-poly1305 inside node itself but v16.19.0 and v18.12.1 are identical in that respect so that can't explain it.) The best suggestion I have is to special-case it in |
Because OpenSSL v1.x doesn't do that by itself (OpenSSL v3.x does.) Fixes: nodejs#45874
/me raises hand |
Because OpenSSL v1.x doesn't do that by itself (OpenSSL v3.x does.) Fixes: nodejs#45874
Will it be available in 18LTS and in 16LTS? |
Whatever happened to "thank you, you're awesome, I want to have your babies"? ...Yes, eventually. |
Yes, you're right, I'm sorry. Nothing happened to kindness, I just simply forgot it. :) thanks for your good work! |
Because OpenSSL v1.x doesn't do that by itself (OpenSSL v3.x does.) Fixes: #45874 PR-URL: #46185 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Because OpenSSL v1.x doesn't do that by itself (OpenSSL v3.x does.) Fixes: nodejs#45874 PR-URL: nodejs#46185 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Because OpenSSL v1.x doesn't do that by itself (OpenSSL v3.x does.) Fixes: #45874 PR-URL: #46185 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Because OpenSSL v1.x doesn't do that by itself (OpenSSL v3.x does.) Fixes: #45874 PR-URL: #46185 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Because OpenSSL v1.x doesn't do that by itself (OpenSSL v3.x does.) Fixes: #45874 PR-URL: #46185 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Because OpenSSL v1.x doesn't do that by itself (OpenSSL v3.x does.) Fixes: #45874 PR-URL: #46185 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Version
v16.19.0 / v18.12.1
Platform
Linux andi-vm 5.15.0-56-generic #62-Ubuntu SMP Tue Nov 22 19:54:14 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Subsystem
No response
What steps will reproduce the bug?
How often does it reproduce? Is there a required condition?
If running the code using the node v16.19.0, the code runs without throwing an error. If I change to node v18.12.1, an error is thrown, as per the docs.
What is the expected behavior?
According to the docs:
I would expect the code snipped above to fail, since the authTag is not set on the decypher.
What do you see instead?
The code snippet above runs without throwing any error. It successfully prints "Some file".
Additional information
As soon as I switch from node v16 to v18, the code snipped throws an error, as expected. Uncommenting the
setAuthTag
line makes the code work. If this is expected behavior in v16 (vs v18), I'd be happy to be guided to a doc/changelog that explains this change.The text was updated successfully, but these errors were encountered: