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

fix private key export without cipher crash #27041

Closed
wants to merge 3 commits into from
Closed

fix private key export without cipher crash #27041

wants to merge 3 commits into from

Conversation

panva
Copy link
Member

@panva panva commented Apr 1, 2019

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

Fixes the following by type checking the cipher before sending of to node_crypto.cc

const { generateKeyPairSync } = require('crypto')
const { privateKey } = generateKeyPairSync('rsa', { modulusLength: 2048 })
privateKey.export({ format: 'pem', type: 'pkcs8', passphrase: 'super-secret' })
node[13162]: ../src/node_crypto.cc:3096:NonCopyableMaybe<node::crypto::PrivateKeyEncodingConfig> node::crypto::GetPrivateKeyEncodingFromJs(const FunctionCallbackInfo<v8::Value> &, unsigned int *, node::crypto::KeyEncodingContext): Assertion `!(context != kKeyContextInput) || (result.cipher_ != nullptr)' failed.
 1: 0x10006778e node::Abort() [/.nvm/versions/node/v11.13.0/bin/node]
 2: 0x1000676c0 node::PrintErrorString(char const*, ...) [/.nvm/versions/node/v11.13.0/bin/node]
 3: 0x10011f2d8 node::crypto::KeyObject::ExportPrivateKey(node::crypto::PrivateKeyEncodingConfig const&) const [/.nvm/versions/node/v11.13.0/bin/node]
 4: 0x10011e16f node::crypto::KeyObject::Export(v8::FunctionCallbackInfo<v8::Value> const&) [/.nvm/versions/node/v11.13.0/bin/node]
 5: 0x10023f927 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo*) [/.nvm/versions/node/v11.13.0/bin/node]
 6: 0x10023eef6 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) [/.nvm/versions/node/v11.13.0/bin/node]
 7: 0x10023e5f0 v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/.nvm/versions/node/v11.13.0/bin/node]
 8: 0xd92061cfc7d 
 9: 0xd920618e458 
10: 0xd920618e458 
11: 0xd920618ba89 
[1]    13162 abort      node

@nodejs-github-bot nodejs-github-bot added the crypto Issues and PRs related to the crypto subsystem. label Apr 1, 2019
@panva
Copy link
Member Author

panva commented Apr 1, 2019

cc @tniessen

@tniessen tniessen added the confirmed-bug Issues with confirmed bugs. label Apr 1, 2019
lib/internal/crypto/keys.js Outdated Show resolved Hide resolved
@panva
Copy link
Member Author

panva commented Apr 2, 2019

@bnoordhuis @BridgeAR done

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 3, 2019
@nodejs-github-bot
Copy link
Collaborator

@BridgeAR
Copy link
Member

BridgeAR commented Apr 4, 2019

Landed in b2bb6c2 🎉

@BridgeAR BridgeAR closed this Apr 4, 2019
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Apr 4, 2019
PR-URL: nodejs#27041
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BethGriggs pushed a commit that referenced this pull request Apr 9, 2019
PR-URL: #27041
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Signed-off-by: Beth Griggs <Bethany.Griggs@uk.ibm.com>
BethGriggs pushed a commit that referenced this pull request Apr 9, 2019
PR-URL: #27041
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Signed-off-by: Beth Griggs <Bethany.Griggs@uk.ibm.com>
@BethGriggs BethGriggs mentioned this pull request Apr 9, 2019
BethGriggs pushed a commit that referenced this pull request Apr 10, 2019
PR-URL: #27041
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Signed-off-by: Beth Griggs <Bethany.Griggs@uk.ibm.com>
BethGriggs pushed a commit that referenced this pull request Apr 10, 2019
PR-URL: #27041
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Signed-off-by: Beth Griggs <Bethany.Griggs@uk.ibm.com>
BethGriggs pushed a commit that referenced this pull request Apr 10, 2019
PR-URL: #27041
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Signed-off-by: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants