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

crypto: add KeyObject.from and keyObject.export JWK format support #36203

Closed
wants to merge 3 commits into from

Conversation

panva
Copy link
Member

@panva panva commented Nov 20, 2020

I'm opening this Draft PR to see/probe if there's interest in taking it further (cleaning up and getting it ready to land, which i'm all willing to do). It builds on top of #36188

This adds the 'jwk' format option value for keyObject.export. It also adds a KeyObject.fromJwk static method (its functionality should probably be added to KeyObject.from but I don't think that one was meant to ship in the first place so maybe this could completely replace it, see the question in code.)

WDYT? Is it worth spending further time on in hopes of being accepted?

refs #24471
refs #26854

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

This API exposes key details. It is conceptually different from the
previously discussed keyObject.fields property since it does not give
access to information that could compromise the security of the key, and
the obtained information cannot be used to uniquely identify a key.

The intended purpose is to determine "security properties" of keys, e.g.
to generate a new key pair with the same parameters, or to decide
whether a key is secure enough.

closes nodejs#30045
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Nov 20, 2020
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/quic

env->context(),
env->jwk_kty_string(),
env->jwk_okp_string()).IsNothing()) {
return Nothing<bool>();
Copy link
Member

Choose a reason for hiding this comment

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

This leaks rawX – is there any reason not to use a standard container here instead, i.e. std::vector<uint8_t> raw_x(len);? ditto below for rawD (/raw_d)

@panva
Copy link
Member Author

panva commented Jan 18, 2021

I'm closing this for now. Once #36879 lands i'll open a new proposal.

@panva panva closed this Jan 18, 2021
@panva panva deleted the keyobject-jwk-support branch October 13, 2022 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants