src: fix crypto.privateEncrypt fails first time

[liuxingbaoyu]

Fix crypto.privateEncrypt fails for the first time after crypto.generateKeyPairSync with certain parameters

Because the error stack is not cleaned up when crypto.generateKeyPairSync exits.

Fixes: #40814

[nodejs-github-bot]

Review requested:

• @nodejs/crypto

[tniessen]

It would be interesting to know why some ciphers populate the OpenSSL error queue even though WritePrivateKey appears to succeed.

[tniessen]

Please either rename this file to better match what it tests, or add the test case to an existing test file that tests key objects.

Also, please add a comment explaining that this is a regression test for #40814.

[liuxingbaoyu]

indeed! I'm also interested in why the exception is populated.

But I don't know about openssl and I didn't find relevant information on the documentation.

I'll show the stack for anyone with experience to investigate in depth.

Callstack:

node.exe!EVP_CIPHER_get_asn1_iv(evp_cipher_ctx_st * ctx, asn1_type_st * type) 行 56	C
node.exe!evp_cipher_asn1_to_param_ex(evp_cipher_ctx_st * c, asn1_type_st * type, evp_cipher_aead_asn1_params * asn1_params) 行 212	C
node.exe!EVP_CIPHER_asn1_to_param(evp_cipher_ctx_st * c, asn1_type_st * type) 行 42	C
node.exe!PKCS5_v2_PBE_keyivgen_ex(evp_cipher_ctx_st * ctx, const char * pass, int passlen, asn1_type_st * param, const evp_cipher_st * c, const evp_md_st * md, int en_de, ossl_lib_ctx_st * libctx, const char * propq) 行 160	C
node.exe!EVP_PBE_CipherInit_ex(asn1_object_st * pbe_obj, const char * pass, int passlen, asn1_type_st * param, evp_cipher_ctx_st * ctx, int en_de, ossl_lib_ctx_st * libctx, const char * propq) 行 151	C
node.exe!PKCS12_pbe_crypt_ex(const X509_algor_st * algor, const char * pass, int passlen, const unsigned char * in, int inlen, unsigned char * * data, int * datalen, int en_de, ossl_lib_ctx_st * libctx, const char * propq) 行 36	C
node.exe!PKCS12_item_i2d_encrypt_ex(X509_algor_st * algor, const ASN1_ITEM_st * it, const char * pass, int passlen, void * obj, int zbuf, ossl_lib_ctx_st * ctx, const char * propq) 行 189	C
node.exe!PKCS8_set0_pbe_ex(const char * pass, int passlen, pkcs8_priv_key_info_st * p8inf, X509_algor_st * pbe, ossl_lib_ctx_st * ctx, const char * propq) 行 76	C
node.exe!PKCS8_encrypt_ex(int pbe_nid, const evp_cipher_st * cipher, const char * pass, int passlen, unsigned char * salt, int saltlen, int iter, pkcs8_priv_key_info_st * p8inf, ossl_lib_ctx_st * libctx, const char * propq) 行 51	C
node.exe!p8info_to_encp8(pkcs8_priv_key_info_st * p8info, key2any_ctx_st * ctx) 行 120	C
node.exe!key_to_encp8(const void * key, int key_nid, void * params, int params_type, int(*)(const void *, unsigned char * *) k2d, key2any_ctx_st * ctx) 行 136	C
node.exe!key_to_epki_pem_priv_bio(bio_st * out, const void * key, int key_nid, const char * pemname, int(*)(const void *, int, int, void * *, int *) p2s, int(*)(const void *, unsigned char * *) k2d, key2any_ctx_st * ctx) 行 232	C
node.exe!key_to_pki_pem_priv_bio(bio_st * out, const void * key, int key_nid, const char * pemname, int(*)(const void *, int, int, void * *, int *) p2s, int(*)(const void *, unsigned char * *) k2d, key2any_ctx_st * ctx) 行 286	C
node.exe!key2any_encode(key2any_ctx_st * ctx, ossl_core_bio_st * cout, const void * key, int type, const char * pemname, int(*)(const void *, int) checker, int(*)(bio_st *, const void *, int, const char *, int(*)(const void *, int, int, void * *, int *), int(*)(const void *, unsigned char * *), key2any_ctx_st *) writer, int(*)(char *, unsigned __int64, unsigned __int64 *, const ossl_param_st *, void *) pwcb, void * pwcbarg, int(*)(const void *, int, int, void * *, int *) key2paramstring, int(*)(const void *, unsigned char * *) key2der) 行 1054	C
node.exe!rsa_to_PrivateKeyInfo_pem_encode(void * ctx, ossl_core_bio_st * cout, const void * key, const ossl_param_st * key_abstract, int selection, int(*)(char *, unsigned __int64, unsigned __int64 *, const ossl_param_st *, void *) cb, void * cbarg) 行 1350	C
node.exe!encoder_process(encoder_process_data_st * data) 行 635	C
node.exe!OSSL_ENCODER_to_bio(ossl_encoder_ctx_st * ctx, bio_st * out) 行 63	C
node.exe!do_pk8pkey(bio_st * bp, const evp_pkey_st * x, int isder, int nid, const evp_cipher_st * enc, const char * kstr, int klen, int(*)(char *, int, int, void *) cb, void * u, const char * propq) 行 125	C
node.exe!PEM_write_bio_PKCS8PrivateKey(bio_st * bp, const evp_pkey_st * x, const evp_cipher_st * enc, const char * kstr, int klen, int(*)(char *, int, int, void *) cb, void * u) 行 53	C
node.exe!node::crypto::`anonymous namespace'::WritePrivateKey(node::Environment * env, evp_pkey_st * pkey, const node::crypto::PrivateKeyEncodingConfig & config) 行 347	C++
node.exe!node::crypto::ManagedEVPPKey::ToEncodedPrivateKey(node::Environment * env, node::crypto::ManagedEVPPKey key, const node::crypto::PrivateKeyEncodingConfig & config, v8::Local<v8::Value> * out) 行 673	C++
node.exe!node::crypto::KeyPairGenTraits<node::crypto::RsaKeyGenTraits>::EncodeKey(node::Environment * env, node::crypto::KeyPairGenConfig<node::crypto::RsaKeyPairParams> * params, v8::Local<v8::Value> * result) 行 196	C++
node.exe!node::crypto::KeyGenJob<node::crypto::KeyPairGenTraits<node::crypto::RsaKeyGenTraits>>::ToResult(v8::Local<v8::Value> * err, v8::Local<v8::Value> * result) 行 107	C++
node.exe!node::crypto::CryptoJob<node::crypto::KeyPairGenTraits<node::crypto::RsaKeyGenTraits>>::Run(const v8::FunctionCallbackInfo<v8::Value> & args) 行 412	C++
node.exe!v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo handler) 行 153	C++
node.exe!v8::internal::`anonymous namespace'::HandleApiCallHelper<0>(v8::internal::Isolate * isolate, v8::internal::Handle<v8::internal::HeapObject> function, v8::internal::Handle<v8::internal::HeapObject> new_target, v8::internal::Handle<v8::internal::FunctionTemplateInfo> fun_data, v8::internal::Handle<v8::internal::Object> receiver, v8::internal::BuiltinArguments args) 行 114	C++

[liuxingbaoyu]

Also, do we need to add an assertion or something?

Once any method is called and the error is left, it is easy to affect other methods.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/43624/

[panva]

@liuxingbaoyu I believe there's a consistent failure when using linked OpenSSL 1.1.1

• ubi81_sharedlibs_openssl111fips_x64
• ubuntu1804_sharedlibs_openssl111_x64

[liuxingbaoyu]

This looks to be another problem with crypto.privateEncrypt, not crypto.generateKeyPairSync.

In openssl3, the error is cleared inside openssl, but not in openssl1.1.1.

I don't know if this is a bug or a behavior change.

Exception source:

    ERR_set_mark();
    ret = pem_read_bio_key_decoder(bp, x, ossl_pw_pem_password, &pwdata,
                                   libctx, propq, selection);
    if (ret == NULL
        && (BIO_seek(bp, pos) < 0
            || (ret = pem_read_bio_key_legacy(bp, x,
                                              ossl_pw_pem_password, &pwdata,
                                              libctx, propq,
                                              selection)) == NULL))
        ERR_clear_last_mark();
    else
        ERR_pop_to_mark();

node/src/crypto/crypto_keys.cc

Lines 257 to 258 in 6b004f1

	// OpenSSL can fail to parse the key but still return a non-null pointer.
	unsigned long err = ERR_peek_error(); // NOLINT(runtime/int)

Also, does anyone know what's the reason here?

[aduh95]

Ping @nodejs/crypto for (re)reviews.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/47557/

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/42793
FetchError: Invalid response body while trying to fetch https://api.github.com/graphql: Premature close
    at consumeBody (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/node_modules/node-fetch/src/body.js:234:60)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Response.text (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/node_modules/node-fetch/src/body.js:158:18)
    at async Request.json (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/lib/request.js:51:18)
    at async Request.query (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/lib/request.js:109:20)
    at async Request.queryAll (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/lib/request.js:136:20)
    at async Request.gql (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/lib/request.js:66:22)
    at async PRData.getComments (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/lib/pr_data.js:97:21)
    at async Promise.all (index 2)
    at async Promise.all (index 1) {
  type: 'system',
  errno: 'ERR_STREAM_PREMATURE_CLOSE',
  code: 'ERR_STREAM_PREMATURE_CLOSE',
  erroredSysCall: undefined
}

https://github.com/nodejs/node/actions/runs/3352810353

[aduh95]

Landed in e512786