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

Implement rsa toPem, toDer, and fromJwk utilities #2389

Merged
merged 3 commits into from
Jul 22, 2024

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jul 12, 2024

Separating out more of the node.js crypto key handling support. There's a lot to do with all this so I'm piecing it out into separate PRs that individually are easier to review. This adds methods but does not add the stuff that uses it yet. Tests for these individual pieces will also be added separately. A fair amount of this is adapted from https://github.com/nodejs/node/blob/main/src/crypto/crypto_keys.cc

@jasnell jasnell requested review from a team as code owners July 12, 2024 16:08
src/workerd/api/crypto/rsa.h Outdated Show resolved Hide resolved
if (result.error != simdutf::SUCCESS) return kj::none;
KJ_ASSERT(result.count <= size);
return buf.first(result.count).attach(kj::mv(buf));
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Reviewer note: This is a more efficient alternative to the existing base64 url decoding method we have based on kj::decodeBase64. This one follows the standard forgiving base64 decode algorithm defined by whatwg specs. Soon I expect this to be moved out of this rsa.c++ file and into a shared location.

@jasnell jasnell requested a review from dom96 July 15, 2024 15:12
@jasnell jasnell merged commit c46c662 into main Jul 22, 2024
9 checks passed
@jasnell jasnell deleted the jsnell/crypto-rsa-updates branch July 22, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants