Repository navigation
tls.createSecureContext SetKey doesn't respect OpenSSL engine set by crypto.setEngine #47008
Description
Activity
I'm skeptical that this is actually an engine issue because that "DECODER routines::unsupported" error means openssl doesn't understand the contents of the PEM file.
If you have a key that openssl doesn't understand natively but your custom engine does, you need to load it (by name) through
ENGINE_load_private_key(); that's what theprivateKeyEngineandprivateKeyIdentifieroptions are for.Are you sure that you can pass a path to pem file as privateKeyIdentifier? This trick doesn't work with gost-engine. As far as I understand, purpose of privateKeyIdentifier is a support of hardware tokens (key is hidden inside engine but not in a pem file).
As I mentioned above, node.js works with gost-engine correctly in Sign.sign().
SecureContext::SetKey tries to load the same key from the same pem via the same PEM_read_bio_PrivateKey function, but fails.Just to make it clear why I believe it's Node.js bug:
1). I use key.pem to sign a messageconst sign = crypto.createSign('md_gost12_256'); sign.update('message to sign'); const SignatureValue = sign.sign(fs.readFileSync('/path/to/key.pem'),'base64');Call stack is:
lib/internal/crypto/sig.js Sign.prototype.sign = function sign(options, encoding)
src/crypto/crypto_sig.cc void Sign::SignFinal
src/crypto/crypto_keys.cc ManagedEVPPKey ManagedEVPPKey::GetPrivateKeyFromJs
src/crypto/crypto_keys.cc ParseKeyResult ParsePrivateKey
pkey->reset(PEM_read_bio_PrivateKey(bio.get(), nullptr, PasswordCallback, &passphrase));
And it works!
2). I use key.pem to create TLS connection:const https = require('https'); const httpsAgent = new https.Agent({ ca: fs.readFileSync('/path/to/server_cert.pem'), rejectUnauthorized: false, secureOptions: crypto.constants.SSL_OP_IGNORE_UNEXPECTED_EOF, key: fs.readFileSync('/path/to/key.pem'), cert: fs.readFileSync('/path/topublic.cer') }); const axios = require ('axios'); const response = async () => { return await axios.get("https://api.dom.gosuslugi.ru:443", {httpsAgent}) } response() .then(res => { console.log('result', res.data); }) .catch(err => { console.log('error', err); })Call stack is:
lib/internal/tls/secure-context.js configSecureContext
lib/internal/tls/secure-context.js setKey
src/crypto/crypto_context.cc void SecureContext::SetKey
EVPKeyPointer key(PEM_read_bio_PrivateKey(bio.get(), nullptr, PasswordCallback, &pass_ptr));
And it fails!As you can see, the same key.pem, the same OpenSSL function PEM_read_bio_PrivateKey, but it works in one place and fails in another.
Yeah, node calls PEM_read_bio_PrivateKey() in both cases. It seems unlikely that it would parse the key successfully in one place but not the other.
Something else is probably going on but you have a bespoke setup. There's no way for me to reproduce any of that locally and no way to confirm whether it's actually a node issue. Your bug report is basically inactionable.
If you have time and inclination to investigate yourself, great. If not, we might as well close this.
Do you have any clue why Node loads engine in one case and ignores it in another?
I doubt that's what happens. Once an engine is loaded, it's loaded. What flags do you pass to
crypto.setEngine()?I don't pass any, because ALL is default.
The most weird thing is that tls.getCiphers() result includes ciphers added by engine even without crypto.setEngine.
You may want to investigate if it isn't a problem with the openssl library you link to.
Actually, setEngine is not needed in my configuration because engine is correctly described in openssl.cnf.
Sign.sign works correctly without setEngine. Tls.getCiphers shows needed cipher without setEngine. But configSecureContext fails.
I don't suppose this to be an OpenSSL issue because, as I mentioned in the original post, TLS connection using OpenSSL command line works fine:
openssl s_client -connect api.dom.gosuslugi.ru:443 -engine gost -verifyCAfile server.pem -partial_chain -cert public.cer -key private.pem -ignore_unexpected_eofDo I understand correctly that when I create new SecureContext it is different from the default Node.JS SecureContext, it's absolutely clean, doesn't load system openssl.cnf and doesn't know anything about engines etc?
No. SecureContext maps directly to openssl's SSL_CTX. It's still affected by things like
crypto.setEngine()because that's a global operation, not a per-context setting.I found it! It turned out to be some error in your BIO loader. I've replaced in "void SecureContext::SetKey" function of src/crypto/crypto_context.cc the code:
BIOPointer bio(LoadBIO(env, args[0]));
withByteSource bkey = ByteSource::FromStringOrBuffer(env, args[0]); BIOPointer bio(BIO_new_mem_buf(bkey.data<char>(), bkey.size()));and everything works! The correct code was taken from ParsePrivateKey function of src/crypto/crypto_keys.cc
I looked at NodeBIO but I don't think it does anything materially different from BIO_new_mem_buf() so... 🤷
If you can identify where it diverges, it can probably be fixed.
1 remaining item
I've investigated further. The problem is that when you create bio like this:
BIOPointer bio(LoadBIO(env, args[0]));we get bio with seek disabled andBIO_seek(bio.get(), 0);doesn't work.
More details: I looked into OpenSSL source (crypto/pem/pem_pkey.c). PEM_read_bio_PrivateKey function first tries to read bio with pem_read_bio_key function, it fails and then pem_read_bio_key_legacy function is used. But after pem_read_bio_key bio pointer doesn't point to the beginning anymore, so BIO_seek(bio, 0) is called. In works with bio created by BIO_new_mem_buf, but not with bio created by LoadBIO.As an experiment, I did this:
ByteSource bkey = ByteSource::FromStringOrBuffer(env, args[0]); //1st (working) way to create BIO BIOPointer bio(BIO_new_mem_buf(bkey.data<char>(), bkey.size())); char*linebuf = (char*)malloc(256); int len = BIO_gets(bio.get(), linebuf, 255); printf("%s-%i", linebuf, len); //print 1st line of BIO BIO_seek(bio.get(), 0); //return BIO position back to 0 len = BIO_gets(bio.get(), linebuf, 255); printf("%s-%i", linebuf, len); //print 1st line of BIO again BIOPointer bio(LoadBIO(env, args[0])); //2nd (buggy) way to create BIO len = BIO_gets(bio.get(), linebuf, 255); printf("%s-%i", linebuf, len); //print 1st line of BIO BIO_seek(bio.get(), 0); //should return BIO position back to 0, but doesn't! len = BIO_gets(bio.get(), linebuf, 255); printf("%s-%i", linebuf, len); //should print 1st line, but prints 2nd line of BIO!NodeBIO::Ctrl() doesn't implement BIO_C_FILE_SEEK (nor BIO_C_FILE_TELL) so that does indeed seem like a likely culprit. OpenSSL doesn't normally BIO_seek() when parsing data structures so that's probably why it went unnoticed for so long.
If implementing those controls helps the GOST engine, then PR welcome.
As far as I see, NodeBIO is based on singly-linked buffers. So I can seek within current buffer and I can switch to the next buffer. But how should I jump to the first buffer?
Please tell me the best choice:
1). We assume that 1024 bytes of single buffer is enough for any PEM key and I implement seeking withing current buffer only.
2). I make buffer chain linked in both sides.
3). I add in NodeBIO a new element - link to the first buffer chunk.There's also option 4) get rid of NodeBIO in crypto_context.cc and use openssl's own memory BIOs. :-)
NodeBIO makes sense for TLS traffic but less so for loading keys or certificates. Loading private keys with BIO_s_secmem() is also desirable, see #30956.
As an example: is it OK if I write
BIO *bio = BIO_new(BIO_s_secmem()); if (!bio) return; ByteSource bsrc = ByteSource::FromStringOrBuffer(env, args[0]); int written = BIO_write(bio, bsrc.data<char>(), bsrc.size()); if (!written) return;instead of
BIOPointer bio(LoadBIO(env, args[0]));?
(It works)The first line should be:
BIOPointer bio(BIO_new(BIO_s_secmem()));Because automatic resource management > manually calling BIO_free.
Check that
written == bsrc.size()in case of partial writes. (Unlikely with memory BIOs but still.)So I've rewritten LoadBIO this way:
BIOPointer LoadBIO(Environment* env, Local<Value> v) { if (v->IsString() || v->IsArrayBufferView()) { BIOPointer bio(BIO_new(BIO_s_secmem())); if (!bio) return nullptr; ByteSource bsrc = ByteSource::FromStringOrBuffer(env, v); int written = BIO_write(bio.get(), bsrc.data<char>(), bsrc.size()); if (written != bsrc.size()) return nullptr; return bio; } return nullptr; }Is it pretty enough?
Looks good to me modulo the signed-vs-unsigned comparison. If you open a pull request, I'll run it through the CI.
- added a commit that references this issue
on May 18, 2023 - added a commit that references this issue
on May 30, 2023 - added a commit that references this issue
on Jul 6, 2023 - added a commit that references this issue
on Jul 6, 2023
Version
19.7.0
Platform
Linux 5.10.0-8-amd64 #1 SMP Debian 5.10.46-4 (2021-08-03) x86_64 GNU/Linux
Subsystem
crypto
What steps will reproduce the bug?
I'm trying to create TLS connection using custom OpenSSL engine (gost-engine). OpenSSL version is 3.0.8 and engine are installed correctly (
openssl s_client -connect api.dom.gosuslugi.ru:443 -engine gost -verifyCAfile server.pem -partial_chain -cert public.cer -key private.pem -ignore_unexpected_eofworks perfectly). Node.JS is recompiled with shared OpenSSL support (./configure --shared-openssl --shared-openssl-libpath=/usr/local/ssl/lib --shared-openssl-includes=/usr/local/ssl/include). Private key is in a .pem file so it should be loaded with "key" option, not "privateKeyIdentifier".So I try to tls.createSecureContext with key: fs.readFileSync('/path/to/private/key.pem').
How often does it reproduce? Is there a required condition?
Always.
What is the expected behavior?
tls.createSecureContext should finish with no errors.
What do you see instead?
I get an error:
Additional information
Setting engine by crypto.setEngine doesn't help.
As far as I see, Node.JS uses PEM_read_bio_PrivateKey both in src/crypto/crypto_keys.cc/ParsePrivateKey for Sign.sign() routine and in src/crypto/crypto_context.cc/SecureContext::SetKey for tls.createSecureContext calls. While in first place it understands which engine is set, in second place the engine is ignored.