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.

Sign up for GitHub

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

Jump to bottom

[v18.x] src,deps,build,test: add OpenSSL config appname #43539

Closed
wants to merge 2 commits into from
Closed

[v18.x] src,deps,build,test: add OpenSSL config appname #43539

wants to merge 2 commits into from

Conversation

RafaelGSS
Copy link
Member

Backport #43124

danbev added 2 commits June 22, 2022 15:44
@danbev @RafaelGSS
src,deps,build,test: add OpenSSL config appname
c2db61a 
This commit adds the setting of an appname (configuration section
name), 'nodejs_conf', to be used when reading OpenSSL configuration
files.

The motivation for this is that currently the default OpenSSL
configuration, 'openssl_conf', element will be used which may be
undesirable as it might configure OpenSSL in unwanted ways. With this
commit it is still possible to use a default openssl.cnf file but the
only section that Node.js will read from is a section named
'nodejs_conf'.

PR-URL: nodejs#43124
Refs: nodejs#40366
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
@danbev @RafaelGSS
src,doc,test: add --openssl-shared-config option
3b93af7 
This commit adds a new command line option named
'--openssl-shared-config' intended to allow reverting to the old OpenSSL
configuration behavior where Node.js would use the configuration section
name (called appname in OpenSSL) 'openssl_conf' which could potentially
be used my other applications..

PR-URL: nodejs#43124
Refs: nodejs#40366
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
@RafaelGSS RafaelGSS requested review from danbev and BethGriggs June 22, 2022 18:45
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch. labels Jun 22, 2022
BethGriggs
BethGriggs approved these changes Jun 22, 2022
View reviewed changes
Copy link
Member

@BethGriggs BethGriggs left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! For anyone else reviewing, I believe this is identical to the original PR - but, we felt for clarity/a statement of intent we should open this as a backport for Node.js 18 as it was marked semver major.

@RafaelGSS
Copy link
Member Author

Landed in 3f0c3e1 and 9578158

@RafaelGSS RafaelGSS closed this Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BethGriggs BethGriggs BethGriggs approved these changes

@danbev danbev danbev approved these changes

Assignees
No one assigned
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@RafaelGSS @nodejs-github-bot @danbev @BethGriggs