Skip to content

Conversation

@danbev
Copy link
Contributor

@danbev danbev commented May 17, 2022

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.

Refs: #40366

@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. labels May 17, 2022
@nodejs-github-bot
Copy link
Collaborator

@mhdawson
Copy link
Member

@danbev this will be breaking for anybody currently depending on the default config right?

@danbev
Copy link
Contributor Author

danbev commented May 17, 2022

this will be breaking for anybody currently depending on the default config right?

Yeah, this will probably break for them and they would have to copy, or rename, the default section/appname from openssl_conf to nodejs_conf.

@mscdex mscdex added the semver-major PRs that contain breaking changes and should be released in the next major version. label May 17, 2022
Copy link
Member

@BethGriggs BethGriggs left a comment

Choose a reason for hiding this comment

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

Could/should we also add a flag to allow the old behaviour of loading from the shared section of the OpenSSL config? (maybe --openssl-shared-config or something more suitably named?)

@danbev
Copy link
Contributor Author

danbev commented May 18, 2022

Could/should we also add a flag to allow the old behaviour of loading from the shared section of the OpenSSL config? (maybe --openssl-shared-config or something more suitably named?)

82c3f44 contains your suggestion (at least my interpretation :) for adding such a flag.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@BethGriggs BethGriggs added the notable-change PRs with changes that should be highlighted in changelogs. label May 23, 2022
@nodejs-github-bot
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants