Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

windows: addons cannot use the bundled OpenSSL #4051

Closed
TooTallNate opened this issue Sep 23, 2012 · 24 comments
Closed

windows: addons cannot use the bundled OpenSSL #4051

TooTallNate opened this issue Sep 23, 2012 · 24 comments

Comments

@TooTallNate
Copy link

Basically, on Unix, addons can use functions from node's bundled openssl rather easily. So there's no problem there

But on Windows, the openssl functions are not exported it seems. We explicitly export the libuv symbols for addons to use so it would be nice to do the same for OpenSSL.

This wiki article explains the current state of affairs in more detail: https://github.com/TooTallNate/node-gyp/wiki/Linking-to-OpenSSL

@piscisaureus you, me and @ry talked about this privately over email a while back and thought it was something we should get fixed.

@bnoordhuis
Copy link
Member

Basically, on Unix, addons can use functions from node's bundled openssl rather easily. So there's no problem there

There is: the node binary only exports what it uses itself, everything else is stripped at link time.

@TooTallNate
Copy link
Author

Ya I had a feeling that would be the case. So basically the same problem exists on Unix, to a certain extent, but you could get lucky...

@TooTallNate
Copy link
Author

@piscisaureus I know you were working on this, is there a patch to try?

@piscisaureus
Copy link

@tjfontaine
Copy link

I meant to spend time comparing -dynamic-list to -whole-archive in size, but the mac linker flag that would appear to match -dynamic-list is -reexported_symbols_list but I think it would be easier to maintain using -reexport_library

@bnoordhuis
Copy link
Member

I meant to spend time comparing -dynamic-list to -whole-archive in size

There are a couple of issues with those switches:

  1. I don't think gyp lets us selectively turn on --whole-archive. Setting it indiscriminately means you pull in all of libc as well.
  2. --dynamic-list doesn't work with old versions of GNU ld or the Solaris linker.

I think someone needs to look into teaching gyp to link with -Wl,--whole-archive -lopenssl -Wl,--no-whole-archive

@mscdex
Copy link

mscdex commented Mar 29, 2013

Any update on this? I'd love to trim my mariasql addon some more by removing the yaSSL dependency and linking against the bundled OpenSSL instead. Linking to the bundled OpenSSL already works great on Linux, but no go on Windows still. :-(

EDIT: I should also note that node-gyp needs to add the openssl directory to the list of include_dirs too. I tried adding the relevant line to my include_dirs in my binding.gyp but for some reason it did not pick it up (node_root_dir isn't defined for binding.gyp and it silently skips it maybe?).

@TooTallNate
Copy link
Author

@mscdex node_root_dir should definitely be set. node-gyp sets it explicitly when invoking gyp: https://github.com/TooTallNate/node-gyp/blob/b0069dbdd5cebcb35d1158c0ed7f1310519a7c50/lib/configure.js#L389

@mscdex
Copy link

mscdex commented May 13, 2013

Looks like someone was cleaning house and deleted the openssl-exports branch by @piscisaureus. What's the status on making openssl exports available? :-S

@trevnorris
Copy link

Went ahead and restored the branch. Not sure of the status though. You'll need to ask @piscisaureus about that.

@TooTallNate
Copy link
Author

It would be great to get this one working you guys!

FTR I do a similar thing with node-ogg and node-vorbis (i.e. node-ogg re-exports all the symbols from libogg.a, and then vorbis.node dynamically links to ogg.node), but for node-ogg I was able to change the "type" to "shared_library" which made the right thing happen. It seems like this is more complicated since we're stuck with the "executable" gyp type for node.

@fresheneesz
Copy link

+1 I think there are a few modules that bit me related to this

@mscdex
Copy link

mscdex commented Jul 26, 2013

+9001

@ghost ghost assigned tjfontaine Jul 27, 2013
@isaacs
Copy link

isaacs commented Jul 27, 2013

@tjfontaine Assigning to you because this is kinda related to the stuff you've been working on. Feel free to reassign as you see fit.

I think the right approach is for Node to export a consistent API surface for "the stuff Node is using". So, node/ssl.h would load the OpenSSL interfaces that this version of node is using, for example.

@jasnell
Copy link
Member

jasnell commented May 18, 2015

@joyent/node-coreteam @mscdex @orangemocha ... where are we at on this one?

@mscdex
Copy link

mscdex commented May 18, 2015

@jasnell I never worked on it, so I really have no idea.

@LinusU
Copy link

LinusU commented Jul 17, 2015

@tjfontaine Do you have any more updated on this? I would love to get this working :)

If there is anything I can do to help I will gladly do so. Unfortunately, I have very limited experience with gyp :(

@LinusU
Copy link

LinusU commented Jul 17, 2015

Ehh, I in no way meant to unassign you from the bug. I don't even have sufficient privileges in this repo to do so. Seems like a bug in github...

@johnnyman727
Copy link

I'm also happy to help out. Would need some guidance though.

@jasnell
Copy link
Member

jasnell commented Aug 26, 2015

@TooTallNate ... any further thoughts on this one?

@TooTallNate
Copy link
Author

I still consider it a desirable feature. It would be nice if all the bundled libraries could be linked to by native addons.

But I don't think gyp has the necessary skill for it still, as @bnoordhuis was mentioning above, so not sure what we want to do with this issue.

@jasnell
Copy link
Member

jasnell commented Aug 29, 2015

@TooTallNate ... given that, I'm going to mark this as a feature-request and defer-to-converged. I'll keep the issue open to future tracking, but it's something that likely won't happen in v0.10 or v0.12 ever unfortunately.

@vladimir-shcherbakov
Copy link

@TooTallNate, do you have an example/sample of an add-on that can use functions from node's bundled openssl on Linux but not on Windows - I'd like to have a repro to experiment with.

@TooTallNate
Copy link
Author

@vladimir-shcherbakov Sure, this "works" on OS X and Linux, since SSLv23_method() is a function that is used in the Node.js codebase, and thus gets included in the resulting binary:

https://github.com/TooTallNate/node-openssl-addon-example/blob/master/binding.cc

Windows does not currently work with that ^

Fishrock123 pushed a commit to nodejs/node that referenced this issue Jul 6, 2016
Export symbols from the bundled openssl for add-ons to link against.

Fixes: nodejs/node-v0.x-archive#4051
PR-URL: #6274
Reviewed-By: James M Snell <jasnell@gmail.com>
bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Oct 18, 2016
Export symbols from the bundled openssl for add-ons to link against.

Fixes: nodejs/node-v0.x-archive#4051
PR-URL: nodejs#6274
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit to nodejs/node that referenced this issue Nov 14, 2016
Export symbols from the bundled openssl for add-ons to link against.

Fixes: nodejs/node-v0.x-archive#4051
PR-URL: #6274
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests