Skip to content

Commit 12eb256

Browse files
authored
Optimize dev proxy upstream performance (#896)
* Streamline the dev proxy guide to match the ts CLI install Replace the verbose `cargo run --manifest-path … --target … -- dev proxy` invocations with the installed `ts` binary (via `cargo install-cli`), so the guide reads the same way as the CLI guide. Note that `ts dev proxy` is macOS-only and phrase the install step as "install or update". Make the quick start a complete first run (`ca install`, then run with `--rewrite-host` and `--launch chrome`), and make `--rewrite-host` the recommended default for dev and staging upstreams. The Host-header guidance now turns on whether the upstream accepts `Host: <FROM>` rather than on the upstream being Trusted Server Compute. Correct several stale or wrong examples, verified against the CLI source: - Firefox `certutil` now uses `-d "sql:…"` and the full CA nickname, matching the auto-import the tool performs. - Add the missing `--connect-timeout` option and note that `[COMMAND]` is `ca`. - Prefer `--basic-auth-file` over inline `--basic-auth` in the staging example. - Clarify the CA key is stored outside the repository only by default, and rename "First run: CA setup" to "The development CA". * Address deep-review findings on the dev proxy guide Keep `--rewrite-host` in the quick start (it is required for upstreams that reject `Host: <FROM>`), but state its trade-off honestly: against a real Trusted Server adapter, first-party URLs then render on the upstream host, not the production domain. Qualify the "first-party URLs always stay on the production domain" claim accordingly — it holds only when the upstream preserves `X-Forwarded-Host`. Also: - Note that Firefox CA auto-import needs `certutil` (`brew install nss`) and qualify the troubleshooting row that assumed it is always present. - State that `--launch` opens only the first mapped hostname while every mapping is still proxied. - Note that connection options (`--rewrite-host`, auth, `--insecure`, `--upstream-plaintext`) apply to every mapping, not per-rule. - Scope the Safari "only while the proxy runs" claim to a clean exit. * docs: design dev proxy performance optimizations * docs: tighten dev proxy performance design * docs: preserve body semantics in retry design * docs: resolve deep review of proxy performance design * docs: define early-response pool behavior * docs: align proxy lease ownership wording * docs: close final proxy performance design gaps * docs: define proxy benchmark controls * docs: plan dev proxy performance optimizations * Address final dev proxy design review * Harden dev proxy shutdown design * Harden dev proxy lifecycle and benchmarks * Establish dev proxy performance baseline * Precompute dev proxy upstream identity * Optimize dev proxy upstream performance * Harden dev proxy connection lifecycle * Plan dev proxy review backfill * Correct dev proxy request outcome metrics * Use a fresh connection after stale readiness * Remove rejected HTTP2 origin state * Cover dev proxy connector configuration * Backfill dev proxy credential safety tests * Cover dev proxy streaming failure lifecycles * Cover forwarding pool isolation and overread * Backfill adversarial proxy shutdown coverage * Cover dev proxy pool key isolation * Complete dev proxy review backfill * Quiet blind tunnel failures for unmatched hosts Only unmatched hosts reach the blind tunnel, and real publisher pages reference many third-party domains that are dead, blackholed, or DNS-blocked. Logging each failure at warn level drowned out failures on mapped upstreams, so log them at debug instead. The browser still receives its 502 and the failure remains visible at debug level. * Address dev proxy review feedback
1 parent 516a827 commit 12eb256

24 files changed

Lines changed: 8536 additions & 321 deletions

crates/trusted-server-cli/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ tokio-rustls = { workspace = true }
5757
webpki-roots = { workspace = true }
5858

5959
[target.'cfg(target_os = "macos")'.dev-dependencies]
60+
tokio = { workspace = true, features = ["test-util"] }
6061
x509-parser = { workspace = true }
6162

6263
[target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies]

crates/trusted-server-cli/src/commands/dev/proxy/browser.rs

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,19 @@ fn restore_auto_proxy(
738738
mod tests {
739739
use super::*;
740740
use crate::commands::dev::proxy::rewrite::{Authority, Rule, RuleTable};
741+
use crate::commands::dev::proxy::upstream::key::AddressPolicy;
742+
743+
fn rule(from: &str, to: &str) -> Rule {
744+
Rule::new(
745+
from.to_string(),
746+
Authority::parse(to, false).expect("should parse authority"),
747+
false,
748+
false,
749+
false,
750+
AddressPolicy::Dns,
751+
)
752+
.expect("should build rule")
753+
}
741754

742755
#[test]
743756
fn shell_quote_wraps_and_escapes() {
@@ -826,12 +839,10 @@ mod tests {
826839

827840
#[test]
828841
fn pac_uses_normalized_connect_address_for_wildcard_bind() {
829-
let rules = RuleTable(vec![Rule {
830-
from: "www.example-publisher.com".into(),
831-
to: Authority::parse("to.edgecompute.app", false).expect("should parse authority"),
832-
rewrite_host: false,
833-
plaintext: false,
834-
}]);
842+
let rules = RuleTable(vec![rule(
843+
"www.example-publisher.com",
844+
"to.edgecompute.app",
845+
)]);
835846
let pac = generate_pac(&rules, "0.0.0.0:18080".parse().expect("addr"));
836847
assert!(
837848
pac.contains("PROXY 127.0.0.1:18080"),
@@ -845,12 +856,10 @@ mod tests {
845856

846857
#[test]
847858
fn pac_proxies_only_https_for_from_hosts() {
848-
let rules = RuleTable(vec![Rule {
849-
from: "www.example-publisher.com".into(),
850-
to: Authority::parse("to.edgecompute.app", false).expect("should parse authority"),
851-
rewrite_host: false,
852-
plaintext: false,
853-
}]);
859+
let rules = RuleTable(vec![rule(
860+
"www.example-publisher.com",
861+
"to.edgecompute.app",
862+
)]);
854863
let pac = generate_pac(
855864
&rules,
856865
"127.0.0.1:18080".parse().expect("should parse addr"),

crates/trusted-server-cli/src/commands/dev/proxy/ca.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ impl CertAuthority {
130130
///
131131
/// Panics if the leaf-cache mutex is poisoned by a prior panic while held.
132132
pub fn server_config(&self, host: &str) -> Result<Arc<ServerConfig>, Report<CaError>> {
133+
let normalized = host.to_ascii_lowercase();
134+
let host = normalized.as_str();
133135
// Fast path: return a cached config without holding the lock during minting.
134136
{
135137
let cache = self
@@ -152,6 +154,19 @@ impl CertAuthority {
152154
Ok(Arc::clone(entry))
153155
}
154156

157+
/// Reports whether a normalized leaf identity is already cached.
158+
///
159+
/// # Panics
160+
///
161+
/// Panics if the leaf-cache mutex was poisoned by an earlier panic.
162+
#[must_use]
163+
pub fn is_cached(&self, host: &str) -> bool {
164+
self.leaves
165+
.lock()
166+
.expect("should be able to acquire leaf cache lock")
167+
.contains_key(&host.to_ascii_lowercase())
168+
}
169+
155170
fn mint(&self, host: &str) -> Result<ServerConfig, Report<CaError>> {
156171
let leaf_key = KeyPair::generate().change_context(CaError::Generate)?;
157172

@@ -305,6 +320,19 @@ mod tests {
305320
assert!(!Arc::ptr_eq(&a, &c), "different host mints a new config");
306321
}
307322

323+
#[test]
324+
fn leaf_cache_normalizes_dns_case() {
325+
let dir = tempfile::tempdir().expect("should create tempdir");
326+
let ca = CertAuthority::load_or_generate(dir.path()).expect("should generate");
327+
let lower = ca
328+
.server_config("www.example.com")
329+
.expect("mint lowercase leaf");
330+
let mixed = ca
331+
.server_config("WWW.Example.COM")
332+
.expect("reuse mixed-case leaf");
333+
assert!(Arc::ptr_eq(&lower, &mixed));
334+
}
335+
308336
#[test]
309337
fn mints_leaf_for_ip_literal_host() {
310338
// An IP-literal host must mint successfully (IP-type SAN, not DNS) — spec §8.3.

crates/trusted-server-cli/src/commands/dev/proxy/config.rs

Lines changed: 148 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ use std::path::PathBuf;
66

77
use base64::Engine as _;
88
use error_stack::{Report, ResultExt as _};
9+
use hyper::header::HeaderValue;
910

1011
use super::ProxyArgs;
1112
use super::rewrite::{Authority, Rule, RuleTable};
13+
use super::upstream::key::AddressPolicy;
1214

1315
/// Errors from configuration resolution.
1416
#[derive(Debug, derive_more::Display)]
@@ -52,27 +54,40 @@ pub enum ConfigError {
5254
impl core::error::Error for ConfigError {}
5355

5456
/// Basic-auth credentials to inject upstream.
55-
#[derive(Debug, Clone)]
57+
#[derive(Clone)]
5658
pub struct BasicAuth {
57-
pub user: String,
58-
pub pass: String,
59+
header: HeaderValue,
5960
}
6061

6162
impl BasicAuth {
62-
/// The `Authorization` header value (`Basic base64(user:pass)`).
63+
/// Precomputes a validated `Authorization` header.
64+
///
65+
/// # Errors
66+
///
67+
/// Returns [`ConfigError::BasicAuth`] if the generated value is not a valid
68+
/// HTTP header value.
69+
pub fn new(user: &str, pass: &str) -> Result<Self, ConfigError> {
70+
let token = base64::engine::general_purpose::STANDARD.encode(format!("{user}:{pass}"));
71+
let header =
72+
HeaderValue::from_str(&format!("Basic {token}")).map_err(|_| ConfigError::BasicAuth)?;
73+
Ok(Self { header })
74+
}
75+
76+
/// The prevalidated `Authorization` header value.
6377
#[must_use]
64-
pub fn header_value(&self) -> String {
65-
let token = base64::engine::general_purpose::STANDARD
66-
.encode(format!("{}:{}", self.user, self.pass));
67-
format!("Basic {token}")
78+
pub fn header_value(&self) -> &HeaderValue {
79+
&self.header
6880
}
6981

7082
fn parse(raw: &str) -> Result<Self, ConfigError> {
7183
let (user, pass) = raw.split_once(':').ok_or(ConfigError::BasicAuth)?;
72-
Ok(Self {
73-
user: user.to_string(),
74-
pass: pass.to_string(),
75-
})
84+
Self::new(user, pass)
85+
}
86+
}
87+
88+
impl core::fmt::Debug for BasicAuth {
89+
fn fmt(&self, formatter: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
90+
formatter.write_str("BasicAuth([REDACTED])")
7691
}
7792
}
7893

@@ -163,6 +178,7 @@ fn build_rules(args: &ProxyArgs) -> Result<RuleTable, ConfigError> {
163178
to,
164179
args.rewrite_host,
165180
args.upstream_plaintext,
181+
args.insecure,
166182
)?);
167183
}
168184
if let (Some(from), Some(to)) = (&args.from, &args.to) {
@@ -171,6 +187,7 @@ fn build_rules(args: &ProxyArgs) -> Result<RuleTable, ConfigError> {
171187
to,
172188
args.rewrite_host,
173189
args.upstream_plaintext,
190+
args.insecure,
174191
)?);
175192
}
176193
Ok(RuleTable(rules))
@@ -221,18 +238,22 @@ fn make_rule(
221238
to: &str,
222239
rewrite_host: bool,
223240
plaintext: bool,
241+
insecure: bool,
224242
) -> Result<Rule, ConfigError> {
225243
let from = from.to_ascii_lowercase();
226244
if !is_valid_host(&from) {
227245
return Err(ConfigError::InvalidFrom { value: from });
228246
}
229247
let to = Authority::parse(to, plaintext).map_err(|_| ConfigError::Rule)?;
230-
Ok(Rule {
248+
Rule::new(
231249
from,
232250
to,
233251
rewrite_host,
234252
plaintext,
235-
})
253+
insecure,
254+
AddressPolicy::Dns,
255+
)
256+
.map_err(|_| ConfigError::Rule)
236257
}
237258

238259
/// Resolves arguments into a [`ResolvedConfig`].
@@ -243,7 +264,7 @@ fn make_rule(
243264
/// invalid/forbidden listen address, malformed credentials, or an unknown
244265
/// browser.
245266
pub fn resolve(args: &ProxyArgs) -> Result<ResolvedConfig, Report<ConfigError>> {
246-
let rules = build_rules(args).map_err(Report::from)?;
267+
let mut rules = build_rules(args).map_err(Report::from)?;
247268
if rules.0.is_empty() {
248269
return Err(Report::new(ConfigError::NoRule));
249270
}
@@ -281,6 +302,12 @@ pub fn resolve(args: &ProxyArgs) -> Result<ResolvedConfig, Report<ConfigError>>
281302
let ca_dir = ca_dir(args);
282303
let resolve = build_resolve(args).map_err(Report::from)?;
283304

305+
for rule in &mut rules.0 {
306+
if let Some(address) = resolve.get(rule.to.host()) {
307+
rule.set_address_policy(AddressPolicy::Resolve(*address));
308+
}
309+
}
310+
284311
// A `--resolve HOST:IP` whose HOST matches no rule's TO host is almost
285312
// certainly a typo: the pin would silently never apply. Warn rather than
286313
// error, so a deliberate pin for a host reached indirectly still works.
@@ -322,7 +349,14 @@ fn resolve_basic_auth(args: &ProxyArgs) -> Result<Option<BasicAuth>, ConfigError
322349

323350
#[cfg(test)]
324351
mod tests {
352+
use hyper::header::HeaderValue;
353+
use rustls::pki_types::ServerName;
354+
325355
use super::*;
356+
use crate::commands::dev::proxy::rewrite::rewrite_for;
357+
use crate::commands::dev::proxy::upstream::key::{
358+
AddressPolicy, OriginKey, ReferenceIdentity, Transport, VerifyMode,
359+
};
326360

327361
fn base_args() -> crate::commands::dev::proxy::ProxyArgs {
328362
// Construct via clap so defaults match the real surface.
@@ -367,7 +401,11 @@ mod tests {
367401
.rules
368402
.first_match("www.example-publisher.com")
369403
.expect("rule present");
370-
assert!(!rule.rewrite_host, "default preserves FROM host");
404+
assert_eq!(
405+
rewrite_for(rule).host_header,
406+
HeaderValue::from_static("www.example-publisher.com"),
407+
"default preserves FROM host"
408+
);
371409
assert_eq!(rule.to.host(), "to.edgecompute.app");
372410
}
373411

@@ -377,11 +415,14 @@ mod tests {
377415
args.map = vec!["www.example-publisher.com=to.edgecompute.app".into()];
378416
args.rewrite_host = true;
379417
let cfg = resolve(&args).expect("should resolve");
380-
assert!(
381-
cfg.rules
382-
.first_match("www.example-publisher.com")
383-
.expect("rule")
384-
.rewrite_host,
418+
assert_eq!(
419+
rewrite_for(
420+
cfg.rules
421+
.first_match("www.example-publisher.com")
422+
.expect("rule")
423+
)
424+
.host_header,
425+
HeaderValue::from_static("to.edgecompute.app"),
385426
"--rewrite-host sends Host: TO"
386427
);
387428
}
@@ -501,15 +542,96 @@ mod tests {
501542

502543
#[test]
503544
fn basic_auth_header_is_base64() {
504-
let auth = BasicAuth {
505-
user: "dev".into(),
506-
pass: "secret".into(),
507-
};
545+
let auth = BasicAuth::new("dev", "secret").expect("should build auth");
508546
assert_eq!(
509547
auth.header_value(),
510-
"Basic ZGV2OnNlY3JldA==",
548+
&HeaderValue::from_static("Basic ZGV2OnNlY3JldA=="),
511549
"Basic base64(user:pass)"
512550
);
551+
let debug = format!("{auth:?}");
552+
assert_eq!(debug, "BasicAuth([REDACTED])", "Debug should be redacted");
553+
assert!(!debug.contains("dev"), "Debug should not contain the user");
554+
assert!(
555+
!debug.contains("secret") && !debug.contains("ZGV2OnNlY3JldA=="),
556+
"Debug should not contain raw or encoded credentials"
557+
);
558+
}
559+
560+
#[test]
561+
fn resolve_precomputes_typed_rule_identity_and_headers() {
562+
let mut args = base_args();
563+
args.map = vec!["www.example.com=TO.Example.com:8443".into()];
564+
args.rewrite_host = true;
565+
args.insecure = true;
566+
args.resolve = vec!["to.example.com:192.0.2.10".into()];
567+
568+
let cfg = resolve(&args).expect("should resolve");
569+
let rule = cfg
570+
.rules
571+
.first_match("www.example.com")
572+
.expect("should find rule");
573+
let outcome = rewrite_for(rule);
574+
575+
assert_eq!(
576+
outcome.host_header,
577+
HeaderValue::from_static("to.example.com:8443"),
578+
"should prevalidate upstream Host"
579+
);
580+
assert_eq!(
581+
outcome.orig_host,
582+
HeaderValue::from_static("www.example.com"),
583+
"should prevalidate forwarding host"
584+
);
585+
assert_eq!(
586+
outcome.sni,
587+
Some(ServerName::try_from("to.example.com").expect("should parse server name")),
588+
"should prevalidate normalized SNI"
589+
);
590+
assert_eq!(
591+
rule.origin_key(),
592+
&OriginKey::new(
593+
Transport::Tls,
594+
ReferenceIdentity::dns("to.example.com"),
595+
8443,
596+
VerifyMode::Insecure,
597+
AddressPolicy::Resolve("192.0.2.10".parse().expect("should parse pin")),
598+
),
599+
"should precompute the complete transport identity"
600+
);
601+
}
602+
603+
#[test]
604+
fn resolve_keeps_ip_reference_identities_http1_only() {
605+
let mut args = base_args();
606+
args.map = vec!["www.example.com=127.0.0.1".into()];
607+
args.rewrite_host = true;
608+
609+
let cfg = resolve(&args).expect("should resolve");
610+
let rule = cfg
611+
.rules
612+
.first_match("www.example.com")
613+
.expect("should find rule");
614+
615+
assert_eq!(
616+
rule.origin_key(),
617+
&OriginKey::new(
618+
Transport::Tls,
619+
ReferenceIdentity::ip("127.0.0.1".parse().expect("should parse IP")),
620+
443,
621+
VerifyMode::Secure,
622+
AddressPolicy::Dns,
623+
),
624+
"IP identities should validate as IP and remain HTTP/1-only"
625+
);
626+
assert_eq!(
627+
rewrite_for(rule).sni,
628+
Some(ServerName::from(
629+
"127.0.0.1"
630+
.parse::<IpAddr>()
631+
.expect("should parse IP server name"),
632+
)),
633+
"TLS should use an IP server name rather than DNS SNI"
634+
);
513635
}
514636

515637
#[test]

0 commit comments

Comments
 (0)