[robobun]

Fixes #45514

Problem

• A node:quic client with servername: "127.0.0.1" and a CA-trusted certificate that carries IP:127.0.0.1 in its subjectAltName rejects session.opened with Peer certificate validation failed: Hostname mismatch [HOSTNAME_MISMATCH]. servername: "localhost" against the same certificate verifies. Node verifies both.
• Cause: src/runtime/node/quic/tls.rs:523 binds the servername with BoringSSL X509_VERIFY_PARAM_set1_host, which matches DNS names only. Node calls OpenSSL SSL_set1_host (src/quic/tlscontext.cc), which tries X509_VERIFY_PARAM_set1_ip_asc first and falls back to set1_host.

Fix

• TlsContext::new parses the servername with bun_core::ip_address::parse_strict. An IPv4 or IPv6 literal is bound with X509_VERIFY_PARAM_set1_ip (raw octets). Any other name still goes through set1_host.
• This mirrors OpenSSL's SSL_set1_host. parse_strict is the same IP-vs-DNS split bun's other TLS clients use (check_x509_server_identity in src/boringssl/lib.rs).
• The SNI is unchanged: Node sends the servername as SNI for an IP literal too (OSSLContext::set_hostname).
• Verified: test/js/node/quic/quic-sni.test.ts (5 new cases, 4 fail on the released bun). Also ran all of test/js/node/quic/.
• Self-reviewed: 2 concerns raised, 2 addressed (a DNS-mismatch negative test, and the accuracy of the workaround note below).

Background

• X509_VERIFY_PARAM is the per-context set of checks BoringSSL runs on the peer chain during the handshake. set1_host stores a DNS name pattern, set1_ip stores 4 or 16 address octets. An IP SAN only ever matches the latter.
• The reporter's workaround, verifyPeer: "manual", skips the servername binding and the auto-reject in session.opened. The chain is still verified and the result arrives in validationErrorReason, so the caller has to check that field and call peerCertificate.checkIP() by hand. Unmodified Node code with the default verifyPeer: "auto" never does that.
• Considered calling X509_VERIFY_PARAM_set1_ip_asc directly: it needs a NUL-terminated copy and a second IP parser. Considered replacing the verify-param check with bun's shared check_server_identity matcher: that has Node lib/tls.js semantics, and Node's QUIC uses OpenSSL's verify-param semantics, so it would diverge from the reference.

Downsides

• A certificate with a non-conformant DNS:127.0.0.1 SAN no longer matches servername: "127.0.0.1". OpenSSL and Node behave the same way.
• Every client session with verifyHostname on pays one parse_strict over the servername (at most 45 bytes, no allocation). A DNS servername longer than 45 bytes is rejected by the length check before any scan.

Notes

• An IP mismatch surfaces as IP address mismatch [UNSPECIFIED]: X509_V_ERR_IP_ADDRESS_MISMATCH is not in the X509Pointer::ErrorCode name table (src/jsc/bindings/ncrypto.cpp), which is a port of Node's table and stops at HOSTNAME_MISMATCH. The test asserts on the reason text.
• BoringSSL's own SSL_set1_host (vendor/boringssl/ssl/ssl_x509.cc:1153) does not do the IP fallback, so calling it instead of X509_VERIFY_PARAM_set1_host would not have fixed this.
• X509_VERIFY_PARAM_set1_ip copies the octets (vendor/boringssl/crypto/x509/x509_vpm.cc:340), so the stack buffer does not outlive the call.
• A zero-padded IPv4 servername (010.0.0.1) is not an IP literal here. BoringSSL's own a2i_ipadd rejects extra leading zeros because some parsers read them as octal (vendor/boringssl/crypto/x509/v3_utl.cc, get_ipv4_component). OpenSSL reads it as decimal 10. Such a name falls to the DNS-name path and fails closed.
• Ran: test/js/node/quic/quic-sni.test.ts, test/js/node/quic/quic-endpoint.test.ts, test/js/node/quic/quic-stream.test.ts on the debug build.

[robobun]

^{Updated 7:42 AM PT - Oct 17th, 2026}

✅ @robobun, your commit 1907059d4844c23774a949347f0a2e359a577c09 passed in Build #127402! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 45713

That installs a local version of the PR into your bun-45713 executable, so you can run:

bun-45713 --bun

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: a79eb854-39fc-4dd5-9a9b-2ac379ad667e

📥 Commits

Reviewing files that changed from the base of the PR and between 017b243 and 1907059.

📒 Files selected for processing: 3

• src/boringssl_sys/boringssl.rs
• src/runtime/node/quic/tls.rs
• test/js/node/quic/quic-sni.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

QUIC TLS certificate verification now checks IP server names against certificate IP subject alternative names. DNS server names continue to use hostname verification. Tests cover matching and mismatched IP and DNS names.

Changes

QUIC IP certificate verification

Layer / File(s)	Summary
IP-aware certificate verification src/boringssl_sys/boringssl.rs, src/runtime/node/quic/tls.rs, test/js/node/quic/quic-sni.test.ts	The TLS context uses X509_VERIFY_PARAM_set1_ip for parsed IP addresses and X509_VERIFY_PARAM_set1_host for other names. Tests cover matching IP SANs and IP or hostname mismatches.

Suggested reviewers: cirospaciari, jarred-sumner

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 19070

QUIC clients using an IP-literal servername now verify against the certificate's IP SANs, as the linked issue requests. DNS servernames behave as before. No merge-blocking risk was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Linked Issues check		Issue #45514 requires IP-literal QUIC servernames to verify against IP subjectAltName entries while DNS names continue to verify. TlsContext::new now uses bun_core::ip_address::parse_strict; it bi…
Out of Scope Changes check		The FFI declaration, TLS binding change, and QUIC tests directly implement issue #45514. The tests validate both the requested success behavior and the related mismatch behavior. No unrelated change i…
Title check		The title clearly and concisely describes the main change: QUIC now verifies an IP servername against certificate IP SANs.
Description check		The description explains the problem, cause, fix, scope, verification steps, test coverage, and known behavior changes. It provides the information required by the repository template, although it use…

Full details: Linked Issues check

Explanation

Issue #45514 requires IP-literal QUIC servernames to verify against IP subjectAltName entries while DNS names continue to verify. TlsContext::new now uses bun_core::ip_address::parse_strict; it binds IPv4 and IPv6 octets with X509_VERIFY_PARAM_set1_ip and uses X509_VERIFY_PARAM_set1_host for other names. The added tests cover successful IPv4 and IPv6 verification, mismatched IP addresses, and a mismatched DNS name.

Full details: Out of Scope Changes check

Explanation

The FFI declaration, TLS binding change, and QUIC tests directly implement issue #45514. The tests validate both the requested success behavior and the related mismatch behavior. No unrelated change is shown in the pull request diff.

Full details: Description check

Explanation

The description explains the problem, cause, fix, scope, verification steps, test coverage, and known behavior changes. It provides the information required by the repository template, although it uses different section headings.

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I also checked: the new X509_VERIFY_PARAM_set1_ip declaration matches the adjacent set1_host decl and SSL_CTX_get0_param's *mut c_void type, and the v4/v6 octet buffers only live until the FFI call, which copies them. The two behavior shifts that come with mirroring OpenSSL (a non-conformant DNS:127.0.0.1 SAN no longer matching an IP servername, and IP mismatches reporting UNSPECIFIED instead of HOSTNAME_MISMATCH as the code) are intentional per the description and match Node, so I did not flag them.

Extended reasoning...

The change touches QUIC client certificate verification in src/runtime/node/quic/tls.rs, adds one BoringSSL FFI declaration, and adds five test cases. It is security-sensitive (TLS hostname/IP binding on the peer certificate), and one confirmed finding about leading-zero IPv4 literals is posted inline, so a human should still look.