Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing exports : TLSv1_2_client_method, TLSv1_2_server_method #20369

Closed
d3x0r opened this issue Apr 27, 2018 · 21 comments
Closed

Missing exports : TLSv1_2_client_method, TLSv1_2_server_method #20369

d3x0r opened this issue Apr 27, 2018 · 21 comments
Labels
addons Issues and PRs related to native addons. build Issues and PRs related to build files or the CI. openssl Issues and PRs related to the OpenSSL dependency. windows Issues and PRs related to the Windows platform.

Comments

@d3x0r
Copy link
Contributor

d3x0r commented Apr 27, 2018

Version: 10.0.0
Platform: Win32-x64
Subsystem:

TLSv1_2_client_method and TLSv1_2_server_method are not exported.

there is TLS_client_method() and TLS_server_method(); but then node versions previous to 10 don't export that, but instead export the previous methods.

As a workaround - going to investigate a node version definition to #ifdef the code.

@d3x0r d3x0r changed the title Missing exports : TLSv1_2_client_methodm, TLSv1_2_server_method Missing exports : TLSv1_2_client_method, TLSv1_2_server_method Apr 27, 2018
@addaleax addaleax added windows Issues and PRs related to the Windows platform. openssl Issues and PRs related to the OpenSSL dependency. addons Issues and PRs related to native addons. labels Apr 27, 2018
@addaleax
Copy link
Member

Just for clarity: What you are saying is that these symbols are not available to addons on Windows when they should be?

@d3x0r
Copy link
Contributor Author

d3x0r commented Apr 27, 2018

They previously were available, so addons can link to and use the same version of OpenSSL as node.


Edit: very annoying to workaround; the code that links against most of OpenSSL doesn't itself know about node.h (node_version.h) and it's not an issue with OpenSSL itself; that and I no longer get to control 'only 1.2' instead of 'best available'

Edit2: Node-gyp doesn't seem to have/get the node version to pass to projects; I can get that with cmake-js but was trying to support both (for those that don't have cmake for some odd reason). So I can't even pass it a define when compiling to specify the NODE_MAJOR_VERSION.

@bnoordhuis
Copy link
Member

Should be easy to fix. @d3x0r Does this patch work for you?

diff --git a/node.gyp b/node.gyp
index ba65eafee9..694c3e86d6 100644
--- a/node.gyp
+++ b/node.gyp
@@ -594,7 +594,7 @@
               # Categories to export.
               '-CAES,BF,BIO,DES,DH,DSA,EC,ECDH,ECDSA,ENGINE,EVP,HMAC,MD4,MD5,'
               'PSK,RC2,RC4,RSA,SHA,SHA0,SHA1,SHA256,SHA512,SOCK,STDIO,TLSEXT,'
-              'FP_API',
+              'FP_API,TLS1_2_METHOD',
               # Defines.
               '-DWIN32',
               # Symbols to filter from the export list.

@bnoordhuis bnoordhuis added the build Issues and PRs related to build files or the CI. label Apr 28, 2018
@bnoordhuis
Copy link
Member

@d3x0r Any luck with that patch?

@d3x0r
Copy link
Contributor Author

d3x0r commented Apr 30, 2018

Applied to version tagged 10.0.0
And ... I dunno; it's not building.

  v8_inspector_copy_protocol_to_intermediate_folder
  cp: cannot create regular file `H:\\dev2\\node\\Release\\obj\\global_intermediate/deps\\v8\\src\\inspector\\js_protocol.pdl': No such file or directory
C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 1. [H:\dev2\n
ode\v8_inspector_compress_protocol_json.vcxproj]
  v8_libbase.vcxproj -> H:\dev2\node\Release\lib\v8_libbase.lib
  v8_libplatform.vcxproj -> H:\dev2\node\Release\lib\v8_libplatform.lib
  v8_libsampler.vcxproj -> H:\dev2\node\Release\lib\v8_libsampler.lib
  zlib.vcxproj -> H:\dev2\node\Release\lib\zlib.lib
Attempting to cancel the build...
C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(1074,5): error MSB6006: "Lib.exe" exited with code -1073741510.
 [H:\dev2\node\deps\openssl\openssl.vcxproj]
Terminate batch job (Y/N)?
^C

H:\dev2\node>dir release\obj\global_intermediate\deps\
The system cannot find the file specified.

even cleaned everything from old builds so it would build clean... but no joy; I don't know.

if you can get a node 10.0.0 to build; the node.lib should contain

TLSv1_2_server_method TLSv1_2_client_method

should be able to be found in clear text

@addaleax
Copy link
Member

@d3x0r I’m not sure but that might be caused by 2b85127. Can you revert that locally and try again? I don’t understand why it would fail for you but not in CI, but there’s a decent chance it’s causing that issue

@d3x0r
Copy link
Contributor Author

d3x0r commented Apr 30, 2018

  • 2018-04-24
    committed 5 days ago (I think that is after the tag I have?) Commits on Apr 27, 2018

That's after V10.0.0 tag.

SHA-1: cf41627

  • 2018-04-24, Version 10.0.0 (Current)

I guess it's not that long ago; I can just build against head...

@addaleax
Copy link
Member

@d3x0r Yes, might even be the case that that fixes the problem for you then. 😄 The patch completely changed the mechanism of the copy operation that’s failing there, so I just thought it would be related to that

@d3x0r
Copy link
Contributor Author

d3x0r commented Apr 30, 2018

Yes that change restores the two missing methods that I know of :) (clicked wrong button close and commented)

@d3x0r d3x0r closed this as completed Apr 30, 2018
@d3x0r d3x0r reopened this Apr 30, 2018
bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue May 18, 2018
Upstream deprecated them and moved them into categories of their own.
Add those categories to the export list.  Node.js doesn't use them but
some add-ons do.

Fixes: nodejs#20369
PR-URL: nodejs#20712
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nie�en <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this issue May 22, 2018
Upstream deprecated them and moved them into categories of their own.
Add those categories to the export list.  Node.js doesn't use them but
some add-ons do.

Fixes: #20369
PR-URL: #20712
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nie�en <tniessen@tnie.de>
@d3x0r
Copy link
Contributor Author

d3x0r commented Nov 5, 2018

unresolved external symbol TLSv1_2_server_method
Has appeared again in node 11.0 ( I guess is still an issue on released Node versions?)

@sam-github
Copy link
Contributor

I can look into the export defs, but how do I reproduce this? I'm not seeing a difference between 6 and 11, am I using readelf incorrectly?

w/core % nvm i 6; node --version; readelf -Ws =node | egrep " TLSv1_2_server_method"
v6.14.4 is already installed.
Now using node v6.14.4 (npm v3.10.10)
v6.14.4
 23946: 0000000000fe1b30    11 FUNC    GLOBAL DEFAULT   13 TLSv1_2_server_method
  8416: 000000000166db20   232 OBJECT  LOCAL  DEFAULT   15 TLSv1_2_server_method_data.16118
 46211: 0000000000fe1b30    11 FUNC    GLOBAL DEFAULT   13 TLSv1_2_server_method
w/core % nvm i 11; node --version; readelf -Ws =node | egrep " TLSv1_2_server_method"
v11.1.0 is already installed.
Now using node v11.1.0 (npm v6.4.1)
v11.1.0
 28414: 00000000013a0120    11 FUNC    GLOBAL DEFAULT   14 TLSv1_2_server_method
 59294: 00000000013a0120    11 FUNC    GLOBAL DEFAULT   14 TLSv1_2_server_method

@d3x0r
Copy link
Contributor Author

d3x0r commented Nov 5, 2018

Hmm On windows....

node.lib
defniatly does not have v1_2...

__imp_DTLS_server_method
TLS_server_method
__imp_TLS_server_method
DTLS_server_method
TLS_server_method
__imp_DTLS_server_method

@sam-github
Copy link
Contributor

Ah, sorry, I've no idea how to fix that on Windows. Perhaps @refack does?

@d3x0r
Copy link
Contributor Author

d3x0r commented Nov 12, 2018

Can this be reopened?

@refack
Copy link
Contributor

refack commented Nov 12, 2018

Hello @d3x0r,
I'll see what I can do. If I can formulate a way to test this, and we figure out what is causing it, we will do our best to fix it, regardless of if the issue is open or closed.

@refack refack self-assigned this Nov 12, 2018
@d3x0r
Copy link
Contributor Author

d3x0r commented Nov 12, 2018

node-gyp configure build

// warning - casting from the definition in the header, to an int....
m:\tmp\node_test\test.c(4): warning C4311: 'type cast': pointer truncation from 'const SSL_METHOD *(__cdecl *)(void)' to 'int' [M:\tmp\node_test\build\test_tls_method.vcxproj]

// and then it fails to link.
test.obj : error LNK2001: unresolved external symbol TLSv1_2_server_method [M:\tmp\node_test\build\test_tls_method.vcxproj]
M:\tmp\node_test\build\Release\test_tls_method.node : fatal error LNK1120: 1 unresolved externals [M:\tmp\node_test\build\test_tls_method.vcxproj]

binding.gyp

{
  "targets": [
    {
      "target_name": "test_tls_method",
      'win_delay_load_hook': 'false',      
      "sources": [ "test.c",
          ],
    }
  ],
}

test.c

#include <openssl/ssl.h>

int main( void ) {
   return (int)TLSv1_2_server_method;
}

Could make it a gist instead....

@d3x0r
Copy link
Contributor Author

d3x0r commented Nov 12, 2018

#20369 (comment)
this worked before...

@refack
Copy link
Contributor

refack commented Nov 12, 2018

Thanks! This will surely help. I hope to get to it tomorrow.

@refack
Copy link
Contributor

refack commented Nov 13, 2018

So the problem is that the TLSv1_2_* were deprecated in openSSL_1_1:

TLSv1_server_method 158 1_1_0 EXIST::FUNCTION:DEPRECATEDIN_1_1_0,TLS1_METHOD

PR - #24329

@richardlau
Copy link
Member

Opened #25991.

richardlau added a commit to richardlau/node-1 that referenced this issue Feb 7, 2019
Methods such as `TLSv1_server_method` are categorized as
`DEPRECATEDIN_1_1_0`. Add the deprecated categories to
the list of categories to include passed to `mkssldef.py`.

Adds a regression test to `test/addons/openssl-binding`.

Refs: nodejs#20369
Refs: nodejs#25981
@refack refack removed their assignment Feb 7, 2019
danbev pushed a commit that referenced this issue Feb 11, 2019
Methods such as `TLSv1_server_method` are categorized as
`DEPRECATEDIN_1_1_0`. Add the deprecated categories to
the list of categories to include passed to `mkssldef.py`.

Adds a regression test to `test/addons/openssl-binding`.

PR-URL: #25991
Refs: #20369
Refs: #25981
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
targos pushed a commit that referenced this issue Feb 11, 2019
Methods such as `TLSv1_server_method` are categorized as
`DEPRECATEDIN_1_1_0`. Add the deprecated categories to
the list of categories to include passed to `mkssldef.py`.

Adds a regression test to `test/addons/openssl-binding`.

PR-URL: #25991
Refs: #20369
Refs: #25981
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@richardlau
Copy link
Member

Fixed by fcaeb1f.

Released in Node.js 11.10.0 (d5d163d8b9).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addons Issues and PRs related to native addons. build Issues and PRs related to build files or the CI. openssl Issues and PRs related to the OpenSSL dependency. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants