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

createPublicKey and createPrivateKey lacks robustness with JWK #44471

Closed
rinne opened this issue Sep 1, 2022 · 2 comments
Closed

createPublicKey and createPrivateKey lacks robustness with JWK #44471

rinne opened this issue Sep 1, 2022 · 2 comments
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem.

Comments

@rinne
Copy link

rinne commented Sep 1, 2022

Version

v18.7.0

Platform

Darwin xxx 21.6.0 Darwin Kernel Version 21.6.0: Wed Aug 10 14:25:27 PDT 2022; root:xnu-8020.141.5~2/RELEASE_X86_64 x86_64

Subsystem

crypto

What steps will reproduce the bug?

// called jwk format but non-object (string) as a parameter

require('crypto').createPrivateKey({ key: "foo", format: 'jwk' });

// core dump follows

require('crypto').createPublicKey({ key: "", format: 'jwk' });

// core dump follows

How often does it reproduce? Is there a required condition?

always

What is the expected behavior?

Exception with an appropriate error object should be thrown.
It should be possible to catch the exception.

What do you see instead?

node[88386]: ../src/crypto/crypto_keys.cc:68:void node::crypto::(anonymous namespace)::GetKeyFormatAndTypeFromJs(node::crypto::AsymmetricKeyEncodingConfig *, const FunctionCallbackInfo<v8::Value> &, unsigned int *, node::crypto::KeyEncodingContext): Assertion `(context == kKeyContextInput && config->format_ == kKeyFormatPEM) || (context == kKeyContextGenerate && config->format_ == kKeyFormatJWK)' failed.
 1: 0x103037252 node::Abort() [/opt/local/bin/node]
 2: 0x103037085 node::AppendExceptionLine(node::Environment*, v8::Local<v8::Value>, v8::Local<v8::Message>, node::ErrorHandlingMode) [/opt/local/bin/node]
 3: 0x103138e07 node::crypto::(anonymous namespace)::GetKeyFormatAndTypeFromJs(node::crypto::AsymmetricKeyEncodingConfig*, v8::FunctionCallbackInfo<v8::Value> const&, unsigned int*, node::crypto::KeyEncodingContext) [/opt/local/bin/node]
 4: 0x103138693 node::crypto::ManagedEVPPKey::GetPrivateKeyEncodingFromJs(v8::FunctionCallbackInfo<v8::Value> const&, unsigned int*, node::crypto::KeyEncodingContext) [/opt/local/bin/node]
 5: 0x103139575 node::crypto::ManagedEVPPKey::GetPublicOrPrivateKeyFromJs(v8::FunctionCallbackInfo<v8::Value> const&, unsigned int*) [/opt/local/bin/node]
 6: 0x10313a3ba node::crypto::KeyObjectHandle::Init(v8::FunctionCallbackInfo<v8::Value> const&) [/opt/local/bin/node]
 7: 0x1031b491b v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [/opt/local/bin/node]
 8: 0x1031b44ab v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/opt/local/bin/node]
 9: 0x1031b3c28 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [/opt/local/bin/node]
10: 0x1038710f9 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit [/opt/local/bin/node]

Additional information

This has been the case with JWK import for all versions so far.

@VoltrexKeyva VoltrexKeyva added the crypto Issues and PRs related to the crypto subsystem. label Sep 1, 2022
@panva panva added the confirmed-bug Issues with confirmed bugs. label Sep 1, 2022
@panva
Copy link
Member

panva commented Sep 1, 2022

@rinne thank you for the report. I'm looking into a fix.

panva added a commit to panva/node that referenced this issue Sep 1, 2022
@panva
Copy link
Member

panva commented Sep 1, 2022

#44475

panva added a commit to panva/node that referenced this issue Sep 1, 2022
RafaelGSS pushed a commit that referenced this issue Sep 5, 2022
Fixes #44471

PR-URL: #44475
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
RafaelGSS pushed a commit that referenced this issue Sep 6, 2022
Fixes #44471

PR-URL: #44475
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
RafaelGSS pushed a commit that referenced this issue Sep 7, 2022
Fixes #44471

PR-URL: #44475
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
RafaelGSS pushed a commit that referenced this issue Sep 7, 2022
Fixes #44471

PR-URL: #44475
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Fyko pushed a commit to Fyko/node that referenced this issue Sep 15, 2022
Fixes nodejs#44471

PR-URL: nodejs#44475
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
juanarbol pushed a commit that referenced this issue Oct 3, 2022
Fixes #44471

PR-URL: #44475
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
juanarbol pushed a commit that referenced this issue Oct 4, 2022
Fixes #44471

PR-URL: #44475
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
juanarbol pushed a commit that referenced this issue Oct 4, 2022
Fixes #44471

PR-URL: #44475
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
juanarbol pushed a commit that referenced this issue Oct 7, 2022
Fixes #44471

PR-URL: #44475
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
juanarbol pushed a commit that referenced this issue Oct 10, 2022
Fixes #44471

PR-URL: #44475
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
juanarbol pushed a commit that referenced this issue Oct 11, 2022
Fixes #44471

PR-URL: #44475
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants