Skip to content

tls: match IPv6 hosts against IP-Address SANs - #64145

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
JumpLink:tls-ipv6-ip-san
Jul 8, 2026
Merged

tls: match IPv6 hosts against IP-Address SANs#64145
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
JumpLink:tls-ipv6-ip-san

Conversation

@JumpLink

Copy link
Copy Markdown
Contributor

tls.checkServerIdentity() stopped matching an IPv6 host against a matching IP Address SAN, returning ERR_TLS_CERT_ALTNAME_INVALID (Cert does not contain a DNS name) where it used to return undefined.

The hostname is now run through domainToASCII() before the net.isIP() gate, and domainToASCII('::1') === '' (an IPv6 literal is not a domain), so net.isIP('') is 0, the IP-SAN branch is skipped, and (with no DNS SAN / CN) it falls through to the no-identifier reason. IPv4 is unaffected because dotted-decimal survives domainToASCII().

consttls=require('node:tls');tls.checkServerIdentity('::1',{subject: {},subjectaltname: 'IP Address:::1'});// v24.16.0: undefined (matched) v24.17.0+/v26.x: ERR_TLS_CERT_ALTNAME_INVALID

This matches IP hosts against the original hostname instead of the IDNA-normalized one. net.isIP() rejects non-ASCII input, so there is no IDNA confusion to guard against for an IP literal; the normalized form is still used for the DNS-name path. Adds IPv6 IP-SAN coverage (match, canonical-form match, non-match) to test-tls-check-server-identity.

Fixes: #64144

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/net

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. tls Issues and PRs related to the tls subsystem. labels Jun 26, 2026
@JumpLink

Copy link
Copy Markdown
ContributorAuthor

@pimterry sorry for the extra ping! The lint-commit-message check was failing because the first commit was missing the DCO Signed-off-by trailer, so I force-pushed to add it. That reset the GitHub Actions approval, and the workflow runs are now sitting at action_required. Would you mind approving the workflow runs again so the full CI can run? Thanks a lot for the earlier approval and for the review!

@pimterry

Copy link
Copy Markdown
Member

Can you rebase this again please @JumpLink? Sorry, there was an issue on main due to two conflicting PRs merging. It's resolved on main, but as you can see it breaks the tests here.

@JumpLink

JumpLink commented Jun 29, 2026

Copy link
Copy Markdown
ContributorAuthor

@pimterry Done, thanks for the heads-up about the conflicting PRs 👍

@pimterry

Copy link
Copy Markdown
Member

Sorry this stalled @JumpLink. The failure is spurious I think, it seems there was another test issue (sigh) with coverage-windows test and GH runner updates. Can you rebase and force push again? The failure is fixed in f1e01e7 I think.

checkServerIdentity() stopped matching an IPv6 host against a matching
IP-Address SAN. The hostname is now run through domainToASCII() before
the net.isIP() gate, and domainToASCII('::1') === '' (an IPv6 literal is
not a domain), so net.isIP('') is 0, the IP-SAN branch is skipped, and
verification fails with "Cert does not contain a DNS name". IPv4 is
unaffected because dotted-decimal survives domainToASCII().
Match IP hosts against the original hostname instead of the IDNA-
normalized one. net.isIP() rejects non-ASCII input, so there is no IDNA
confusion to guard against for an IP literal; the normalized form is
still used for the DNS-name path.
Fixes: nodejs#64144
Signed-off-by: Pascal Garber <pascal@artandcode.studio>
@JumpLink

Copy link
Copy Markdown
ContributorAuthor

@pimterry Rebased onto latest main. This pulls in #64222 ("build: suppress clang errors building libffi on Windows"), which is what was turning the coverage-windows build red on the previous run — my earlier base predated that fix.

Since the force-push reset the workflow approval, could you kick the CI off again when you get a chance? Thanks!

@pimterrypimterry added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 7, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 7, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@codecov

codecovBot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.25%. Comparing base (3ee5f31) to head (1a0b0f4).
⚠️ Report is 47 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #64145 +/- ##
==========================================
- Coverage 92.04% 90.25% -1.80% 
==========================================
Files 381 741 +360 Lines 169252 240985 +71733 Branches 25941 45403 +19462 ==========================================
+ Hits 155792 217501 +61709 - Misses 13173 15057 +1884 - Partials 287 8427 +8140 
Files with missing linesCoverage Δ
lib/tls.js93.13% <100.00%> (+1.34%)⬆️

... and 500 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.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/74649/

@pimterrypimterry added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 8, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 8, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 1d87a24 into nodejs:mainJul 8, 2026
81 of 82 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 1d87a24

@JumpLink
JumpLink deleted the tls-ipv6-ip-san branch July 8, 2026 19:18
aduh95 pushed a commit that referenced this pull request Jul 21, 2026
checkServerIdentity() stopped matching an IPv6 host against a matching
IP-Address SAN. The hostname is now run through domainToASCII() before
the net.isIP() gate, and domainToASCII('::1') === '' (an IPv6 literal is
not a domain), so net.isIP('') is 0, the IP-SAN branch is skipped, and
verification fails with "Cert does not contain a DNS name". IPv4 is
unaffected because dotted-decimal survives domainToASCII().
Match IP hosts against the original hostname instead of the IDNA-
normalized one. net.isIP() rejects non-ASCII input, so there is no IDNA
confusion to guard against for an IP literal; the normalized form is
still used for the DNS-name path.
Fixes: #64144
Signed-off-by: Pascal Garber <pascal@artandcode.studio>
PR-URL: #64145
Reviewed-By: Tim Perry <pimterry@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Jul 29, 2026
checkServerIdentity() stopped matching an IPv6 host against a matching
IP-Address SAN. The hostname is now run through domainToASCII() before
the net.isIP() gate, and domainToASCII('::1') === '' (an IPv6 literal is
not a domain), so net.isIP('') is 0, the IP-SAN branch is skipped, and
verification fails with "Cert does not contain a DNS name". IPv4 is
unaffected because dotted-decimal survives domainToASCII().
Match IP hosts against the original hostname instead of the IDNA-
normalized one. net.isIP() rejects non-ASCII input, so there is no IDNA
confusion to guard against for an IP literal; the normalized form is
still used for the DNS-name path.
Fixes: #64144
Signed-off-by: Pascal Garber <pascal@artandcode.studio>
PR-URL: #64145
Reviewed-By: Tim Perry <pimterry@gmail.com>
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
checkServerIdentity() stopped matching an IPv6 host against a matching
IP-Address SAN. The hostname is now run through domainToASCII() before
the net.isIP() gate, and domainToASCII('::1') === '' (an IPv6 literal is
not a domain), so net.isIP('') is 0, the IP-SAN branch is skipped, and
verification fails with "Cert does not contain a DNS name". IPv4 is
unaffected because dotted-decimal survives domainToASCII().
Match IP hosts against the original hostname instead of the IDNA-
normalized one. net.isIP() rejects non-ASCII input, so there is no IDNA
confusion to guard against for an IP literal; the normalized form is
still used for the DNS-name path.
Fixes: #64144
Signed-off-by: Pascal Garber <pascal@artandcode.studio>
PR-URL: #64145
Reviewed-By: Tim Perry <pimterry@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ciPRs that need a full CI run.tlsIssues and PRs related to the tls subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tls: checkServerIdentity() no longer matches IPv6 IP-Address SANs (regressed in v24.17.0)

3 participants

@JumpLink@nodejs-github-bot@pimterry