Skip to content

Commit

Permalink
crypto: use openssl's own memory BIOs in crypto_context.cc
Browse files Browse the repository at this point in the history
NodeBIO's memory buffer structure does not support BIO_C_FILE_SEEK and B
IO_C_FILE_TELL. This prevents OpenSSL PEM_read_bio_PrivateKey from readi
ng some private keys. So I switched to OpenSSL'w own protected memory bu
ffers.

Fixes: #47008
PR-URL: #47160
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
GauriSpears authored and targos committed May 30, 2023
1 parent d3e3a91 commit f564b03
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 11 deletions.
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,7 @@
'sources': [
'test/cctest/test_crypto_clienthello.cc',
'test/cctest/test_node_crypto.cc',
'test/cctest/test_node_crypto_env.cc',
'test/cctest/test_quic_cid.cc',
'test/cctest/test_quic_tokens.cc',
]
Expand Down
20 changes: 9 additions & 11 deletions src/crypto/crypto_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,16 @@ inline X509_STORE* GetOrCreateRootCertStore() {
// Takes a string or buffer and loads it into a BIO.
// Caller responsible for BIO_free_all-ing the returned object.
BIOPointer LoadBIO(Environment* env, Local<Value> v) {
HandleScope scope(env->isolate());

if (v->IsString()) {
Utf8Value s(env->isolate(), v);
return NodeBIO::NewFixed(*s, s.length());
}

if (v->IsArrayBufferView()) {
ArrayBufferViewContents<char> buf(v.As<ArrayBufferView>());
return NodeBIO::NewFixed(buf.data(), buf.length());
if (v->IsString() || v->IsArrayBufferView()) {
BIOPointer bio(BIO_new(BIO_s_secmem()));
if (!bio) return nullptr;
ByteSource bsrc = ByteSource::FromStringOrBuffer(env, v);
if (bsrc.size() > INT_MAX) return nullptr;
int written = BIO_write(bio.get(), bsrc.data<char>(), bsrc.size());
if (written < 0) return nullptr;
if (static_cast<size_t>(written) != bsrc.size()) return nullptr;
return bio;
}

return nullptr;
}

Expand Down
31 changes: 31 additions & 0 deletions test/cctest/test_node_crypto_env.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#include "crypto/crypto_bio.h"
#include "gtest/gtest.h"
#include "node_options.h"
#include "node_test_fixture.h"
#include "openssl/err.h"

using v8::Local;
using v8::String;

/*
* This test verifies that an object created by LoadBIO supports BIO_tell
* and BIO_seek, otherwise PEM_read_bio_PrivateKey fails on some keys
* (if OpenSSL needs to rewind pointer between pem_read_bio_key()
* and pem_read_bio_key_legacy() inside PEM_read_bio_PrivateKey).
*/
class NodeCryptoEnv : public EnvironmentTestFixture {};

TEST_F(NodeCryptoEnv, LoadBIO) {
v8::HandleScope handle_scope(isolate_);
Argv argv;
Env env{handle_scope, argv};
// just put a random string into BIO
Local<String> key = String::NewFromUtf8(isolate_, "abcdef").ToLocalChecked();
node::crypto::BIOPointer bio(node::crypto::LoadBIO(*env, key));
#if OPENSSL_VERSION_NUMBER >= 0x30000000L
BIO_seek(bio.get(), 2);
ASSERT_EQ(BIO_tell(bio.get()), 2);
#endif
ASSERT_EQ(ERR_peek_error(), 0UL) << "There should not have left "
"any errors on the OpenSSL error stack\n";
}

0 comments on commit f564b03

Please sign in to comment.