tls.createSecureContext SetKey doesn't respect OpenSSL engine set by crypto.setEngine

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_eof works 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:

error Error: error:1E08010C:DECODER routines::unsupported
    at setKey (node:internal/tls/secure-context:92:11)
    at configSecureContext (node:internal/tls/secure-context:174:7)
    at Object.createSecureContext (node:_tls_common:118:3)
    at Object.connect (node:_tls_wrap:1631:48)
    at Agent.createConnection (node:https:150:22)
    at Agent.createSocket (node:_http_agent:342:26)
    at Agent.addRequest (node:_http_agent:289:10)
    at new ClientRequest (node:_http_client:338:16)
    at Object.request (node:https:360:10)
    at RedirectableRequest._performRequest (/home/GKU-data/JS/node_modules/follow-redirects/index.js:284:24) {
  library: 'DECODER routines',
  reason: 'unsupported',
  code: 'ERR_OSSL_UNSUPPORTED'

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.

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 the privateKeyEngine and privateKeyIdentifier options 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 message

const 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_eof

Do 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]));
with

ByteSource 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.

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 and BIO_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.

crypto: use openssl's own memory BIOs in crypto_context.cc

Navigation Menu

Repository navigation

tls.createSecureContext SetKey doesn't respect OpenSSL engine set by crypto.setEngine #47008

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior?

What do you see instead?

Additional information

1 remaining item

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

tls.createSecureContext SetKey doesn't respect OpenSSL engine set by crypto.setEngine #47008

Description

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior?

What do you see instead?

Additional information

Activity

bnoordhuis commented on Mar 8, 2023

GauriSpears commented on Mar 8, 2023

GauriSpears commented on Mar 8, 2023

bnoordhuis commented on Mar 8, 2023

GauriSpears commented on Mar 9, 2023

bnoordhuis commented on Mar 9, 2023

GauriSpears commented on Mar 9, 2023

GauriSpears commented on Mar 9, 2023

bnoordhuis commented on Mar 9, 2023

GauriSpears commented on Mar 9, 2023

GauriSpears commented on Mar 9, 2023

bnoordhuis commented on Mar 10, 2023

GauriSpears commented on Mar 12, 2023

bnoordhuis commented on Mar 12, 2023

1 remaining item

GauriSpears commented on Mar 15, 2023

bnoordhuis commented on Mar 16, 2023

GauriSpears commented on Mar 17, 2023

bnoordhuis commented on Mar 17, 2023

GauriSpears commented on Mar 18, 2023

bnoordhuis commented on Mar 18, 2023

GauriSpears commented on Mar 18, 2023

bnoordhuis commented on Mar 18, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions