proxyless: search the resolver × wire space for a strategy that reaches the destination - #14
Merged
Merged
Conversation
…es the destination Chunk 2 of the outline-sdk proxyless port. New `flint-proxyless` crate: the Rust counterpart to the Outline smart dialer's proxyless path (`x/smart`, findDNS × findTLS) — reach the real destination with no proxy and no exit hop by finding a `(resolver, wire)` pairing the local network does not block. Chunk 1 made the two axes orthogonal; this consumes them as a genuine cartesian product. A `Strategy` is one point in it, a `Space` declares the candidates, and `find` enumerates them **wire-major** — every resolver against the no-op plan first, then against each shaping plan — so "is any resolver reachable with no shaping at all" is settled before paying for shaping, mirroring the smart dialer resolving DNS before it searches TLS transports. Candidates race through `race_windowed` with a window of 4: each probe is a real lookup plus a real handshake, and firing dozens at once would be slow, wasteful, and conspicuous exactly where someone is watching. The certificate is the oracle. A censor can block a connection, and can answer a plaintext query with anything it likes, but cannot produce a valid certificate for the destination. So success is not "DNS answered" or "TCP connected" but *a TLS handshake to the resolved address completing against a verified chain and hostname*. Every dial here — probe and destination alike — uses `CertVerification::Roots`; an unverified dial would destroy the property the design rests on. Note this is deliberately stricter than the legacy resolver dial, which still inherits `CertVerification::None` (design §11). That oracle is what admits the poisonable resolver kinds here even though `default_pool()` excludes them: a forged answer cannot survive the handshake, so a poisoned resolver loses the race instead of silently winning it. Two properties worth stating because they are easy to get wrong: - A candidate must reach **all** test domains, not any, so one accidentally-open path cannot vouch for a hostile network. An empty domain list is rejected rather than trivially "succeeding" against everything. - A cached winner is **re-verified, not trusted** — what worked yesterday may be blocked today — so a caught strategy self-heals into a fresh search instead of pinning the client to a dead path. `StrategyCache` stores a `(resolver name, wire index)` pair, both plain values a caller can persist with any encoding, and the stable resolver name survives pool reordering or a signed update; a stale entry resolves to None rather than the wrong strategy. The verification step is injectable (`find_with` / `find_cached_with`), so the search logic — enumeration order, the all-domains rule, concurrency, failure reporting — is unit-tested with no network and no TLS engine. The real end-to-end path is a `#[ignore]`d live test, since dialing needs the `boring` feature. Scope stated in the crate docs and §6.2: no exit hop means traffic leaves the user's own address for the real destination. This defeats blocking, not observation, and does nothing against IP blackholing — a reachability tool, not an anonymity one, and not a silent stand-in for a proxy path. Gate: cargo fmt --all --check, cargo clippy --workspace --all-targets -D warnings, cargo test --workspace (12 flint-proxyless tests, 1 live-ignored), cargo doc with zero warnings, and cargo check -p flint-proxyless --features boring all clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BufGsK81otiqihvrZdfoJX
📝 WalkthroughWalkthroughAdds the ChangesProxyless reachability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant find_cached_with
participant StrategyCache
participant probe
participant DNSResolver
participant TLSDial
Caller->>find_cached_with: request strategy for network
find_cached_with->>StrategyCache: read cached winner
find_cached_with->>probe: verify candidate across domains
probe->>DNSResolver: resolve domain through strategy
DNSResolver-->>probe: resolved address
probe->>TLSDial: establish verified TLS
TLSDial-->>probe: handshake result
probe-->>find_cached_with: candidate success or failure
find_cached_with-->>Caller: verified Strategy
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
crates/flint-proxyless/src/lib.rs (3)
341-355: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
resolve_firstbakes inPROBE_PORTeven for callers that discard it.
dial()(Line 337) only uses.ip()from the result and substitutes its ownport, so thePROBE_PORT-taggedSocketAddrbuilt here is thrown away in that path. ReturningIpAddrinstead (lettingprobe()attachPROBE_PORTitself) would make the helper's contract match both call sites without the discarded intermediate value.🤖 Prompt for 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. In `@crates/flint-proxyless/src/lib.rs` around lines 341 - 355, The resolve_first helper should return the resolved IpAddr rather than constructing a SocketAddr with PROBE_PORT. Update its return type and result construction, then adjust callers such as dial() and probe() so each attaches its own required port while preserving the existing address resolution and error behavior.
99-162: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDuplicate index-decomposition logic between
strategy()andentry_for().Both methods independently re-derive the same
index % n(resolver) /index / n(wire) wire-major mapping. Sinceentry_for's output is later fed straight back throughEntry::resolve(cache.rs) to reconstruct aStrategy, any future drift between the two formulas would silently mismatch cached winners to the wrong candidate rather than erroring.♻️ Suggested consolidation
impl Space { + /// Decompose `index` into `(resolver_index, wire_index)`, or `None` if out of range. + fn decompose(&self, index: usize) -> Option<(usize, usize)> { + let n = self.resolvers.len(); + if n == 0 || index >= self.len() { + return None; + } + Some((index % n, index / n)) + } + pub fn strategy(&self, index: usize) -> Option<Strategy> { - let n = self.resolvers.len(); - if n == 0 || index >= self.len() { - return None; - } + let (r, w) = self.decompose(index)?; Some(Strategy { - resolver: self.resolvers[index % n].clone(), - wire: self.wires[index / n].clone(), + resolver: self.resolvers[r].clone(), + wire: self.wires[w].clone(), }) } pub fn entry_for(&self, index: usize) -> Option<cache::Entry> { - let n = self.resolvers.len(); - if n == 0 || index >= self.len() { - return None; - } + let (r, w) = self.decompose(index)?; Some(cache::Entry { - resolver: self.resolvers[index % n].name.clone(), - wire: index / n, + resolver: self.resolvers[r].name.clone(), + wire: w, }) } }🤖 Prompt for 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. In `@crates/flint-proxyless/src/lib.rs` around lines 99 - 162, Consolidate the wire-major index decomposition used by Space::strategy and Space::entry_for into a single shared helper or canonical path. Reuse that logic in both methods so the resolver index and wire index remain identical, while preserving the existing out-of-range behavior and returned Strategy/cache::Entry values.
211-231: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPer-candidate failure detail is discarded on total failure.
race_windowedreturns the accumulatedVec<io::Error>on failure, butsearchthrows it away and reports only a count (FindError::AllFailed { tried }). For a tool whose whole point is diagnosing why a network blocks reachability, surfacing at least the last/representative error (DNS failure vs. handshake failure vs. cert mismatch) would materially help callers and operators distinguish censorship signatures from local misconfiguration.🤖 Prompt for 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. In `@crates/flint-proxyless/src/lib.rs` around lines 211 - 231, Update search’s race_windowed error mapping to retain and expose a representative per-candidate io::Error, such as the last accumulated error, in FindError::AllFailed instead of discarding the Vec<io::Error>. Extend or use the existing FindError representation while preserving the tried count and successful search behavior.crates/flint-proxyless/Cargo.toml (1)
17-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake the runtime dependency used or remove it.
flint-proxylessdeclarestokiowithrt/timein production dependencies, but the crate only usestokioinside#[cfg(test)]; the#[async_trait]impl depends on executor/runtime features from the calling crate, not this crate’s direct usage. Either gate a needed runtime feature behind a feature flag or remove the[dependencies]entry to avoid forcingrt/timeon downstream crates.🤖 Prompt for 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. In `@crates/flint-proxyless/Cargo.toml` around lines 17 - 21, Remove the unused tokio entry from the flint-proxyless [dependencies] section, since tokio is only referenced by cfg(test) code. Preserve the test-only tokio usage through the crate’s existing dev-dependency configuration or add the appropriate dev dependency if needed, without imposing runtime features on downstream users.
🤖 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.
Nitpick comments:
In `@crates/flint-proxyless/Cargo.toml`:
- Around line 17-21: Remove the unused tokio entry from the flint-proxyless
[dependencies] section, since tokio is only referenced by cfg(test) code.
Preserve the test-only tokio usage through the crate’s existing dev-dependency
configuration or add the appropriate dev dependency if needed, without imposing
runtime features on downstream users.
In `@crates/flint-proxyless/src/lib.rs`:
- Around line 341-355: The resolve_first helper should return the resolved
IpAddr rather than constructing a SocketAddr with PROBE_PORT. Update its return
type and result construction, then adjust callers such as dial() and probe() so
each attaches its own required port while preserving the existing address
resolution and error behavior.
- Around line 99-162: Consolidate the wire-major index decomposition used by
Space::strategy and Space::entry_for into a single shared helper or canonical
path. Reuse that logic in both methods so the resolver index and wire index
remain identical, while preserving the existing out-of-range behavior and
returned Strategy/cache::Entry values.
- Around line 211-231: Update search’s race_windowed error mapping to retain and
expose a representative per-candidate io::Error, such as the last accumulated
error, in FindError::AllFailed instead of discarding the Vec<io::Error>. Extend
or use the existing FindError representation while preserving the tried count
and successful search behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 02b0c8c8-44d4-4771-bc2a-58e0b76474d7
📒 Files selected for processing (5)
Cargo.tomlcrates/flint-proxyless/Cargo.tomlcrates/flint-proxyless/src/cache.rscrates/flint-proxyless/src/lib.rsdocs/design.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New
flint-proxylesscrate — the Rust counterpart to the Outline SDK smart dialer's proxyless path (x/smart,findDNS × findTLS): reach the real destination with no proxy and no exit hop, by finding a(resolver, wire)pairing the local network does not block.Chunk 2 of the proxyless port. #13 made the DNS and shaping axes orthogonal; this consumes them as a genuine cartesian product.
The certificate is the oracle
This is the load-bearing idea, so it is worth stating before the mechanics.
A censor can block a connection, and can answer a plaintext DNS query with anything it likes. What it cannot do is produce a valid certificate for the destination. So the test for "did this strategy work" is not did DNS answer or did TCP connect — it is did a TLS handshake to the resolved address complete against a verified chain and hostname.
Every dial in this crate, probe and destination alike, uses
CertVerification::Roots. An unverified dial would destroy the property the whole design rests on. Note this is deliberately stricter than the legacy resolver dial, which still inheritsCertVerification::None(design §11, the follow-up from #13).That oracle is also what admits the poisonable resolver kinds here even though
default_pool()excludes them: a forged answer cannot survive the handshake, so a poisoned resolver loses the race rather than silently winning it.flowchart TB S["Space — resolvers × wires"] --> E["enumerate wire-major<br/>all resolvers on no-op plan first"] E --> R["race_windowed, window = 4"] R --> P["probe candidate"] subgraph ORACLE["probe — the oracle"] direction TB P1["resolve domain via Kind + WirePlan<br/>flint_dns::resolve_one_shaped"] P2["dial resolved IP with the same WirePlan"] P3["verify chain + hostname<br/>CertVerification::Roots"] P1 --> P2 --> P3 end P --> ORACLE ORACLE -->|"all test domains pass"| W["winning Strategy"] ORACLE -.->|"any domain fails"| X["candidate abandoned"] W --> C["StrategyCache — resolver name + wire index<br/>re-verified on next use, never trusted"] W --> D["dial host:port — verified destination stream"]Search behaviour, and why
Wire-major enumeration. Every resolver against the no-op plan first, then against each shaping plan in turn. That settles "is any resolver reachable with no shaping at all" before paying for shaping — the same instinct as the smart dialer resolving DNS before searching TLS transports.
A window of 4. Each probe is a real DNS lookup plus a real TLS handshake. Firing dozens at once would be slow, wasteful, and conspicuous on exactly the networks that are watching.
race_windowedrefills as each finishes, so the common case (an early candidate works) never touches the tail. There is a test asserting peak in-flight never exceeds the window.All test domains, not any. One domain might be reachable by accident while the network is still hostile to the rest, so a partial pass must not win. An empty domain list is rejected outright rather than trivially "succeeding" against everything — a search with no oracle proves nothing.
A cached winner is re-verified, not trusted. What worked yesterday may be blocked today. On a cache hit the strategy is re-probed; if that fails the entry is forgotten and the full search runs, so a strategy the censor has since caught cannot pin the client to a dead path.
StrategyCachestores a(resolver name, wire index)pair — both plain values a caller can persist with whatever encoding it already uses, and the stable resolver name survives pool reordering or a signed pool update. A stale entry resolves toNonerather than to the wrong strategy.Testability
The verification step is injectable (
find_with/find_cached_with), so the search logic — enumeration order, the all-domains rule, concurrency bounds, failure reporting, cache hit/miss/self-heal — is unit-tested with no network and no TLS engine. That matters here becauseflint_dial::dialneeds theboringfeature, which the default CI job does not enable; without injection none of this would be covered bycargo test --workspace.The real end-to-end path is a
#[ignore]d live test (live_find_reaches_real_domains), matching the existing convention inflint-dns.Scope, stated plainly
No exit hop means traffic leaves the user's own address for the real destination. This defeats blocking, not observation — an on-path censor still sees which host is being contacted, it just cannot classify or cut the TLS — and it does nothing against IP-level blackholing. It is a reachability tool, not an anonymity one, and must not silently stand in for a proxy path a user believes is carrying their traffic. This is in the crate docs and design §6.2 so it cannot be missed by a future caller.
Verification
cargo fmt --all -- --check— cleancargo clippy --workspace --all-targets -- -D warnings— cleancargo test --workspace— clean; 12 flint-proxyless tests (1 live-ignored)cargo doc -p flint-proxyless— zero warningscargo check -p flint-proxyless --features boring— clean (the live path's engine wiring)Next
Chunk 3 is
disorder/TTL as a new shaping axis, and the spark-side transport plusKindling::with_proxylessfor config fetches follow. The §11 resolver-verification gap from #13 remains the highest-value separate follow-up — this crate does not depend on it being fixed, since it verifies its own dials, but the legacy DoH path still does not.🤖 Generated with Claude Code
https://claude.ai/code/session_01BufGsK81otiqihvrZdfoJX
Summary by CodeRabbit
New Features
Documentation