feat(ext/node): add crypto.createCipheriv #18091

kt3k · 2023-03-09T10:43:57Z

This PR implements very basic feature of crypto.createCipheriv with only 'aes-128-cbc' algorithm support.

The behaviors/values of cipheriv.update and cipheriv.final are aligned to Node.js.

(The below works in the same way in this branch and Node for example)

import crypto from "node:crypto";
const cipher = crypto.createCipheriv("aes-128-cbc", new Uint8Array(16), new Uint8Array(16));
console.log(cipher.update(new Uint8Array(55)).toString("hex"));
console.log(cipher.final().toString("hex"));

The following features will be added in the follow up PRs:

Decipheriv
stream.Transform interface support
More algorithms (aes-128-gcm, etc)

closes #17848

ext/node/crypto/cipher.rs

bartlomieju · 2023-03-13T13:02:06Z

ext/node/Cargo.toml

+aes = "0.8.2"
+cbc = "0.1.2"


Consider moving these two to root Cargo.toml and using aes.workspace = true here (make sure to update other references in various Cargo.toml files)

bartlomieju · 2023-03-13T13:02:59Z

ext/node/crypto/mod.rs

+ state.resource_table.add(
+ match cipher::CipherContext::new(algorithm, key, iv) {
+ Ok(context) => context,
+ Err(_) => return 0,
+ },
+ )


Appears that this resource will never be dropped which will cause the same issue as here: #18140

Not sure this is the correct way to implement it, but Cipheriv.prototype.final method calls Deno.close(this.context) which closes the cipher resource, and the test cases pass without disabling the sanitizer

Ah! Makes sense, thanks for clarifying.

magurotuna · 2023-03-13T13:01:37Z

ext/node/crypto/cipher.rs

+use std::cell::RefCell;
+use std::rc::Rc;
+
+pub enum Cipher {


Maybe no need to make it pub?

magurotuna · 2023-03-13T13:01:41Z

ext/node/crypto/cipher.rs

+ // TODO(kt3k): add more algorithms Aes192Cbc, Aes256Cbc, Aes128ECB, Aes128GCM, etc.
+}
+
+pub enum Decipher {


magurotuna · 2023-03-13T13:02:04Z

ext/node/crypto/cipher.rs

+}
+
+pub struct CipherContext {
+ pub cipher: Rc<RefCell<Cipher>>,


ditto (I'm pointing to L24 only)

magurotuna · 2023-03-13T13:02:09Z

ext/node/crypto/cipher.rs

+}
+
+pub struct DecipherContext {
+ pub decipher: Rc<RefCell<Decipher>>,


ditto (I'm pointing to L28 only)

magurotuna · 2023-03-13T13:02:23Z

ext/node/crypto/cipher.rs

+}
+
+impl Cipher {
+ pub fn new(


magurotuna · 2023-03-13T13:02:40Z

ext/node/crypto/cipher.rs

+ })
+ }
+
+ pub fn encrypt(&mut self, input: &[u8], output: &mut [u8]) {


bartlomieju

LGTM

littledivy · 2023-03-13T17:46:54Z

ext/node/polyfills/internal/crypto/cipher.ts

+ for (let i = 0; i < input.length; i += 16) {
+ ops.op_node_cipheriv_encrypt(
+ this.context,
+ input.subarray(i, i + 16),
+ output.subarray(i, i + 16),
+ );
+ }
+ return outputEncoding === "buffer"
+ ? output
+ : output.toString(outputEncoding);


Maybe this could done in Rust, subarrays are much cheaper there.

if (outputEncoding === "buffer") ops.op_node_encrypt_block(...);

Moved the iteration over chunks to the rust side

ext/node/polyfills/internal/crypto/cipher.ts

littledivy · 2023-03-13T17:49:58Z

ext/node/crypto/mod.rs

+ let context = match state.resource_table.get::<cipher::CipherContext>(rid) {
+ Ok(context) => context,
+ Err(_) => return false,
+ };


This should just consume the resource i.e resource_table.take. We don't need to call core.close from JS

Added op_node_cipheriv_final which is called at the end of encryption and consumes the resource. (op_node_cipheriv_encrypt was used both for the last and middle encryptions, but that was probably confusing)

littledivy

LGTM

littledivy · 2023-03-14T05:34:50Z

ext/crypto/Cargo.toml

@@ -14,7 +14,7 @@ description = "Web Cryptography API implementation for Deno"
 path = "lib.rs"

 [dependencies]
-aes = "0.8.1"


cbc crate is now in workspace scope too. cbc.workspace = true

kt3k · 2023-03-14T07:00:54Z

Tagged the commit as fix(ext/node): as any missing node.js APIs can be considered as a bug of node compatibility.

kt3k force-pushed the node-crypto-create-cipheriv branch from 3a92185 to 28fcb0a Compare March 10, 2023 15:14

wip(ext/node): add crypto.createCipheriv

c889a97

kt3k force-pushed the node-crypto-create-cipheriv branch from 28fcb0a to a964d5d Compare March 13, 2023 12:08

kt3k added 2 commits March 13, 2023 21:16

feat: Cipheriv (aes-128-cbc)

7197603

test(ext/node): test createCipheriv

a8fabf6

kt3k force-pushed the node-crypto-create-cipheriv branch from a964d5d to a8fabf6 Compare March 13, 2023 12:26

kt3k added 2 commits March 13, 2023 21:27

fmt

4bb7f1c

modify

261a089

magurotuna reviewed Mar 13, 2023

View reviewed changes

ext/node/crypto/cipher.rs Outdated Show resolved Hide resolved

ext/node/crypto/cipher.rs Outdated Show resolved Hide resolved

ext/node/crypto/cipher.rs Outdated Show resolved Hide resolved

kt3k added 2 commits March 13, 2023 21:46

address review comments

262f186

address review comment, fix lint

3bd961a

kt3k marked this pull request as ready for review March 13, 2023 12:52

kt3k requested review from bartlomieju and littledivy March 13, 2023 12:52

bartlomieju reviewed Mar 13, 2023

View reviewed changes

magurotuna reviewed Mar 13, 2023

View reviewed changes

bartlomieju mentioned this pull request Mar 13, 2023

Node compatibility: "Uncaught Error: Not implemented: crypto.Cipheriv" #17848

Closed

kt3k added 2 commits March 13, 2023 23:51

move deps to workspace

98bed29

reduce unnecessary public items

ddaaaae

bartlomieju approved these changes Mar 13, 2023

View reviewed changes

littledivy reviewed Mar 13, 2023

View reviewed changes

kt3k added 2 commits March 14, 2023 10:41

address review comments from divy

c662ef4

add op_node_cipheriv_final

76ed25d

kt3k force-pushed the node-crypto-create-cipheriv branch from 298abd1 to 76ed25d Compare March 14, 2023 03:04

kt3k added 4 commits March 14, 2023 12:20

modify Cipher::r#final

b256306

fmt

3d15adb

fix lint error

83c8df5

move chunk iteration to rust side

69fd6f8

littledivy approved these changes Mar 14, 2023

View reviewed changes

use workspace cbc crate

f25a3cd

kt3k merged commit e80cc17 into denoland:main Mar 14, 2023

kt3k deleted the node-crypto-create-cipheriv branch March 14, 2023 07:00

kt3k added a commit that referenced this pull request Mar 16, 2023

fix(ext/node): add crypto.createCipheriv (#18091)

9d01613

kt3k mentioned this pull request Mar 17, 2023

fix(ext/node): add createDecipheriv #18245

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ext/node): add crypto.createCipheriv #18091

feat(ext/node): add crypto.createCipheriv #18091

kt3k commented Mar 9, 2023 •

edited

Loading

bartlomieju Mar 13, 2023

bartlomieju Mar 13, 2023

kt3k Mar 13, 2023

bartlomieju Mar 13, 2023

magurotuna Mar 13, 2023

magurotuna Mar 13, 2023

magurotuna Mar 13, 2023 •

edited

Loading

magurotuna Mar 13, 2023 •

edited

Loading

magurotuna Mar 13, 2023

magurotuna Mar 13, 2023

bartlomieju left a comment

littledivy Mar 13, 2023

kt3k Mar 14, 2023

littledivy Mar 13, 2023

kt3k Mar 14, 2023

littledivy left a comment

littledivy Mar 14, 2023

kt3k commented Mar 14, 2023

feat(ext/node): add crypto.createCipheriv #18091

feat(ext/node): add crypto.createCipheriv #18091

Conversation

kt3k commented Mar 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magurotuna Mar 13, 2023 • edited Loading

Choose a reason for hiding this comment

magurotuna Mar 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bartlomieju left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littledivy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kt3k commented Mar 14, 2023

kt3k commented Mar 9, 2023 •

edited

Loading

magurotuna Mar 13, 2023 •

edited

Loading

magurotuna Mar 13, 2023 •

edited

Loading