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

crypto: add OP flag constants added in OpenSSL v1.1.1 #33929

Closed
wants to merge 2 commits into from

Conversation

mkrawczuk
Copy link
Contributor

  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows [commit guidelines]

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 17, 2020
@mkrawczuk
Copy link
Contributor Author

Full OpenSSL option flag list is available here:
https://wiki.openssl.org/index.php/List_of_SSL_OP_Flags

Also I have noticed that several OpenSSL constants here are missing description:
https://nodejs.org/api/crypto.html#crypto_openssl_options

I think it would be a good idea to add it, or to put a reference to the SSL wiki where the descriptions can be found, or do both. Let me know what you think.

doc/api/crypto.md Outdated Show resolved Hide resolved
doc/api/crypto.md Outdated Show resolved Hide resolved
@addaleax
Copy link
Member

@nodejs/crypto

@addaleax addaleax added the crypto Issues and PRs related to the crypto subsystem. label Jun 19, 2020
@nodejs-github-bot
Copy link
Collaborator

@mildsunrise
Copy link
Member

I think it would be a good idea to add it, or to put a reference to the SSL wiki where the descriptions can be found, or do both. Let me know what you think.

+1 on doing both

mkrawczuk added a commit to mkrawczuk/node that referenced this pull request Jun 25, 2020
Some of the SSL_OP_* constants are missing description in the documentation.
Instead of rewriting the description from OpenSSL's wiki, I have decided to put
a link to a detailed list in the 'OpenSSL Options' section.

I see no point of doing both - adding a reference to the wiki and adding
constant descriptions - but I might do if presented with convincing arguments.

This is a follow-up to nodejs#33929.
jasnell pushed a commit that referenced this pull request Jul 3, 2020
Some of the SSL_OP_* constants are missing description in the
documentation. Instead of rewriting the description from OpenSSL's
wiki, I have decided to put a link to a detailed list in the
'OpenSSL Options' section.

I see no point of doing both - adding a reference to the wiki and
adding constant descriptions - but I might do if presented with
convincing arguments.

This is a follow-up to #33929.

PR-URL: #34050
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2020
Some of the SSL_OP_* constants are missing description in the
documentation. Instead of rewriting the description from OpenSSL's
wiki, I have decided to put a link to a detailed list in the
'OpenSSL Options' section.

I see no point of doing both - adding a reference to the wiki and
adding constant descriptions - but I might do if presented with
convincing arguments.

This is a follow-up to #33929.

PR-URL: #34050
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
Some of the SSL_OP_* constants are missing description in the
documentation. Instead of rewriting the description from OpenSSL's
wiki, I have decided to put a link to a detailed list in the
'OpenSSL Options' section.

I see no point of doing both - adding a reference to the wiki and
adding constant descriptions - but I might do if presented with
convincing arguments.

This is a follow-up to #33929.

PR-URL: #34050
Reviewed-By: James M Snell <jasnell@gmail.com>
@mkrawczuk
Copy link
Contributor Author

This PR seems to have gotten a little stuck. Is there anything that should be done to move further?
cc @nodejs/crypto, @jasnell @mildsunrise @tniessen

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@mildsunrise
Copy link
Member

Landed in 3306070

@mildsunrise mildsunrise closed this Aug 7, 2020
mildsunrise pushed a commit that referenced this pull request Aug 7, 2020
PR-URL: #33929
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alba Mendez <me@alba.sh>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
addaleax pushed a commit that referenced this pull request Aug 8, 2020
PR-URL: #33929
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alba Mendez <me@alba.sh>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@codebytere codebytere mentioned this pull request Aug 10, 2020
codebytere pushed a commit that referenced this pull request Aug 11, 2020
PR-URL: #33929
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alba Mendez <me@alba.sh>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
Some of the SSL_OP_* constants are missing description in the
documentation. Instead of rewriting the description from OpenSSL's
wiki, I have decided to put a link to a detailed list in the
'OpenSSL Options' section.

I see no point of doing both - adding a reference to the wiki and
adding constant descriptions - but I might do if presented with
convincing arguments.

This is a follow-up to #33929.

PR-URL: #34050
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
PR-URL: #33929
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alba Mendez <me@alba.sh>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
PR-URL: #33929
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alba Mendez <me@alba.sh>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@codebytere codebytere mentioned this pull request Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants