Skip to content

crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL - #52217

Merged
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
codebytere:openssl-init-boringssl
Apr 11, 2024
Merged

crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL#52217
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
codebytere:openssl-init-boringssl

Conversation

@codebytere

Copy link
Copy Markdown
Member

As in title. It's possible for NODE_EXTRA_CA_CERTS to work in a BoringSSL context, so there's no need to include that in the guard. This allows Electron users to leverage it.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Mar 26, 2024

@tniessentniessen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This increases the number of #endif directives without changing the number of #if directives. You'll need to split the existing #if ... && ... into two separate directives in order to use separate #endif directives for the two conditions.

@codebytere

Copy link
Copy Markdown
MemberAuthor

@tniessen oops, forgot to push that 😅 fixed.

@codebytere
codebytereforce-pushed the openssl-init-boringssl branch from 3b0f72c to 248c632CompareMarch 26, 2024 11:04

@tniessentniessen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On a side note, the commit message does not adhere to the guidelines, I'd suggest crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL or something along those lines.

Comment threadsrc/node.cc Outdated
@codebytere
codebytereforce-pushed the openssl-init-boringssl branch from 248c632 to 452d594CompareMarch 26, 2024 11:28
@tniessentniessen changed the title crypto: BoringSSL can work with NODE_EXTRA_CA_CERTScrypto: enable NODE_EXTRA_CA_CERTS with BoringSSLMar 26, 2024
@codebytere
codebytereforce-pushed the openssl-init-boringssl branch from 452d594 to 8904a61CompareMarch 26, 2024 12:02
@tniessentniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 26, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 26, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Comment threadsrc/node.cc Outdated
Co-authored-by: Yagiz Nizipli <yagiz@nizipli.com>
Comment threadsrc/node.cc Outdated
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@richardlaurichardlau added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Apr 11, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 11, 2024
@nodejs-github-bot
nodejs-github-bot merged commit 28d68f3 into nodejs:mainApr 11, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 28d68f3

aduh95 pushed a commit that referenced this pull request Apr 29, 2024
PR-URL: #52217
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@aduh95

Copy link
Copy Markdown
Contributor

Should this be labelled semver-minor?

marco-ippolito pushed a commit that referenced this pull request May 2, 2024
PR-URL: #52217
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@marco-ippolitomarco-ippolito mentioned this pull request May 2, 2024
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
PR-URL: #52217
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request May 8, 2024
jkleinsc pushed a commit to electron/electron that referenced this pull request May 13, 2024
* chore: bump node in DEPS to v20.13.0
* crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL
nodejs/node#52217
* test: skip test for dynamically linked OpenSSL
nodejs/node#52542
* lib, url: add a `windows` option to path parsing
nodejs/node#52509
* src: use dedicated routine to compile function for builtin CJS loader
nodejs/node#52016
* test: mark test as flaky
nodejs/node#52671
* build,tools: add test-ubsan ci
nodejs/node#46297
* src: preload function for Environment
nodejs/node#51539
* chore: fixup patch indices
* deps: update c-ares to 1.28.1
nodejs/node#52285
* chore: handle updated filenames
- nodejs/node#51999
- nodejs/node#51927
* chore: bump node in DEPS to v20.13.1
* events: extract addAbortListener for safe internal use
nodejs/node#52081
* module: print location of unsettled top-level await in entry points
nodejs/node#51999
* fs: add stacktrace to fs/promises
nodejs/node#49849
* chore: update patches
---------
Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
@codebytere
codebytere deleted the openssl-init-boringssl branch May 31, 2024 13:32
codebytere added a commit to electron/electron that referenced this pull request May 31, 2024
codebytere added a commit to electron/electron that referenced this pull request May 31, 2024
codebytere added a commit to electron/electron that referenced this pull request May 31, 2024
* chore: bump node in DEPS to v20.13.1
* chore: bump node in DEPS to v20.14.0
* crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL
nodejs/node#52217
* test: skip test for dynamically linked OpenSSL
nodejs/node#52542
* lib, url: add a `windows` option to path parsing
nodejs/node#52509
* src: use dedicated routine to compile function for builtin CJS loader
nodejs/node#52016
* test: mark test as flaky
nodejs/node#52671
* build,tools: add test-ubsan ci
nodejs/node#46297
* src: preload function for Environment
nodejs/node#51539
* chore: fixup patch indices
* deps: update c-ares to 1.28.1
nodejs/node#52285
* chore: handle updated filenames
* events: extract addAbortListener for safe internal use
nodejs/node#52081
* module: print location of unsettled top-level await in entry points
nodejs/node#51999
* fs: add stacktrace to fs/promises
nodejs/node#49849
* chore: fixup patch indices
---------
Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Jun 1, 2024
* chore: bump node in DEPS to v20.13.1
* chore: bump node in DEPS to v20.14.0
* chore: update build_add_gn_build_files.patch
* chore: update patches
* chore: update patches
* build: encode non-ASCII Latin1 characters as one byte in JS2C
nodejs/node#51605
* crypto: use EVP_MD_fetch and cache EVP_MD for hashes
nodejs/node#51034
* chore: update filenames.json
* chore: update patches
* src: support configurable snapshot
nodejs/node#50453
* test: remove test-domain-error-types flaky designation
nodejs/node#51717
* src: avoid draining platform tasks at FreeEnvironment
nodejs/node#51290
* chore: fix accidentally deleted v8 dep
* lib: define FormData and fetch etc. in the built-in snapshot
nodejs/node#51598
* chore: remove stray log
* crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL
nodejs/node#52217
* test: skip test for dynamically linked OpenSSL
nodejs/node#52542
* lib, url: add a `windows` option to path parsing
nodejs/node#52509
* src: use dedicated routine to compile function for builtin CJS loader
nodejs/node#52016
* test: mark test as flaky
nodejs/node#52671
* build,tools: add test-ubsan ci
nodejs/node#46297
* src: preload function for Environment
nodejs/node#51539
* deps: update c-ares to 1.28.1
nodejs/node#52285
* chore: fixup
* events: extract addAbortListener for safe internal use
nodejs/node#52081
* module: print location of unsettled top-level await in entry points
nodejs/node#51999
* fs: add stacktrace to fs/promises
nodejs/node#49849
* chore: fixup indices
---------
Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Cheng <zcbenz@gmail.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Sign up for freeto 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++.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@codebytere@nodejs-github-bot@aduh95@addaleax@lpinca@anonrig@tniessen@richardlau