FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

crypto: pass empty passphrases to OpenSSL properly · nodejs/node@0ebf44b · GitHub

/ node Public

Commit 0ebf44b

Browse files
tniessen
authored and
targos
committed
crypto: pass empty passphrases to OpenSSL properly
Fixes: #35898 PR-URL: #35914 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 2868f52 commit 0ebf44b

7 files changed

Lines changed: 103 additions & 24 deletions

File tree

‎src/crypto/crypto_context.cc‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -522,13 +522,19 @@ void SecureContext::SetKey(const FunctionCallbackInfo<Value>& args) {
522522
if (!bio)
523523
return;
524524

525-
node::Utf8Value passphrase(env->isolate(), args[1]);
525+
ByteSource passphrase;
526+
if (args[1]->IsString())
527+
passphrase = ByteSource::FromString(env, args[1].As<String>());
528+
// This redirection is necessary because the PasswordCallback expects a
529+
// pointer to a pointer to the passphrase ByteSource to allow passing in
530+
// const ByteSources.
531+
const ByteSource* pass_ptr = &passphrase;
526532

527533
EVPKeyPointer key(
528534
PEM_read_bio_PrivateKey(bio.get(),
529535
nullptr,
530536
PasswordCallback,
531-
*passphrase));
537+
&pass_ptr));
532538

533539
if (!key) {
534540
unsigned long err = ERR_get_error(); // NOLINT(runtime/int)

‎src/crypto/crypto_keygen.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,8 +251,10 @@ struct KeyPairGenConfig final : public MemoryRetainer {
251251

252252
void MemoryInfo(MemoryTracker* tracker) const override {
253253
tracker->TrackField("key", key);
254-
tracker->TrackFieldWithSize("private_key_encoding.passphrase",
255-
private_key_encoding.passphrase_.size());
254+
if (!private_key_encoding.passphrase_.IsEmpty()) {
255+
tracker->TrackFieldWithSize("private_key_encoding.passphrase",
256+
private_key_encoding.passphrase_->size());
257+
}
256258
tracker->TrackField("params", params);
257259
}
258260

‎src/crypto/crypto_keys.cc‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,7 @@ ParseKeyResult ParsePrivateKey(EVPKeyPointer* pkey,
210210
const PrivateKeyEncodingConfig& config,
211211
const char* key,
212212
size_t key_len) {
213-
// OpenSSL needs a non-const pointer, that's why the const_cast is required.
214-
char* const passphrase = const_cast<char*>(config.passphrase_.get());
213+
const ByteSource* passphrase = config.passphrase_.get();
215214

216215
if (config.format_ == kKeyFormatPEM) {
217216
BIOPointer bio(BIO_new_mem_buf(key, key_len));
@@ -221,7 +220,7 @@ ParseKeyResult ParsePrivateKey(EVPKeyPointer* pkey,
221220
pkey->reset(PEM_read_bio_PrivateKey(bio.get(),
222221
nullptr,
223222
PasswordCallback,
224-
passphrase));
223+
&passphrase));
225224
} else {
226225
CHECK_EQ(config.format_, kKeyFormatDER);
227226

@@ -238,7 +237,7 @@ ParseKeyResult ParsePrivateKey(EVPKeyPointer* pkey,
238237
pkey->reset(d2i_PKCS8PrivateKey_bio(bio.get(),
239238
nullptr,
240239
PasswordCallback,
241-
passphrase));
240+
&passphrase));
242241
} else {
243242
PKCS8Pointer p8inf(d2i_PKCS8_PRIV_KEY_INFO_bio(bio.get(), nullptr));
244243
if (p8inf)
@@ -260,7 +259,7 @@ ParseKeyResult ParsePrivateKey(EVPKeyPointer* pkey,
260259
return ParseKeyResult::kParseKeyOk;
261260
if (ERR_GET_LIB(err) == ERR_LIB_PEM &&
262261
ERR_GET_REASON(err) == PEM_R_BAD_PASSWORD_READ) {
263-
if (config.passphrase_.get() == nullptr)
262+
if (config.passphrase_.IsEmpty())
264263
return ParseKeyResult::kParseKeyNeedPassphrase;
265264
}
266265
return ParseKeyResult::kParseKeyFailed;
@@ -293,6 +292,28 @@ MaybeLocal<Value> WritePrivateKey(
293292
BIOPointer bio(BIO_new(BIO_s_mem()));
294293
CHECK(bio);
295294

295+
// If an empty string was passed as the passphrase, the ByteSource might
296+
// contain a null pointer, which OpenSSL will ignore, causing it to invoke its
297+
// default passphrase callback, which would block the thread until the user
298+
// manually enters a passphrase. We could supply our own passphrase callback
299+
// to handle this special case, but it is easier to avoid passing a null
300+
// pointer to OpenSSL.
301+
char* pass = nullptr;
302+
size_t pass_len = 0;
303+
if (!config.passphrase_.IsEmpty()) {
304+
pass = const_cast<char*>(config.passphrase_->get());
305+
pass_len = config.passphrase_->size();
306+
if (pass == nullptr) {
307+
// OpenSSL will not actually dereference this pointer, so it can be any
308+
// non-null pointer. We cannot assert that directly, which is why we
309+
// intentionally use a pointer that will likely cause a segmentation fault
310+
// when dereferenced.
311+
CHECK_EQ(pass_len, 0);
312+
pass = reinterpret_cast<char*>(-1);
313+
CHECK_NE(pass, nullptr);
314+
}
315+
}
316+
296317
bool err;
297318

298319
PKEncodingType encoding_type = config.type_.ToChecked();
@@ -303,12 +324,11 @@ MaybeLocal<Value> WritePrivateKey(
303324
RSAPointer rsa(EVP_PKEY_get1_RSA(pkey));
304325
if (config.format_ == kKeyFormatPEM) {
305326
// Encode PKCS#1 as PEM.
306-
const char* pass = config.passphrase_.get();
307327
err = PEM_write_bio_RSAPrivateKey(
308328
bio.get(), rsa.get(),
309329
config.cipher_,
310-
reinterpret_cast<unsigned char*>(const_cast<char*>(pass)),
311-
config.passphrase_.size(),
330+
reinterpret_cast<unsigned char*>(pass),
331+
pass_len,
312332
nullptr, nullptr) != 1;
313333
} else {
314334
// Encode PKCS#1 as DER. This does not permit encryption.
@@ -322,17 +342,17 @@ MaybeLocal<Value> WritePrivateKey(
322342
err = PEM_write_bio_PKCS8PrivateKey(
323343
bio.get(), pkey,
324344
config.cipher_,
325-
const_cast<char*>(config.passphrase_.get()),
326-
config.passphrase_.size(),
345+
pass,
346+
pass_len,
327347
nullptr, nullptr) != 1;
328348
} else {
329349
// Encode PKCS#8 as DER.
330350
CHECK_EQ(config.format_, kKeyFormatDER);
331351
err = i2d_PKCS8PrivateKey_bio(
332352
bio.get(), pkey,
333353
config.cipher_,
334-
const_cast<char*>(config.passphrase_.get()),
335-
config.passphrase_.size(),
354+
pass,
355+
pass_len,
336356
nullptr, nullptr) != 1;
337357
}
338358
} else {
@@ -344,12 +364,11 @@ MaybeLocal<Value> WritePrivateKey(
344364
ECKeyPointer ec_key(EVP_PKEY_get1_EC_KEY(pkey));
345365
if (config.format_ == kKeyFormatPEM) {
346366
// Encode SEC1 as PEM.
347-
const char* pass = config.passphrase_.get();
348367
err = PEM_write_bio_ECPrivateKey(
349368
bio.get(), ec_key.get(),
350369
config.cipher_,
351-
reinterpret_cast<unsigned char*>(const_cast<char*>(pass)),
352-
config.passphrase_.size(),
370+
reinterpret_cast<unsigned char*>(pass),
371+
pass_len,
353372
nullptr, nullptr) != 1;
354373
} else {
355374
// Encode SEC1 as DER. This does not permit encryption.
@@ -640,7 +659,8 @@ ManagedEVPPKey::GetPrivateKeyEncodingFromJs(
640659
THROW_ERR_OUT_OF_RANGE(env, "passphrase is too big");
641660
return NonCopyableMaybe<PrivateKeyEncodingConfig>();
642661
}
643-
result.passphrase_ = passphrase.ToNullTerminatedCopy();
662+
result.passphrase_ = NonCopyableMaybe<ByteSource>(
663+
passphrase.ToNullTerminatedCopy());
644664
} else {
645665
CHECK(args[*offset]->IsNullOrUndefined() && !needs_passphrase);
646666
}

‎src/crypto/crypto_keys.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,10 @@ using PublicKeyEncodingConfig = AsymmetricKeyEncodingConfig;
6363

6464
struct PrivateKeyEncodingConfig : public AsymmetricKeyEncodingConfig {
6565
const EVP_CIPHER* cipher_;
66-
ByteSource passphrase_;
66+
// The ByteSource alone is not enough to distinguish between "no passphrase"
67+
// and a zero-length passphrase (which can be a null pointer), therefore, we
68+
// use a NonCopyableMaybe.
69+
NonCopyableMaybe<ByteSource> passphrase_;
6770
};
6871

6972
// This uses the built-in reference counter of OpenSSL to manage an EVP_PKEY

‎src/crypto/crypto_util.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,13 @@ bool EntropySource(unsigned char* buffer, size_t length) {
7474
}
7575

7676
int PasswordCallback(char* buf, int size, int rwflag, void* u) {
77-
const char* passphrase = static_cast<char*>(u);
77+
const ByteSource* passphrase = *static_cast<const ByteSource**>(u);
7878
if (passphrase != nullptr) {
7979
size_t buflen = static_cast<size_t>(size);
80-
size_t len = strlen(passphrase);
80+
size_t len = passphrase->size();
8181
if (buflen < len)
8282
return -1;
83-
memcpy(buf, passphrase, len);
83+
memcpy(buf, passphrase->get(), len);
8484
return len;
8585
}
8686

‎src/util.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,15 @@ class NonCopyableMaybe {
602602
return empty_;
603603
}
604604

605+
const T* get() const {
606+
return empty_ ? nullptr : &value_;
607+
}
608+
609+
const T* operator->() const {
610+
CHECK(!empty_);
611+
return &value_;
612+
}
613+
605614
T&& Release() {
606615
CHECK_EQ(empty_, false);
607616
empty_ = true;

‎test/parallel/test-crypto-keygen.js‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ if (!common.hasCrypto)
77
const assert = require('assert');
88
const {
99
constants,
10+
createPrivateKey,
1011
createSign,
1112
createVerify,
1213
generateKeyPair,
@@ -1171,3 +1172,41 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
11711172
);
11721173
}
11731174
}
1175+
1176+
{
1177+
// Passing an empty passphrase string should not cause OpenSSL's default
1178+
// passphrase prompt in the terminal.
1179+
// See https://github.com/nodejs/node/issues/35898.
1180+
1181+
for (const type of ['pkcs1', 'pkcs8']) {
1182+
generateKeyPair('rsa', {
1183+
modulusLength: 1024,
1184+
privateKeyEncoding: {
1185+
type,
1186+
format: 'pem',
1187+
cipher: 'aes-256-cbc',
1188+
passphrase: ''
1189+
}
1190+
}, common.mustSucceed((publicKey, privateKey) => {
1191+
assert.strictEqual(publicKey.type, 'public');
1192+
1193+
for (const passphrase of ['', Buffer.alloc(0)]) {
1194+
const privateKeyObject = createPrivateKey({
1195+
passphrase,
1196+
key: privateKey
1197+
});
1198+
assert.strictEqual(privateKeyObject.asymmetricKeyType, 'rsa');
1199+
}
1200+
1201+
// Encrypting with an empty passphrase is not the same as not encrypting
1202+
// the key, and not specifying a passphrase should fail when decoding it.
1203+
assert.throws(() => {
1204+
return testSignVerify(publicKey, privateKey);
1205+
}, {
1206+
name: 'TypeError',
1207+
code: 'ERR_MISSING_PASSPHRASE',
1208+
message: 'Passphrase required for encrypted key'
1209+
});
1210+
}));
1211+
}
1212+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL