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

Include OpenSSL headers with node distributions #9269

Closed
geek0x23 opened this issue Feb 23, 2015 · 10 comments
Closed

Include OpenSSL headers with node distributions #9269

geek0x23 opened this issue Feb 23, 2015 · 10 comments
Milestone

Comments

@geek0x23
Copy link

When node is installed (or a pre-built binary from nodejs.org is used), there are several header files that are packaged with Node which make native addon development possible. Amongst them are headers for v8, c-ares, libuv, etc. I've noticed that there's even an openssl subdirectory that contains a single header file: opensslconf.h.

Well it turns out that OSX ships with OpenSSL 0.9.8 by default, and when Node is compiled using its own bundled version of OpenSSL, the verison is much more recent and includes some features that I'd like to use, specifically.

So this feature request is relatively simple: When Node is built using it's bundled version of OpenSSL (no --shared-openssl), can the headers for the included version be shipped in that include/node/openssl subdirectory?

@tjfontaine
Copy link

A related issue is #5112

Keep in mind that in part because we don't have a well defined API/ABI it means that if you link against our compiled openssl you will be stuck using only the symbols we consume. So if there's a deficiency in our existing consumption of openssl, it's likely that just linking against ours won't be sufficient.

Some of this can be fixed with map files etc, but we have to decide what we want our public symbols for node to be. Or you can ship your own copy of openssl with your module, but you want to make sure it's scoped local to your binary addon such that our static openssl doesn't interfere with yours.

@geek0x23
Copy link
Author

For me this is perfectly acceptable since I don't actually link statically to OpenSSL. I always end up using whatever node bundles.

@misterdjules
Copy link

@SWAJ It seems to me #14089 fixes this issue, could you please confirm? Thanks!

@geek0x23
Copy link
Author

Confirmed. I was able to build 9d19dfb and the headers I'm looking for are now included. Thanks Julien! Feel free to close this one.

@misterdjules
Copy link

Thank you very much @SWAJ 👍 That fix and 4028669 will be included in the upcoming v0.10.39 and v0.12.5 releases. When that will be depends on our progress on fixing recent issues with OpenSSL and logjam. More information here: github.com//issues/25509.

Closing then!

@geek0x23
Copy link
Author

EDIT: sorry I didn't catch this sooner!

@misterdjules I think there may be some more headers missing, unfortunately. It seems the OpenSSL headers rely on other headers, which aren't present. A good example is openssl/evp.h:

In file included from ../src/crypto.c:4:
/usr/local/include/node/openssl/evp.h:1:10: fatal error: '../../crypto/evp/evp.h' file not found
#include "../../crypto/evp/evp.h"
         ^
1 error generated.

I'm not sure if this is something we can fix at the node layer or not. When performing a compilation of OpenSSL manually, and subsequently doing a make install, evp.h never includes relative path references like this to other headers. This makes me think the OpenSSL build process must generate the fully-constructed headers.

@misterdjules
Copy link

@SWAJ Thank you for the feedback 👍 Reopening and adding to the next milestone.

@misterdjules misterdjules reopened this Jun 24, 2015
@misterdjules misterdjules added this to the 0.10.41 milestone Jun 24, 2015
@shigeki
Copy link

shigeki commented Jun 25, 2015

This is to be fixed in nodejs/node#2016 . I will submit a new PR after that.

shigeki pushed a commit to shigeki/node-v0.x-archive that referenced this issue Jun 26, 2015
On upgrading openssl, all symlinks in pulic header files are replaced
with nested include files. The issue was raised that installing them
leads to lost its references to real header files.
To avoid this, all public header files are copied into the
`deps/openssl/openssl/include/openssl/` directory.
As a result, we have duplicated header files under
`deps/openssl/openssl/` but copied files are refereed in build as
specified to include path in openssl.gyp.

Fixes: nodejs#9269
@misterdjules
Copy link

@SWAJ This should be fixed by 8277822. Could you please give it a try and let us know if that fixes your issue?

Thank you 👍

@misterdjules misterdjules modified the milestones: 0.10.40, 0.10.41 Jul 14, 2015
@misterdjules
Copy link

This issue had been moved to 0.10.41, but it was fixed by 8277822 in v0.10.40. So moving it back again to the 0.10.40 milestone.

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

4 participants