Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

feat: replace keypair and ursa-optional with node crypto #219

Merged
merged 2 commits into from
Dec 1, 2021

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented Dec 1, 2021

We use ursa-optional to geenrate RSA keypairs in node.js, falling back to keypair if compilation of ursa-optional failed at install time.

Node.js has supported generating RSA keypairs since 10.x so use that instead and remove the ursa-optional and kepair dependencies.

As a bonus node 15+ also supports jwk format keys everywhere so the pem-jwk dep can go too.

BREAKING CHANGE: requires node 15+

We use `ursa-optional` to geenrate RSA keypairs in node.js, falling
back to `keypair` if compilation of `ursa-optional` failed at
install time.

Node.js has supported generating RSA keypairs since 10.x so use that
instead and remove the `ursa-optional` and `kepair` dependencies.

As a bonus it also supports `jwk` format keys everywhere so the
`pem-jwk` dep can go too.

exports.utils = require('./rsa-utils')

exports.generateKey = async function (bits) { // eslint-disable-line require-await
const key = keypair({ bits })
const key = await keypair('rsa', {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use the sync version here, but used the async version seeing as how the function is async anyway.

@achingbrain
Copy link
Member Author

Can put pem-jwk back in if we care about node 14 compatibility, but we shouldn't as 16 is now LTS.

Copy link
Member

@hugomrdias hugomrdias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🔥🔥

@hugomrdias
Copy link
Member

Now we just need to find a way to work around the RSA encryption thing to take out forge 😭😭

@achingbrain
Copy link
Member Author

@hugomrdias some thoughts on that here: #220

@achingbrain achingbrain merged commit 759535c into master Dec 1, 2021
@achingbrain achingbrain deleted the feat/replace-ursa-with-node-crypto branch December 1, 2021 11:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants