Node: the solution for the issue #6374 has being applied only for Node 6.0.0

[Anatoliy4041]

Dear all,
The issue #6374 was solved, however, it was applied just fo Node 6.0.0 version and there is no any changes in node_crypto.cc in higher versions. Could we sort it out?
Thanks!

[mscdex]

I'm not sure what you mean, originally it landed in v6.0.0 and it is also in v7.0.0.

[Anatoliy4041]

I just downloaded Node 7.1.0 and did't find it. In addition, there is no changes in the current https://github.com/nodejs/node/blob/master/src/node_crypto.cc

[gibfahn]

@Anatoliy4041 So this was added in v6.0.0, and was in v6 until v6.8.0.

It was overwritten by db411cf in master, and that was backported to v6.x in 1ea0358a. That commit should be in v6, v7, and master. Are you saying that @indutny's commit caused a regression?

[sam-github]

@Anatoliy4041 We'll close this soon unless you give more information, it doesn't look like an issue.

[Anatoliy4041]

Hi Guys! I'm appologize for the late answer.

Yes, I found the regression with regards to #6374 in lateset verion and it leads (in my case) to the issue I can't apply in NodeJS other crypto engines then those to be set as default in openssl (e.g. NodeJS doesn't recognize the algorithms provided by GOST engine). This problem was described in detail in #5739 by stefanmb so I just would like to propouse to fix it in lates versions of NodeJS

[sam-github]

@Anatoliy4041 sorry, but that didn't clarify anything... can we close this issue?

[Anatoliy4041]

@sam-github Could you please exlain what's not clear? Then I'll try to clerify in another way.
In my opinion, if there was the issue (described #5739, #6374) which were perfectly solved for elder version of NodeJS, however it is not applied for the latest one - that should be dealt in some way.

[gibfahn]

In my opinion, if there was the issue (described #5739, #6374) which were perfectly solved for elder version of NodeJS, however it is not applied for the latest one

This is not true, the commit is still in the latest versions of node (master, v6, and v7). However it was superseded by @indutny 's commit in v6.9.0, which made this change:

-  OPENSSL_config(NULL);
+  OPENSSL_no_config();

The issue should still be fixed though. So master, the latest v7, and the latest v6 should work.

If you think @indutny 's change broke the fix that @stefanmb applied, then you should find that v6.8.1 works, but v6.9.0 doesn't. If you have confirmed this then please let us know and we'll look into the issue. However the answer won't be to just apply #6374 to v6, that's already been done.

[gibfahn]

@Anatoliy4041 if you want to see if the node v6 contains the backport, you can just look for the commit in the tag like so:

Commit: d6237aa7c642e82b539803b6d2069eb4eddce6a6

git clone https://github.com/nodejs/node.git && cd node
git checkout v6.9.2 # Latest v6 version
git branch --contains d6237aa7c642e82b539803b6d2069eb4eddce6a6

You should see:

➜  node git:(94a3aef) git branch --contains d6237aa7c642e82b539803b6d2069eb4eddce6a6
* (HEAD detached at v6.9.2)

Which tells you that the v6.9.2 tag (latest v6 release) still contains the backport of Stefan's fix, it was just overwritten by @indutny's fix as explained above.

[fast0490f]

I understand how this debate is designed to discuss what you need to save OPENSSL_no_config (null); in all versions, and take this into account in future versions as a solution to the problem with additional encryption methods. The impetus was the use of [PR] (https://github.com/stefanmb/node/commit/4029815a0e1d6e4419a4e0ac8c8697f1edda09a0) version 6.x.x and completely absent in 7.x.x although complaints about this solution had not been reported. Maybe it's not a very elegant solution, and you need to consider more substantial, but it is very helpful, and I would like to keep it. Sorry for my English :\

[gibfahn]

@fast0490f This sentence isn't true:

and completely absent in 7.x.x

The commit was in v7, but it was replaced by something better.

---

what you need to save OPENSSL_no_config (null); in all versions

There was never an OPENSSL_no_config (null); in the code anywhere. Here's what happened:

• Stefan moves OPENSSL_config(NULL); in this commit: https://github.com/nodejs/node/commit/56b9478f53ed36dd28182bc0fdb8f2fecbe10adf
• Indutny replaces OPENSSL_config(NULL); with OPENSSL_no_config(); in this commit: https://github.com/nodejs/node/commit/c32be9a7aa2389f15d990ae70ebcc94f4c0be62b#diff-801e3948990f4965a8ea4aca4a423864

---

v7 should still work due to the second commit. No-one is saying they don't want this fix, we're saying we already have a better version of this fix.

[gibfahn]

@indutny this comment from @Anatoliy4041 appears to be saying that https://github.com/nodejs/node/commit/c32be9a7aa2389f15d990ae70ebcc94f4c0be62b caused a regression in the fix from https://github.com/nodejs/node/commit/56b9478f53ed36dd28182bc0fdb8f2fecbe10adf. Can you comment?

[indutny]

@gibfahn I see.

@Anatoliy4041 have you tried passing --openssl-config= argument to node? I can't see it anywhere in discussion. Hopefully this should work.

[fast0490f]

@gibfahn

what you need to save OPENSSL_no_config (null); in all versions

I made a mistake and meant `OPENSSL_config(NULL)

@indutny @Anatoliy4041

have you tried passing --openssl-config= argument to node?

I tried node --openssl-config="/etc/ssl/openssl.cnf" and it works.

tls.getCiphers();
Showing all connected encryption algorithms from openssl.cnf also checked the GOST algorithms and they are working. I confirm that everything works fine. I think can close the discussion

[gibfahn]

Okay great, I'll close the issue. @Anatoliy4041 if you still think there's a regression feel free to comment on here.