Skip to content

fix(proxy): normalize trailing-dot CONNECT hosts before policy evaluation#2248

Open
letv1nnn wants to merge 1 commit into
NVIDIA:mainfrom
letv1nnn:2201-normalize-trailing-dot-host
Open

fix(proxy): normalize trailing-dot CONNECT hosts before policy evaluation#2248
letv1nnn wants to merge 1 commit into
NVIDIA:mainfrom
letv1nnn:2201-normalize-trailing-dot-host

Conversation

@letv1nnn

Copy link
Copy Markdown

Summary

Fixes inconsistent handling of trailing-dot hostnames (e.g. api.example.com.) in the sandbox supervisor's CONNECT proxy. Such hosts are valid RFC 1034 FQDNs used in Kubernetes ndots:5 environments to bypass search-domain expansion, but were treated as distinct from their undotted equivalents across policy evaluation, middleware selection, and credential matching.

Related Issue

#2201

Changes

  • Strip one trailing dot from CONNECT host before policy checks, normalize_connect_host(raw_host: &str) -> &str {...}
  • DNS resolvers still get the original dotted form (preserves ndots:5 bypass)
  • Unit tests for the normalization helper

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@copy-pr-bot

copy-pr-bot Bot commented Jul 13, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@TaylorMutch

Copy link
Copy Markdown
Collaborator

/ok to test 8384df5

@letv1nnn letv1nnn force-pushed the 2201-normalize-trailing-dot-host branch from 8384df5 to 1544036 Compare July 13, 2026 19:20

@r3v5 r3v5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR: #2248
Author: letv1nnn (Artem Lytvyn)
Branch: 2201-normalize-trailing-dot-host -> main

Overview

Strips trailing dot from FQDN CONNECT hosts (e.g. api.example.com. → api.example.com) before policy evaluation, middleware selection, and credential matching. Fixes mismatches in K8s ndots:5 environments where dotted and undotted forms were treated as distinct hosts.

Key Design Decisions

  • Normalize once, early (proxy.rs:613): Single normalize_connect_host() call right after parse_target(), so all downstream policy logic sees the clean hostname.
  • Selective raw_host pass-through (proxy.rs:784, proxy.rs:837): resolve_and_check_trusted_gateway and resolve_and_check_allowed_ips still receive raw_host to preserve the ndots:5 DNS bypass
    behavior.

Notable Code

crates/openshell-supervisor-network/src/proxy.rs:4546

fn normalize_connect_host(raw_host: &str) -> &str {
    raw_host.strip_suffix('.').unwrap_or(raw_host)
}

Clean, zero-allocation — returns a sub-slice. Only strips one dot per RFC 1034.

Comment thread crates/openshell-supervisor-network/src/proxy.rs Outdated
Comment thread crates/openshell-supervisor-network/src/proxy.rs Outdated
@letv1nnn letv1nnn force-pushed the 2201-normalize-trailing-dot-host branch from 1544036 to 0ae8014 Compare July 14, 2026 13:18
@letv1nnn letv1nnn requested a review from r3v5 July 14, 2026 13:20

@r3v5 r3v5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good from my side!

@johntmyers

Copy link
Copy Markdown
Collaborator

/ok to test 0ae8014

@johntmyers johntmyers added the test:e2e Requires end-to-end coverage label Jul 14, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for 0ae8014. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@johntmyers johntmyers self-assigned this Jul 14, 2026
@johntmyers

Copy link
Copy Markdown
Collaborator

Hosts need to be normalized in the forward-proxy path as well. Currently, non-CONNECT requests are dispatched prior to the normalization. I think if we duplicate the normalization for now that's ok. We're going to refactor the proxy and align CONNECT and non-CONNECT paths under the same code. But to merge this as-is we need to make sure we normalize for both pathways.

@johntmyers

Copy link
Copy Markdown
Collaborator

Also need to normalize hostnames for hosts file (/etc/hosts) lookups and retain the raw_host for DNS.

@johntmyers johntmyers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments

…tion

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
@letv1nnn letv1nnn force-pushed the 2201-normalize-trailing-dot-host branch from 0ae8014 to 6e09c21 Compare July 14, 2026 16:17
@letv1nnn letv1nnn requested a review from johntmyers July 14, 2026 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants