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

build: export more openssl symbols #7576

Closed
wants to merge 1 commit into from
Closed

build: export more openssl symbols #7576

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jul 7, 2016

This makes the generated openssl.def include these additional symbols:

SSL_CTX_use_PrivateKey_file <- I use this one
SSL_CTX_use_RSAPrivateKey_file
SSL_CTX_use_certificate_chain_file <- I use this one
SSL_CTX_use_certificate_file
SSL_CTX_use_serverinfo_file
SSL_add_dir_cert_subjects_to_stack
SSL_add_file_cert_subjects_to_stack
SSL_load_client_CA_file
SSL_set_fd <- I use this one
SSL_set_rfd
SSL_set_wfd
SSL_use_PrivateKey_file
SSL_use_RSAPrivateKey_file
SSL_use_certificate_file
X509_STORE_load_locations
X509_STORE_set_default_paths
X509_load_cert_crl_file
X509_load_cert_file
X509_load_crl_file

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
Affected core subsystem(s)

Windows build

Description of change

Export more OpenSSL symbols

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Jul 7, 2016
@mscdex mscdex added the openssl Issues and PRs related to the OpenSSL dependency. label Jul 7, 2016
@mscdex
Copy link
Contributor

mscdex commented Jul 7, 2016

/cc @bnoordhuis

@@ -551,7 +551,8 @@
'mkssldef_flags': [
# Categories to export.
'-CAES,BF,BIO,DES,DH,DSA,EC,ECDH,ECDSA,ENGINE,EVP,HMAC,MD4,MD5,'
'NEXTPROTONEG,PSK,RC2,RC4,RSA,SHA,SHA0,SHA1,SHA256,SHA512,TLSEXT',
'NEXTPROTONEG,PSK,RC2,RC4,RSA,SHA,SHA0,SHA1,SHA256,SHA512,TLSEXT'
'STDIO,SOCK',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you keep the list in alphabetical order?

@bnoordhuis
Copy link
Member

In principle LGTM with a style nit. The commit log should follow the project standard, though. It's described in CONTRIBUTING.md.

@ghost ghost changed the title Export OpenSSL STDIO and SOCK symbol categories build: export more openssl symbols Jul 10, 2016
@ghost
Copy link
Author

ghost commented Jul 10, 2016

This is now on top of the history and I think it follows the formatting rules.

@ghost ghost closed this Jul 11, 2016
@ghost ghost reopened this Jul 11, 2016
@ghost
Copy link
Author

ghost commented Jul 11, 2016

Rebased again

@bnoordhuis
Copy link
Member

@ghost
Copy link
Author

ghost commented Jul 12, 2016

What happens now? Should I rebase and add Reviewed By: and run the CI again?

This exports even more openssl symbols when building
on Windows. SSL_set_fd is one example of added symbol.

PR-URL: #7576
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@silverwind
Copy link
Contributor

LGTM, but I'm a bit confused why for example X509 should be available through SOCK,STDIO when a quick grep through openssl sources for OPENSSL_NO_ shows a separate X509 category:

AES,BF,BIO,BUF_FREELISTS,BUFFER,CAMELLIA,CAST,CMS,COMP,DEPRECATED,DES,DGRAM,DH,
DSA,EC,EC2M,EC_NISTP_64_GCC_128,ECDH,ECDSA,ENGINE,ERR,EVP,FP_API,GMP,GOST,
HASH_COMP,HEARTBEATS,HMAC,IDEA,JPAKE,KRB5,LHASH,MD2,MD4,MD5,MDC2,NEXTPROTONEG,
OCSP,PSK,RC2,RC4,RC5,RFC3779,RIPEMD,RSA,SCTP,SEED,SHA,SHA0,SHA1,SHA256,SHA512,
SOCK,SRP,SRTP,SSL2_METHOD,SSL3_METHOD,SSL_INTERN,SSL_TRACE,STACK,STATIC_ENGINE,
STDIO,STORE,TLSEXT,UNIT_TEST,WHIRLPOOL,X509

@ghost
Copy link
Author

ghost commented Jul 12, 2016

The STDIO category spans over multiple categories:

X509_print_ex_fp 1093 1_1_0 EXIST::FUNCTION:STDIO
PEM_write 1128 1_1_0 EXIST::FUNCTION:STDIO

I guess STDIO category holds things that modifies files in some way (SSL_set_fd is also in the STDIO category).

silverwind pushed a commit that referenced this pull request Jul 12, 2016
This exports even more openssl symbols when building
on Windows. SSL_set_fd is one example of added symbol.

PR-URL: #7576
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Roman Reiss <me@silverwind.io>
@silverwind
Copy link
Contributor

silverwind commented Jul 12, 2016

Thanks for the explanation, landed in b8b8c36.

I appended "on Windows" to the commit message as from my understanding of https://github.com/nodejs/node/pull/6274/files, this gyp section only applies on Windows.

@silverwind silverwind closed this Jul 12, 2016
@ghost
Copy link
Author

ghost commented Jul 12, 2016

Nice 👍

@ghost
Copy link
Author

ghost commented Jul 12, 2016

Btw, will this be included in the next 6.x version or should this commit be tagged in some way for that? I would like to have it in next version.

@silverwind
Copy link
Contributor

silverwind commented Jul 12, 2016

It will be in the next 6.x for sure (unless it gets reverted). Only semver-tagged changes would be delayed until the next minor/major.

@bnoordhuis bnoordhuis added the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 13, 2016
@bnoordhuis
Copy link
Member

I added the semver-minor label because it's an expansion of the API surface.

@cjihrig cjihrig mentioned this pull request Aug 8, 2016
cjihrig pushed a commit that referenced this pull request Aug 10, 2016
This exports even more openssl symbols when building
on Windows. SSL_set_fd is one example of added symbol.

PR-URL: #7576
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Roman Reiss <me@silverwind.io>
@cjihrig cjihrig mentioned this pull request Aug 11, 2016
cjihrig added a commit that referenced this pull request Aug 11, 2016
Notable changes:

* build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) #7983 and #7576
* child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) #7811 and #7838
* fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) #7942
* repl: The REPL now supports editor mode. (Prince J Wesley) #7275
* util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) #8013

Refs: #8020
PR-URL: #8070
cjihrig added a commit that referenced this pull request Aug 11, 2016
Notable changes:

* build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) #7983 and #7576
* child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) #7811 and #7838
* child_process: argv[0] can now be set to arbitrary values in spawned processes. (Pat Pannuto) #7696
* fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) #7942
* repl: The REPL now supports editor mode. (Prince J Wesley) #7275
* util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) #8013

Refs: #8020
PR-URL: #8070
cjihrig added a commit that referenced this pull request Aug 15, 2016
Notable changes:

* build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) #7983 and #7576
* child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) #7811 and #7838
* child_process: argv[0] can now be set to arbitrary values in spawned processes. (Pat Pannuto) #7696
* fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) #7942
* repl: The REPL now supports editor mode. (Prince J Wesley) #7275
* util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) #8013

Refs: #8020
PR-URL: #8070
cjihrig added a commit to cjihrig/node that referenced this pull request Aug 16, 2016
Notable changes:

* build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) nodejs#7983 and nodejs#7576
* child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) nodejs#7811 and nodejs#7838
* child_process: argv[0] can now be set to arbitrary values in spawned processes. (Pat Pannuto) nodejs#7696
* fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) nodejs#7942
* repl: The REPL now supports editor mode. (Prince J Wesley) nodejs#7275
* util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) nodejs#8013

Refs: nodejs#8020
PR-URL: nodejs#8070
bnoordhuis pushed a commit to bnoordhuis/io.js that referenced this pull request Oct 18, 2016
This exports even more openssl symbols when building
on Windows. SSL_set_fd is one example of added symbol.

PR-URL: nodejs#7576
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Roman Reiss <me@silverwind.io>
@MylesBorins MylesBorins added this to the v4.7.0 milestone Oct 24, 2016
MylesBorins pushed a commit that referenced this pull request Nov 14, 2016
This exports even more openssl symbols when building
on Windows. SSL_set_fd is one example of added symbol.

PR-URL: #7576
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Roman Reiss <me@silverwind.io>
@MylesBorins MylesBorins removed this from the 4.7.0 milestone Nov 14, 2016
@MylesBorins MylesBorins mentioned this pull request Nov 22, 2016
MylesBorins pushed a commit that referenced this pull request Dec 6, 2016
This LTS release comes with 108 commits. This includes 30 which are doc
related, 28 which are test related, 16 which are build / tool related,
and 4 commits which are updates to dependencies.

Notable Changes:

The SEMVER-MINOR changes include:

* build:
  - export openssl symbols on Windows making it possible to build
    addons linked against the bundled version of openssl (Alex Hultman)
    #7576
* debugger:
  - make listen address configurable in the debugger server
    (Ben Noordhuis) #3316
* dgram:
  - generalized send queue to handle close fixing a potential throw
    when dgram socket is closed in the listening event handler.
    (Matteo Collina) #7066
* http:
  - Introduce the 451 status code "Unavailable For Legal Reasons"
    (Max Barinov) #4377
* tls:
  - introduce `secureContext` for `tls.connect` which is useful for
    caching client certificates, key, and CA certificates.
    (Fedor Indutny) #4246

Notable SEMVER-PATCH changes include:

* build:
  - introduce the configure --shared option for embedders (sxa555)
    #6994
* gtest:
  - the test reporter now outputs tap comments as yamlish
    (Johan Bergström) #9262
* src:
  - node no longer aborts when c-ares initialization fails
    (Ben Noordhuis) #8710
* tls:
  - fix memory leak when writing data to TLSWrap instance during
    handshake (Fedor Indutny)
    #9586

PR-URL: #9736
MylesBorins pushed a commit that referenced this pull request Dec 6, 2016
This LTS release comes with 108 commits. This includes 30 which are doc
related, 28 which are test related, 16 which are build / tool related,
and 4 commits which are updates to dependencies.

Notable Changes:

The SEMVER-MINOR changes include:

* build:
  - export openssl symbols on Windows making it possible to build
    addons linked against the bundled version of openssl (Alex Hultman)
    #7576
* debugger:
  - make listen address configurable in the debugger server
    (Ben Noordhuis) #3316
* dgram:
  - generalized send queue to handle close fixing a potential throw
    when dgram socket is closed in the listening event handler.
    (Matteo Collina) #7066
* http:
  - Introduce the 451 status code "Unavailable For Legal Reasons"
    (Max Barinov) #4377
* tls:
  - introduce `secureContext` for `tls.connect` which is useful for
    caching client certificates, key, and CA certificates.
    (Fedor Indutny) #4246

Notable SEMVER-PATCH changes include:

* build:
  - introduce the configure --shared option for embedders (sxa555)
    #6994
* gtest:
  - the test reporter now outputs tap comments as yamlish
    (Johan Bergström) #9262
* src:
  - node no longer aborts when c-ares initialization fails
    (Ben Noordhuis) #8710
* tls:
  - fix memory leak when writing data to TLSWrap instance during
    handshake (Fedor Indutny)
    #9586

PR-URL: #9736
MylesBorins pushed a commit that referenced this pull request Dec 6, 2016
This LTS release comes with 108 commits. This includes 30 which are doc
related, 28 which are test related, 16 which are build / tool related,
and 4 commits which are updates to dependencies.

Notable Changes:

The SEMVER-MINOR changes include:

* build:
  - export openssl symbols on Windows making it possible to build
    addons linked against the bundled version of openssl (Alex Hultman)
    #7576
* debugger:
  - make listen address configurable in the debugger server
    (Ben Noordhuis) #3316
* dgram:
  - generalized send queue to handle close fixing a potential throw
    when dgram socket is closed in the listening event handler.
    (Matteo Collina) #7066
* http:
  - Introduce the 451 status code "Unavailable For Legal Reasons"
    (Max Barinov) #4377
* tls:
  - introduce `secureContext` for `tls.connect` which is useful for
    caching client certificates, key, and CA certificates.
    (Fedor Indutny) #4246

Notable SEMVER-PATCH changes include:

* build:
  - introduce the configure --shared option for embedders (sxa555)
    #6994
* gtest:
  - the test reporter now outputs tap comments as yamlish
    (Johan Bergström) #9262
* src:
  - node no longer aborts when c-ares initialization fails
    (Ben Noordhuis) #8710
* tls:
  - fix memory leak when writing data to TLSWrap instance during
    handshake (Fedor Indutny)
    #9586

PR-URL: #9736
imyller added a commit to imyller/meta-nodejs that referenced this pull request Dec 7, 2016
    This LTS release comes with 108 commits. This includes 30 which are doc
    related, 28 which are test related, 16 which are build / tool related,
    and 4 commits which are updates to dependencies.

    Notable Changes:

    The SEMVER-MINOR changes include:

    * build:
      - export openssl symbols on Windows making it possible to build
        addons linked against the bundled version of openssl (Alex Hultman)
        nodejs/node#7576
    * debugger:
      - make listen address configurable in the debugger server
        (Ben Noordhuis) nodejs/node#3316
    * dgram:
      - generalized send queue to handle close fixing a potential throw
        when dgram socket is closed in the listening event handler.
        (Matteo Collina) nodejs/node#7066
    * http:
      - Introduce the 451 status code "Unavailable For Legal Reasons"
        (Max Barinov) nodejs/node#4377
    * tls:
      - introduce `secureContext` for `tls.connect` which is useful for
        caching client certificates, key, and CA certificates.
        (Fedor Indutny) nodejs/node#4246

    Notable SEMVER-PATCH changes include:

    * build:
      - introduce the configure --shared option for embedders (sxa555)
        nodejs/node#6994
    * gtest:
      - the test reporter now outputs tap comments as yamlish
        (Johan Bergstrom) nodejs/node#9262
    * src:
      - node no longer aborts when c-ares initialization fails
        (Ben Noordhuis) nodejs/node#8710
    * tls:
      - fix memory leak when writing data to TLSWrap instance during
        handshake (Fedor Indutny)
        nodejs/node#9586

    PR-URL: nodejs/node#9736

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
imyller added a commit to imyller/meta-nodejs that referenced this pull request Dec 7, 2016
    This LTS release comes with 108 commits. This includes 30 which are doc
    related, 28 which are test related, 16 which are build / tool related,
    and 4 commits which are updates to dependencies.

    Notable Changes:

    The SEMVER-MINOR changes include:

    * build:
      - export openssl symbols on Windows making it possible to build
        addons linked against the bundled version of openssl (Alex Hultman)
        nodejs/node#7576
    * debugger:
      - make listen address configurable in the debugger server
        (Ben Noordhuis) nodejs/node#3316
    * dgram:
      - generalized send queue to handle close fixing a potential throw
        when dgram socket is closed in the listening event handler.
        (Matteo Collina) nodejs/node#7066
    * http:
      - Introduce the 451 status code "Unavailable For Legal Reasons"
        (Max Barinov) nodejs/node#4377
    * tls:
      - introduce `secureContext` for `tls.connect` which is useful for
        caching client certificates, key, and CA certificates.
        (Fedor Indutny) nodejs/node#4246

    Notable SEMVER-PATCH changes include:

    * build:
      - introduce the configure --shared option for embedders (sxa555)
        nodejs/node#6994
    * gtest:
      - the test reporter now outputs tap comments as yamlish
        (Johan Bergstrom) nodejs/node#9262
    * src:
      - node no longer aborts when c-ares initialization fails
        (Ben Noordhuis) nodejs/node#8710
    * tls:
      - fix memory leak when writing data to TLSWrap instance during
        handshake (Fedor Indutny)
        nodejs/node#9586

    PR-URL: nodejs/node#9736

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. openssl Issues and PRs related to the OpenSSL dependency. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants