Commit 7617772
crypto: use openssl's own memory BIOs in crypto_context.cc
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>1 parent ed0b62c commit 7617772
3 files changed
Lines changed: 41 additions & 11 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1275,6 +1275,7 @@ | |||
| 1275 | 1275 | 'sources': [ | |
| 1276 | 1276 | 'test/cctest/test_crypto_clienthello.cc', | |
| 1277 | 1277 | 'test/cctest/test_node_crypto.cc', | |
| 1278 | + 'test/cctest/test_node_crypto_env.cc', | ||
| 1278 | 1279 | ] | |
| 1279 | 1280 | }], | |
| 1280 | 1281 | ['v8_enable_inspector==1', { | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -62,18 +62,16 @@ inline X509_STORE* GetOrCreateRootCertStore() { | |||
| 62 | 62 | // Takes a string or buffer and loads it into a BIO. | |
| 63 | 63 | // Caller responsible for BIO_free_all-ing the returned object. | |
| 64 | 64 | BIOPointer LoadBIO(Environment* env, Local<Value> v) { | |
| 65 | - HandleScope scope(env->isolate()); | ||
| 66 | - | ||
| 67 | - if (v->IsString()) { | ||
| 68 | - Utf8Value s(env->isolate(), v); | ||
| 69 | - return NodeBIO::NewFixed(*s, s.length()); | ||
| 70 | - } | ||
| 71 | - | ||
| 72 | - if (v->IsArrayBufferView()) { | ||
| 73 | - ArrayBufferViewContents<char> buf(v.As<ArrayBufferView>()); | ||
| 74 | - return NodeBIO::NewFixed(buf.data(), buf.length()); | ||
| 65 | + if (v->IsString() || v->IsArrayBufferView()) { | ||
| 66 | + BIOPointer bio(BIO_new(BIO_s_secmem())); | ||
| 67 | + if (!bio) return nullptr; | ||
| 68 | + ByteSource bsrc = ByteSource::FromStringOrBuffer(env, v); | ||
| 69 | + if (bsrc.size() > INT_MAX) return nullptr; | ||
| 70 | + int written = BIO_write(bio.get(), bsrc.data<char>(), bsrc.size()); | ||
| 71 | + if (written < 0) return nullptr; | ||
| 72 | + if (static_cast<size_t>(written) != bsrc.size()) return nullptr; | ||
| 73 | + return bio; | ||
| 75 | 74 | } | |
| 76 | - | ||
| 77 | 75 | return nullptr; | |
| 78 | 76 | } | |
| 79 | 77 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,31 @@ | |||
| 1 | + #include "crypto/crypto_bio.h" | ||
| 2 | + #include "gtest/gtest.h" | ||
| 3 | + #include "node_options.h" | ||
| 4 | + #include "node_test_fixture.h" | ||
| 5 | + #include "openssl/err.h" | ||
| 6 | + | ||
| 7 | + using v8::Local; | ||
| 8 | + using v8::String; | ||
| 9 | + | ||
| 10 | + /* | ||
| 11 | + * This test verifies that an object created by LoadBIO supports BIO_tell | ||
| 12 | + * and BIO_seek, otherwise PEM_read_bio_PrivateKey fails on some keys | ||
| 13 | + * (if OpenSSL needs to rewind pointer between pem_read_bio_key() | ||
| 14 | + * and pem_read_bio_key_legacy() inside PEM_read_bio_PrivateKey). | ||
| 15 | + */ | ||
| 16 | + class NodeCryptoEnv : public EnvironmentTestFixture {}; | ||
| 17 | + | ||
| 18 | + TEST_F(NodeCryptoEnv, LoadBIO) { | ||
| 19 | + v8::HandleScope handle_scope(isolate_); | ||
| 20 | + Argv argv; | ||
| 21 | + Env env{handle_scope, argv}; | ||
| 22 | + // just put a random string into BIO | ||
| 23 | + Local<String> key = String::NewFromUtf8(isolate_, "abcdef").ToLocalChecked(); | ||
| 24 | + node::crypto::BIOPointer bio(node::crypto::LoadBIO(*env, key)); | ||
| 25 | + #if OPENSSL_VERSION_NUMBER >= 0x30000000L | ||
| 26 | + BIO_seek(bio.get(), 2); | ||
| 27 | + ASSERT_EQ(BIO_tell(bio.get()), 2); | ||
| 28 | + #endif | ||
| 29 | + ASSERT_EQ(ERR_peek_error(), 0UL) << "There should not have left " | ||
| 30 | + "any errors on the OpenSSL error stack\n"; | ||
| 31 | + } | ||
0 commit comments