tls: omit SNI for IP literal hosts by edsiper · Pull Request #11827 · fluent/fluent-bit

edsiper · 2026-05-19T15:52:14Z

Omit SNI when the target is an IP literal, while still sending SNI for real hostnames and explicit valid tls.vhost hostnames.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Summary by CodeRabbit

Bug Fixes
- SNI is no longer sent when connecting to IP literal addresses (IPv4 and IPv6), including bracketed IPv6 and zone‑identified literals.
Tests
- Added integration test scenario and tests to verify TLS SNI is omitted for IPv6 literal connections.

coderabbitai · 2026-05-19T15:52:38Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 764d8819-d6e6-48c6-b253-d1255475c2ce

📥 Commits

Reviewing files that changed from the base of the PR and between dc1fc9e and 62abdba.

📒 Files selected for processing (3)

src/tls/openssl.c
tests/integration/scenarios/tls/config/out_http_tls_ipv6_literal.yaml
tests/integration/scenarios/tls/tests/test_tls_sni_001.py

📝 Walkthrough

Walkthrough

The PR changes TLS handling to avoid sending SNI when connecting to IP-literal hostnames, updates platform headers, adapts hostname verification for IP literals, and adds an IPv6 integration test that asserts SNI is omitted for an IPv6 literal target.

Changes

TLS SNI IPv6 Literal Handling

Layer / File(s)	Summary
TLS includes and platform support `src/tls/openssl.c`	Adds `<string.h>` and updates platform-specific headers: Windows adds Winsock networking headers (`winsock2.h`, `ws2tcpip.h`) alongside `wincrypt.h` with `CERT_FIND_SHA256_HASH` fallback; non-Windows includes `<arpa/inet.h>`.
IP literal detection and conditional SNI setup `src/tls/openssl.c`	Introduces `host_is_ip_literal()` to recognize IPv4 literals, bracketed IPv6 literals, and IPv6 zone identifiers; adds `setup_sni()` to conditionally invoke `SSL_set_tlsext_host_name` only when hostname is not an IP literal.
Hostname verification adapts for IP literals `src/tls/openssl.c`	Updates `setup_hostname_validation` to use `X509_VERIFY_PARAM_set1_ip_asc` for IP literals and `X509_VERIFY_PARAM_set1_host` for DNS hostnames.
SNI handshake integration `src/tls/openssl.c`	Replaces direct `SSL_set_tlsext_host_name` calls in `tls_net_handshake` with `setup_sni(...)` for explicit `vhost` and `tls->vhost`, ensuring SNI is not emitted for IP literals.
Test scenario configuration and harness `tests/integration/scenarios/tls/config/out_http_tls_ipv6_literal.yaml`, `tests/integration/scenarios/tls/tests/test_tls_sni_001.py`	Adds YAML scenario targeting IPv6 literal `::1` with TLS and test module that implements an IPv6-capable threaded HTTP server, `TLSReceiver` that captures SNI via `set_servername_callback`, and a `Service` wrapper to manage lifecycle and request tracking.
Test case and IPv6 availability helper `tests/integration/scenarios/tls/tests/test_tls_sni_001.py`	Adds `ipv6_loopback_available()` to probe binding to `::1`; test `test_tls_sni_omits_ipv6_literals` skips if IPv6 unavailable, runs the service, waits for at least one outbound request, and asserts the first recorded SNI is `None`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

cosmo0920

Poem

🐰 A rabbit hops through IPv6 lands so bright,
Skipping SNI for literals, keeping TLS light,
Brackets and zones, the rabbit knows when to hide,
No hostname sent where IPs reside—
Hooray for tests that prove the rule right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title 'tls: omit SNI for IP literal hosts' directly and clearly describes the main change: suppressing SNI for IP literal addresses in TLS connections.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch tls-ip-sni-11815

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

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

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/tls/openssl.c`:
- Around line 1651-1655: The hostname verification currently always calls
X509_VERIFY_PARAM_set1_host() in setup_hostname_validation(), which fails for IP
literals; modify setup_hostname_validation() to check host_is_ip_literal(vhost)
(the same check used in setup_sni) and, if true, call
X509_VERIFY_PARAM_set1_ip_asc(param, vhost) instead of
X509_VERIFY_PARAM_set1_host(param, vhost), otherwise keep using
X509_VERIFY_PARAM_set1_host(); reference setup_sni, setup_hostname_validation,
host_is_ip_literal, X509_VERIFY_PARAM_set1_host, and
X509_VERIFY_PARAM_set1_ip_asc when making the change.

In `@tests/integration/scenarios/tls/tests/test_tls_sni_001.py`:
- Around line 45-46: Rename the shadowing parameter in the log_message method:
change the argument name from format to a non-shadowing name like fmt in the
log_message(self, fmt, *args) definition and update any internal references
inside log_message (if any) to use fmt; this removes the built-in shadowing
flagged by Ruff A002 while keeping the method signature and behavior intact.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7230e8c2-7b4e-411e-8f99-862f80145634

📥 Commits

Reviewing files that changed from the base of the PR and between bcc2436 and ccffeb2.

📒 Files selected for processing (3)

src/tls/openssl.c
tests/integration/scenarios/tls/config/out_http_tls_ipv6_literal.yaml
tests/integration/scenarios/tls/tests/test_tls_sni_001.py

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/tls/openssl.c`:
- Around line 86-138: host_is_ip_literal() currently normalizes bracketed IPv6
and zone-scoped addresses into a local buffer but only returns a boolean, and
the caller still passes the original hostname to X509_VERIFY_PARAM_set1_ip_asc()
which requires a bare numeric IP; change the code so the caller passes a
normalized literal: either extend host_is_ip_literal() to return the normalized
string (e.g., via an out parameter or returned pointer) alongside the boolean,
or, at the site where X509_VERIFY_PARAM_set1_ip_asc() is called, strip
surrounding '[' ']' and drop any "%zone" suffix before calling
X509_VERIFY_PARAM_set1_ip_asc(); ensure you use the same normalization logic
currently in host_is_ip_literal() (bracket removal and truncation at '%') and
only call X509_VERIFY_PARAM_set1_ip_asc() when the normalized string is a valid
inet_pton parse.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4952745f-7180-4c72-bfa2-a1fb310ecf65

📥 Commits

Reviewing files that changed from the base of the PR and between ccffeb2 and dc1fc9e.

📒 Files selected for processing (3)

src/tls/openssl.c
tests/integration/scenarios/tls/config/out_http_tls_ipv6_literal.yaml
tests/integration/scenarios/tls/tests/test_tls_sni_001.py

🚧 Files skipped from review as they are similar to previous changes (2)

tests/integration/scenarios/tls/config/out_http_tls_ipv6_literal.yaml
tests/integration/scenarios/tls/tests/test_tls_sni_001.py

Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>

edsiper requested a review from cosmo0920 as a code owner May 19, 2026 15:52

github-actions Bot added the docs-required label May 19, 2026

edsiper added this to the Fluent Bit v5.0.6 milestone May 19, 2026

edsiper temporarily deployed to pr May 19, 2026 15:52 — with GitHub Actions Inactive

coderabbitai Bot reviewed May 19, 2026

View reviewed changes

Comment thread src/tls/openssl.c

Comment thread tests/integration/scenarios/tls/tests/test_tls_sni_001.py Outdated

edsiper temporarily deployed to pr May 19, 2026 16:12 — with GitHub Actions Inactive

edsiper force-pushed the tls-ip-sni-11815 branch from ccffeb2 to dc1fc9e Compare May 20, 2026 00:42

edsiper temporarily deployed to pr May 20, 2026 00:42 — with GitHub Actions Inactive

coderabbitai Bot reviewed May 20, 2026

View reviewed changes

Comment thread src/tls/openssl.c Outdated

edsiper temporarily deployed to pr May 20, 2026 01:03 — with GitHub Actions Inactive

edsiper had a problem deploying to pr May 20, 2026 01:03 — with GitHub Actions Failure

edsiper temporarily deployed to pr May 20, 2026 01:03 — with GitHub Actions Inactive

edsiper added 2 commits May 19, 2026 22:29

tls: omit SNI for IP literal hosts

e07eeee

Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>

tests: integration: cover TLS SNI IP literals

62abdba

Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>

edsiper force-pushed the tls-ip-sni-11815 branch from dc1fc9e to 62abdba Compare May 20, 2026 04:30

edsiper temporarily deployed to pr May 20, 2026 04:30 — with GitHub Actions Inactive

edsiper temporarily deployed to pr May 20, 2026 04:49 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tls: omit SNI for IP literal hosts#11827

tls: omit SNI for IP literal hosts#11827
edsiper wants to merge 2 commits into
masterfrom
tls-ip-sni-11815

edsiper commented May 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 19, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

edsiper commented May 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

edsiper commented May 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 19, 2026 •

edited

Loading