Skip to content

fix(net): normalize trailing-dot SNI in secret-injection matcher#771

Draft
G4614 wants to merge 1 commit into
boxlite-ai:mainfrom
G4614:fix/sec-13-14-host-matching
Draft

fix(net): normalize trailing-dot SNI in secret-injection matcher#771
G4614 wants to merge 1 commit into
boxlite-ai:mainfrom
G4614:fix/sec-13-14-host-matching

Conversation

@G4614

@G4614 G4614 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Source-level security audit fix. A guest-supplied SNI carrying the FQDN trailing
dot (e.g. api.openai.com.) is admitted by the allow_net filter (which
strips the dot) but the secret-injection matcher lowercased only and did not trim
it — so the connection was allowed and MITM'd, yet the <BOXLITE_SECRET:...>
placeholder was forwarded upstream unsubstituted instead of replaced.

Fix

Add a shared normalizeHost (lowercase + TrimSuffix(".")) and apply it in
both Matches and SecretsForHost, matching how the allowlist normalizes
hostnames. Behavior is unchanged for SNIs without a trailing dot.

Test (two-sided)

TestSecretHostMatcher_TrailingDot fails on every assertion with the change
reverted (trailing-dot host not matched) and passes with it applied.

Audit finding #14 (low). Note: finding #13 (allow_net wildcard depth) was
intentionally not changed — deep-subdomain matching is documented existing
behavior (TestTCPFilter_Wildcard) and never escapes the allowed parent domain.

🤖 Generated with Claude Code

A guest-supplied SNI carrying the FQDN trailing dot (e.g.
`api.openai.com.`) is admitted by the allow_net filter, which strips the
dot (tcp_filter.go MatchesHostname), but the secret-injection matcher
lowercased only and did not trim it. The result was an inconsistency: the
connection was allowed, MITM was attempted, yet `SecretHostMatcher.Matches`
returned false, so the `<BOXLITE_SECRET:...>` placeholder was forwarded
upstream unsubstituted instead of being replaced (or fail-closed).

Add a shared `normalizeHost` (lowercase + TrimSuffix ".") and apply it in
both `Matches` and `SecretsForHost`, matching how the allowlist normalizes
hostnames. Behavior is unchanged for SNIs without a trailing dot.

Two-sided test: TestSecretHostMatcher_TrailingDot fails on every assertion
with the production change reverted (placeholder host not matched), passes
with it applied.

Audit finding boxlite-ai#14 (low).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e2216cb9-245d-4b58-aed5-4c27e9840579

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@cla-assistant

cla-assistant Bot commented Jun 15, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


boxlite security fixes seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant