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

Rewrite node:crypto #13681

Open
Tracked by #159
Jarred-Sumner opened this issue Sep 2, 2024 · 2 comments
Open
Tracked by #159

Rewrite node:crypto #13681

Jarred-Sumner opened this issue Sep 2, 2024 · 2 comments
Labels
tracking An umbrella issue for tracking big features

Comments

@Jarred-Sumner
Copy link
Collaborator

Jarred-Sumner commented Sep 2, 2024

There are several open issues about node:crypto

Our current implementation of node:crypto is an incrementally modified fork of the browserify crypto polyfill with a lot of BoringSSL bindings added in key places.

The browserify crypto polyfill was a great starting point, but we've outgrown it.

When you skim through the code, you'll note that very little of it makes sense in the context of Bun

bun/src/js/node/crypto.ts

Lines 2238 to 2258 in 1bec6c3

var out = self2._cipher.encryptBlockRaw(self2._prev);
return incr32(self2._prev), out;
}
var blockSize = 16;
exports.encrypt = function (self2, chunk) {
var chunkNum = Math.ceil(chunk.length / blockSize),
start = self2._cache.length;
self2._cache = Buffer2.concat([self2._cache, Buffer2.allocUnsafe(chunkNum * blockSize)]);
for (var i = 0; i < chunkNum; i++) {
var out = getBlock(self2),
offset = start + i * blockSize;
self2._cache.writeUInt32BE(out[0], offset + 0),
self2._cache.writeUInt32BE(out[1], offset + 4),
self2._cache.writeUInt32BE(out[2], offset + 8),
self2._cache.writeUInt32BE(out[3], offset + 12);
}
var pad = self2._cache.slice(0, chunk.length);
return (self2._cache = self2._cache.slice(chunk.length)), xor(chunk, pad);
};
},
});

It has a JavaScript implementation of MD5, SHA1, crypto.randomBytes, DES, AES, diffieHelman, etc. We have BoringSSL. We even expose BoringSSL's MD5, SHA1, etc. Let's use it.

Let's also get rid of the commonJS wrappers in it. The only JavaScript code in node:crypto should be related to streams, or wrapping a native implementation in a stream. Everything else should be in native code.

The other issue here is in the particular stream implementations themselves. There are likely many small subtly incompatible bugs in the various classes exposed by node:crypto. We need to be running node's tests against our crypto implementation. Ideally, we'd be running other test suites too for this.

@Jarred-Sumner Jarred-Sumner added the tracking An umbrella issue for tracking big features label Sep 2, 2024
@Jarred-Sumner Jarred-Sumner mentioned this issue Sep 2, 2024
52 tasks
@wpaulino
Copy link
Contributor

wpaulino commented Sep 2, 2024

I plan to start with the switch to BoringSSL for publicEncrypt/privateDecrypt.

@jlucaso1
Copy link
Contributor

jlucaso1 commented Sep 26, 2024

I need help to complete the feature to implement X25519. I have copied files from safari webkit implementation, but with no success
#13798

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracking An umbrella issue for tracking big features
Projects
None yet
Development

No branches or pull requests

3 participants