Skip to content

test: send SNI in CA certificate test - #66835

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tamird:test-tls-add-ca-sni
Oct 22, 2026
Merged

nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tamird:test-tls-add-ca-sni

Conversation

@tamird

@tamird tamird commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Set servername when connecting to www.nodejs.org so the server selects its certificate for that hostname. Without SNI, it returns a certificate that does not cover www.nodejs.org, and the test fails before exercising CA handling. The nightly internet test is failing with this error 1.

Reproduce the certificate mismatch without Node.js:

    openssl s_client -connect www.nodejs.org:443 -noservername \
      -verify_hostname www.nodejs.org -verify_return_error </dev/null

This reports "verify error:num=62:Hostname mismatch". Replacing -noservername with -servername www.nodejs.org succeeds.

Set servername when connecting to www.nodejs.org so the server selects
its certificate for that hostname. Without SNI, it returns a certificate
that does not cover www.nodejs.org, and the test fails before exercising
CA handling. The nightly internet test is failing with this error [1].

Reproduce the certificate mismatch without Node.js:

    openssl s_client -connect www.nodejs.org:443 -noservername \
      -verify_hostname www.nodejs.org -verify_return_error </dev/null

This reports "verify error:num=62:Hostname mismatch". Replacing
-noservername with -servername www.nodejs.org succeeds.

[1]: https://github.com/nodejs/node/actions/runs/39084453901

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
Assisted-by: Codex
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure. labels Oct 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

tamird added a commit to tamird/gvisor that referenced this pull request Oct 18, 2026
Link the retained runtime-image patch to nodejs/node#66835 so its upstream
status can be tracked. The patch hunk and image recipe are unchanged.
@panva panva added fast-track PRs proposed for a shorter-than-standard waiting period before landing. author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Oct 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Fast-track has been requested by @panva. Please 👍 to approve.

@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 22, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@richardlau

Copy link
Copy Markdown
Member

FWIW this was probably caused by new certificates which have replaced the old certificates that were due to expire tomorrow.

cc @ryanaslett as an FYI. I'd prefer to adapt the test than broaden the certificate.

@aduh95 aduh95 removed the fast-track PRs proposed for a shorter-than-standard waiting period before landing. label Oct 22, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@panva panva added the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 22, 2026
@codecov

codecov Bot commented Oct 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.86%. Comparing base (e2d5522) to head (ca90389).
⚠️ Report is 63 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66835      +/-   ##
==========================================
- Coverage   90.50%   89.86%   -0.64%     
==========================================
  Files         797      797              
  Lines      277793   277793              
  Branches    53366    53234     -132     
==========================================
- Hits       251405   249645    -1760     
- Misses      16801    18438    +1637     
- Partials     9587     9710     +123     

see 129 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@panva

panva commented Oct 22, 2026

Copy link
Copy Markdown
Member

Thank you for noticing and fixing this @tamird, also congratulations on your first contribution. The PR is now in the commit queue and will land on main shortly.

@tamird

tamird commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks all! Credit goes to the LLMs :)

@nodejs-github-bot
nodejs-github-bot merged commit 727e0ca into nodejs:main Oct 22, 2026
68 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 727e0ca

@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 22, 2026
@tamird
tamird deleted the test-tls-add-ca-sni branch October 22, 2026 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants